The default configuration file, sites/default/default.settings.php, calls ini_set('magic_quotes_runtime', 0). This gives the impression that the user may actually change this setting, but doing this severely breaks Drupal and fubars the database (recovery seems hard).
To prevent users from making this mistake I suggest moving this setting out of the configuration file and into bootstrap.inc. I don't think it should be put in .htaccess, because this only works with Apache.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | magic_quotes_runtime_followup.patch | 1.66 KB | gpk |
| #6 | trace.txt | 7.48 KB | c960657 |
| #4 | magic_quotes_runtime-4.patch | 4.51 KB | c960657 |
| #3 | magic_quotes_runtime-3.patch | 4.46 KB | c960657 |
| #1 | magic_quotes_runtime-2.patch | 4.25 KB | c960657 |
Comments
Comment #1
c960657 commentedini_set() moved to drupal_initialize_variables(), following the convention introduced in #304924: Extend drupal_error_handler to manage exceptions.
Comment #2
dries commentedThe new code comments in sites/default/default.settings.php don't seem to wrap at the same width ad the old ones. Quick reroll?
Comment #3
c960657 commentedNow the whole paragraph is wrapped at 80 characters (webchick recommends this in her code reviews, though I couldn't find mention of this on drupal.org), with a newline following the URL to avoid punctuation. Parts of the file is already wrapped at 80 characters, though the indentation varies from paragraph to paragraph. I can rewrap the whole file if you like.
Related issue: #303154: Document ini_set() calls in default.settings.php
Comment #4
c960657 commentedChasing HEAD.
Comment #5
dries commentedThe only disadvantage I see is that magic_quotes are disabled much later in the process, and therefore we might have opened up a security issue. Are any hooks being called before magic quotes are set?
Comment #6
c960657 commentedAFAICT magic_quotes_runtime are now called earlier in the bootstrap process than before - see the attached Xdebug function trace. Or did I misunderstand you?
I tried enabling magic_quotes_runtime in .htaccess and the ran the complete test suite - all tests pass.
Comment #8
c960657 commentedComment #9
catchPatch looks good and c960657 is right about bootstrap ordering.
Comment #10
dries commentedc960657, you're absolutely right about the bootstrap ordering. Sorry to distract us. Committed this patch to CVS HEAD. Keep up the good work.
Comment #11
gpk commentedMinor docs cleanup including runtime/non-runtime confusion in bootstrap.inc.
Comment #12
webchickThanks for the comment clean-up! I committed as is, but I changed:
to:
Comment #13
gpk commentedOoh you are a stickler :P :)
Comment #15
webchickI try. ;)
Thanks for the clean-up jobs you've been doing though, I really appreciate it! It's important that our comments stay as squeaky clean as our code. :)
Comment #16
gpk commented>Thanks for the clean-up jobs you've been doing though
All 2 of them .. Some people are easy to please :D