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

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 11558 - WebRequest: remove magic_quotes cruft
WebRequest: remove magic_quotes cruft
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
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: ---

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

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.
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:
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 . 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
Comment 10 Gerrit Notification Bot 2014-07-23 20:52:57 UTC
Change 144996 merged by jenkins-bot:
Remove support for magic_quotes_gpc

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