WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#31306 closed defect (bug) (fixed)

add_query_arg changes numerical arguments

Reported by: Lex_Robinson Owned by: wonderboymusic
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.1
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

I noticed this when I used '1=1' as a default query value to prevent a ? being prepended. If an argument passed to add_query_arg is a number, it gets replaced with an incrementing value. It doesn't re-order arguments or parse non-decimal numbers.
I suspect this is being caused by array_merge on line 782 of wp-includes/functions.php

var_dump( add_query_arg( array( 'foo' => 'bar' ), '1=1' ) );
// string '0=1&foo=bar' (length=11)

var_dump( add_query_arg( array( 'a' => 'c' ), '1=1&a=b&10=10' ) );
// string '0=1&a=c&1=10' (length=12)

var_dump( add_query_arg( array( '19' => '19' ), '20=20' ) );
// string '0=20&1=19' (length=9)

var_dump( add_query_arg( array( '3e1' => '3' ), '0x1=1&a=b&010=2' ) );
// string '0x1=1&a=b&010=2&3e1=3' (length=21)

Attachments (1)

31306.patch (1.2 KB) - added by tyxla 6 years ago.
Fixing the disappearance of numeric keys in add_query_arg(). Including a unit test.

Download all attachments as: .zip

Change History (11)

#1 @Lex_Robinson
6 years ago

  • Severity changed from normal to minor

#2 @tyxla
6 years ago

I was able to reproduce this issue, too.

And @Lex_Robinson, yes, your assumption is correct here - array_merge() flattens the integer indexes. Replacing that array_merge() with array_replace() will do the job, overriding what's necessary from the old query vars, but preserving the query vars with numeric keys.

Patch coming up shortly.

@tyxla
6 years ago

Fixing the disappearance of numeric keys in add_query_arg(). Including a unit test.

#3 @tyxla
6 years ago

  • Keywords has-patch added

#4 @DrewAPicture
6 years ago

  • Milestone changed from Awaiting Review to 4.2
  • Severity changed from minor to normal

This ticket was mentioned in Slack in #core by drew. View the logs.


6 years ago

#6 @DrewAPicture
6 years ago

  • Keywords commit added

31306.patch still applies, unit tests pass. Moving for commit consideration.

#7 @mordauk
6 years ago

Patch applies cleanly and tests pass for me.

#8 @wonderboymusic
6 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 31966:

Respect numerical keys in add_query_arg(), use array_replace() instead of array_merge().

Adds unit test.

Props tyxla.
Fixes #31306.

#9 @boonebgorges
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

array_replace() is PHP 5.3+.

#10 @boonebgorges
6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 31967:

Avoid the use of array_replace() in add_query_arg().

array_replace() was introduced PHP 5.3+. Instead, we walk the array manually.

See [31966].

Fixes #31306.

Note: See TracTickets for help on using tickets.