Last modified: 2014-07-24 01:15:16 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T13558, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 11558 - WebRequest: remove magic_quotes cruft
WebRequest: remove magic_quotes cruft
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.21.x
All All
: Lowest enhancement (vote)
: ---
Assigned To: Chad H.
:
Depends on:
Blocks: code_quality
  Show dependency treegraph
 
Reported: 2007-10-04 14:44 UTC by Brion Vibber
Modified: 2014-07-24 01:15 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
A bit of late-night tinkering (6.32 KB, patch)
2008-09-05 05:14 UTC, Chad H.
Details
Another hack at it (20.24 KB, patch)
2008-12-06 00:01 UTC, Chad H.
Details

Description Brion Vibber 2007-10-04 14:44:19 UTC
WebRequest is currently a little wonky in how it handles some things. It would be nice to do a cleanup, changing a few properties.

magic_quotes removal should be done on demand instead of modifying the global arrays at startup.

This avoids unnecessary transformation of unused parameters, and could aid in integration with other frameworks that might also be trying to do the same transformations on their end, potentially corrupting input data under the current regime.

Heck, currently instantiating two WebRequest objects in the same script will corrupt your data if magic_quotes_gpc is on, as the de-slashing will run once for each object. :)


Note however that WebRequest::checkMagicQuotes() currently applies the slash fixes to $_COOKIE, $_SERVER, and $_ENV, which are accessed directly in various places. It might be wise to add relevant access functions for them onto WebRequest.
Comment 1 Chad H. 2008-09-05 05:14:35 UTC
Created attachment 5291 [details]
A bit of late-night tinkering

In this patch, I've done two major things.
1) $_GET and $_POST are assigned to $data _before_ being de-quoted ($this->data is run through fix_magic_quotes(), as opposed to the raw $_GET and $_POST globals).
2) I've done the same for $_COOKIE and have given them the member variable $cookies in which to reside and be accessed via getCookies(). All direct calls to $_COOKIE in core have been updated.

While this doesn't address the major @fixme of the memory footprint incurred due to always dequoting on every request, it is a step in the right direction. Moving $_SERVER, $_ENV and $_REQUEST to member variables (and removing direct calls) would be another good step.
Comment 2 Daniel Friesen 2008-09-05 11:49:39 UTC
Just for a help note. I do have my own WebRequest class I use in another app of mine.
http://svn.nadir-point.com/viewvc/electronicme/trunk/electronicme/includes/model/WebRequest.php?view=markup
It handles quoting of the data lazily, feel free to use it as a base for trying to get MediaWiki's WebRequest to work without the memory footprint.
Note that eventually I'm going to move it into a library. So if that url disappears it'll be located at:
http://svn.nadir-point.com/viewvc/php-projects/trunk/WebModel/
Comment 3 Chad H. 2008-09-06 12:40:58 UTC
Went ahead and applied in r40530. Still needs some more work, but a good step in the right direction nonetheless.
Comment 4 Chad H. 2008-12-06 00:01:14 UTC
Created attachment 5563 [details]
Another hack at it

Spent part of the afternoon retooling WebRequest entirely. Values are now lazy-loaded and de-quoted on demand, rather than all at construction. Before this patch can be applied, however, ANY and ALL calls to any superglobals need to be updated to use their appropriate accessor method (getFileValue, getCookieValue, etc etc).

Kept all of the nice wrappers we use, should be backwards-compat.
Comment 5 Chad H. 2012-10-28 14:47:59 UTC
I looked at this again, and really I don't see any reason to change anything here. Things have changed a lot in 5 years ;-)

* Magic quotes is already deprecated in 5.3 (we require 5.3.2), so many people are unlikely to have it enabled
* Magic quotes is removed entirely in 5.4, so when we start requiring that we can just remove all the magic quotes cruft.
* Since we won't be iterating over it, copying the request info to $data on instantiation rather than lazy-loading seems fine (the extra logic won't save us *that* much memory on a typical request).
* The original problem of "can't instantiate multiple WebRequest objects" isn't really true unless you have magic quotes on.

So, repurposing this bug as a reminder and marking LATER for sometime after 5.4.
Comment 6 Andre Klapper 2012-12-21 14:09:38 UTC
[Removing RESOLVED LATER as discussed in http://lists.wikimedia.org/pipermail/wikitech-l/2012-November/064240.html . Reopening and setting priority to "Lowest". For future reference, please use either RESOLVED WONTFIX (for issues that will not be fixed), or simply set lowest priority. Thanks a lot!]
Comment 7 Quim Gil 2013-03-09 19:47:13 UTC
(In reply to comment #6)
> please use
> either RESOLVED WONTFIX (for issues that will not be fixed), or simply set
> lowest priority. Thanks a lot!]

The discussion reads as WONTFIX.
Comment 8 Chad H. 2013-03-09 22:38:58 UTC
It's not wontfix.
Comment 9 Gerrit Notification Bot 2014-07-09 15:48:16 UTC
Change 144996 had a related patch set uploaded by Chad:
Remove support for magic_quotes_gpc

https://gerrit.wikimedia.org/r/144996
Comment 10 Gerrit Notification Bot 2014-07-23 20:52:57 UTC
Change 144996 merged by jenkins-bot:
Remove support for magic_quotes_gpc

https://gerrit.wikimedia.org/r/144996

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links