[wp-trac] [WordPress Trac] #17923: add_query_arg() should encode values

[WordPress Trac]

#17923: add_query_arg() should encode values
-----------------------------------+-----------------------------
 Reporter:  Viper007Bond           |       Owner:
     Type:  defect (bug)           |      Status:  new
 Priority:  normal                 |   Milestone:  Future Release
Component:  General                |     Version:  3.2
 Severity:  normal                 |  Resolution:
 Keywords:  needs-patch has-patch  |
-----------------------------------+-----------------------------

Comment (by kurtpayne):

 Replying to [comment:11 Viper007Bond]:
 > Just noticed something else -- why the change in methods between the
 array vs strings?
 >
 > For arrays, you leave it where the value gets assigned to a variable and
 then check to see if encoding is enabled and if so, you then encode the
 value.
 >
 > However for strings, you do both at once.
 >
 > In short, wouldn't this be better?
 >
 > (first line already exists in core, second and third lines would be new)
 >
 > {{{
 > $qs[func_get_arg( 0 )] = func_get_arg( 1 );
 > if ( true === @func_get_arg( 3 ) )
 >       $qs[func_get_arg( 0 )] = rawurlencode( $qs[func_get_arg( 0 )] );
 > }}}
 >
 > Wow, I am sure being nitpicky today. Sorry. lol

 Very valid question, actually.  The function should only encode ''new''
 arguments.  Anything that's currently in the URL may already be encoded
 and should be left alone.  If the `$qs` array is encoded after it's split,
 things may be re-encoded and yield funny results.  Example:

 {{{
 $url = '/wordpress/';
 $url = add_query_arg('a', 'b&c', $url, true);  # /wordpress/?a=b%26c
 $url = add_query_arg('a', 'b&c', $url, true);  # /wordpress/?a=b%2526c
 }}}

 As you may suspect, I found this out the hard way.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/17923#comment:12>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software