WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#38306 closed defect (bug) (fixed)

WP_REST_Request->get_params strips numeric keys

Reported by: sswells Owned by: joehoyle
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Right now, a numeric key is getting stripped.

Request Body = {"41150":"API value"}

$request->get_params = [1] => API value

So line 453 in class-wp-rest-request.php:
$params = array_merge( $params, (array) $this->params[ $type ] );

should be:
$params = $params + (array) $this->params[ $type ];

Any chance of getting this into 4.7?

Attachments (1)

38306.diff (1.4 KB) - added by joehoyle 4 years ago.

Download all attachments as: .zip

Change History (12)

This ticket was mentioned in Slack in #core-restapi by rachelbaker. View the logs.


4 years ago

#2 @rachelbaker
4 years ago

@sswells We cannot use the combine operator for the associative arrays because it does not merge arrays recursively. See https://3v4l.org/ultWd

It looks like the only way to patch this correctly needs to allow numeric array keys to be retained without being re-indexed, while still merging recursive data as well.

#3 @sswells
4 years ago

Thanks. I didn't think about that since my test data was flat. Looks like my fix won't cover it then.

#4 @rachelbaker
4 years ago

  • Version changed from 4.6.1 to 4.4

#5 @joehoyle
4 years ago

  • Milestone changed from Awaiting Review to 4.7

#6 @rachelbaker
4 years ago

@joehoyle Saw you milestoned this as 4.7. Are you going to shepherd this ticket before beta?

#7 @joehoyle
4 years ago

  • Owner set to joehoyle
  • Status changed from new to assigned

@rachelbaker yes! Set myself as owner.

#8 @rachelbaker
4 years ago

While discussing this ticket with @joehoyle he pointed out that @sswells was using a numeric string in her array key, and that would result in the + operator recursively combining the arrays: https://3v4l.org/qdhJC

@joehoyle
4 years ago

#9 @joehoyle
4 years ago

  • Keywords has-patch has-unit-tests added

array_merge and the + operator are both issues, as even the + will make numeric keys integers after it's done. It seems the easiest and best way is to manually implement a foreach. Added unit test also.

#10 @rmccue
4 years ago

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

In 39087:

REST API: Change method of merging parameters.

array_merge() incorrectly reindexes numeric parameters, causing things like {"123": true} to be "dropped".

Props sswells, joehoyle.
Fixes #38306.

This ticket was mentioned in Slack in #core-restapi by rmccue. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.