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
-- 
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [http://daniel.friesen.name]

_______________________________________________
Wikitech-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to