Our wfArrayToCGI and wfCgiToArray conversion functions (they transform data between 'foo=bar&hello=world' and array( 'foo' => 'bar', 'hello' => 'world' ) formats) seam to be fairly messed up.
In a discussion over r104518 on the way it did query parameters to pass to getLocalURL I noticed our lack of sane null handling in wfArrayToCGI. However after examining it even more I found that the output of this area of code seams completely messed up.
Some examples of how these behave:
echo wfArrayToCGI(array('foo' => 'bar', 'baz' => '', 'asdf' => null, 'qwerty' => false));
foo=bar&asdf=&qwerty= # In array -> cgi conversion we will omit a key for an empty string, but yield an empty value for null and false # Frankly it should be the other way around. An empty string is reasonable input to expect an empty but present cgi key.
# By the way, this is our treatment of an empty value in our method that goes in the opposite direction
var_dump(wfCgiToArray('foo=bar&asdf='));
array(2) { ["foo"]=> string(3) "bar" ["asdf"]=> string(0) "" }
# Naturally you can expect how a round trip is screwed up
var_dump(wfArrayToCGI(wfCgiToArray('foo=bar&asdf=')));
string(7) "foo=bar" # Because we treat an empty string as an omission instead of a value the round trip omits something we had in the first place
# cgi to array conversion could use some proper handling of an edge case too
var_dump(wfCgiToArray('foo=bar&asdf=&qwerty'));
PHP Notice: Undefined offset: 1 in /Users/daniel/Workspace/mediawiki/trunk/phase3/includes/GlobalFunctions.php on line 388 array(3) { ["foo"]=> string(3) "bar" ["asdf"]=> string(0) "" ["qwerty"]=> string(0) "" } # Personally I think that 'foo=bar&asdf=&qwerty' should yield an array like # array( 'foo' => 'bar', 'asdf' => '', 'qwerty );
Does anyone think anything would break if I re-coded these two deep core methods to work in a seemingly more sane way.
[r104518] https://www.mediawiki.org/wiki/Special:Code/MediaWiki/104518