Last modified: 2008-02-12 22:11:26 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.
Currently WebRequest uses $_REQUEST as its data source, which contains both $_GET and $_POST data for convenience.
But it also contains $_COOKIE data, which may lead to unexpected behavior. If another tool on the site sets a cookie whose name conflicts with one of MediaWiki's expected input parameters, Weird Things can happen as "phantom" input keeps showing up.
Polling $_GET and $_POST in order instead of $_REQUEST would avoid this.
Created attachment 4209 [details]
Ok, here's a patch that makes #wgRequest to use only $_GET and $_POST, not $_COOKIES and $_FILES
Created attachment 4210 [details]
Ok, another more reviewed patch
My one concern with this is that it'll be re-generating the merged array consisting of *all* input on every access for *any* input.
When dealing with large form submissions that doesn't sound very efficient, and could bloat our already-obscene memory usage. (Might be worth checking if that's actually so, though.)
Might be cleaner to do this as a fallback: have a list of data sources and check each source in turn.
(In reply to comment #3)
> My one concern with this is that it'll be re-generating the merged array
> consisting of *all* input on every access for *any* input.
You could potentially do some lazy-initialisation to counter this.
On some really unscientific testing, it looks like the array_merge() won't significantly boost memory usage (the strings are apparently refcounted and not duplicated as long as they're not changed, yay?).
The preemptive stripslashes()ing (bug 11558), though, *does* increase memory usage throughout the lifetime of the script -- it looks like both the original strings and the stripped strings sit in memory until the script dies, whereas an on-demand stripped string can be freed once it's out of scope.
Went ahead and swapped out the $_REQUESTs for a merged array of $_GET and $_POST in r30882.
The merged array is being initialized in the constructor, right after magic quotes removal. Per my notes in above comment, this shouldn't alter memory usage much at present.
Over the previous patch, this fixes some other cases by ensuring all operations against $_REQUEST use the same data set.
Bug 7681 was an example of the cookie processing in $_REQUEST causing unexpected results. Now happily fixed. :)