Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#45448 closed defect (bug) (fixed)

rest_sanitize_value_from_schema() returns invalid value when using a schema of type array with a null value

Reported by: hogash's profile hogash Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.9.8
Component: REST API Keywords: has-patch needs-unit-tests
Focuses: rest-api Cc:

Description

When using the following configuration, you will get an array containing 1 empty string instead of an empty array;

$schema = array(
	'type'  => 'array',
	'items' => [
		'type' => 'string',
	],
);

$value = rest_sanitize_value_from_schema( null, $schema );
var_dump( $value );

The solution for this would be to modify the following line in rest_sanitize_value_from_schema():

$value = preg_split( '/[\s,]+/', $value );

with

$value = preg_split( '/[\s,]+/', $value, null, PREG_SPLIT_NO_EMPTY );

Attachments (2)

45448.diff (685 bytes) - added by wpboss 5 years ago.
I have fixed this and test it.
45448-1.diff (683 bytes) - added by wpboss 5 years ago.
I have fixed as per your suggestion.

Download all attachments as: .zip

Change History (10)

#1 @mukesh27
5 years ago

  • Focuses rest-api added

@wpboss
5 years ago

I have fixed this and test it.

#2 @wpboss
5 years ago

  • Keywords has-patch added

#3 @birgire
5 years ago

@wpboss I had a look at preg_split() and stumbled upon an interesting comment here:

https://secure.php.net/manual/en/function.preg-split.php#121475

that null input for the limit will generate an error in strict_type mode in PHP 7.1+

I could verify that with this snippet:

<?php
declare(strict_types=1);
$value = null;
$value = preg_split( '/[\s,]+/', $value, null, PREG_SPLIT_NO_EMPTY );
var_dump( $value );

will generate an fatal error for PHP 7.0+ according to:

https://3v4l.org/Urmrp

So instead of:

$value = preg_split( '/[\s,]+/', $value, null, PREG_SPLIT_NO_EMPTY );

it looks like one could consider:

$value = preg_split( '/[\s,]+/', $value, -1, PREG_SPLIT_NO_EMPTY );

as already used elsewhere in core.

I have fixed this and test it.

@wpboss I guess you mean manually tested it. It would also be interesting to check if the current unit tests for rest_sanitize_value_from_schema() and the restapi group will run successfully.

Version 2, edited 5 years ago by birgire (previous) (next) (diff)

@wpboss
5 years ago

I have fixed as per your suggestion.

#4 @birgire
5 years ago

Thanks for the update @wpboss

ps: I created a new ticket #45486 for the correction of the inline documentation of rest_sanitize_value_from_schema() that I noticed here.

#5 @SergeyBiryukov
5 years ago

  • Keywords needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.1
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

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


5 years ago

#7 @earnjam
5 years ago

This would also be resolved by #43977

#8 @earnjam
5 years ago

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

This was fixed in [44546].

Note: See TracTickets for help on using tickets.