Make WordPress Core

Opened 2 years ago

Closed 20 months ago

Last modified 20 months ago

#55838 closed defect (bug) (fixed)

Warning: strip_tags() expects parameter 1 to be string, array given in wp-includes/formatting.php:2246

Reported by: dd32's profile dd32 Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Posts, Post Types Keywords: has-patch commit
Focuses: rest-api Cc:

Description

E_WARNING: strip_tags() expects parameter 1 to be string, array given in wp-includes/formatting.php:2246

This is a slightly funky one, and is triggered by a request similar to:

https://example.org/wp-json/wp/v2/posts?slug[0][1]=2
https://example.org/wp-json/wp/v2/posts?status[0][1]=2

The Posts endpoint allows for status/slug to be either an array, or a string (optionally comma separated).

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php?marks=2849-2853,2859-2864#L2842

Unfortunately it does't enforce that to be an array of strings, allowing an array of arrays to pass through.

It seems that the best place to fix this is in wp_parse_slug_list(), although perhaps an argument could be made for altering wp_parse_list() too.

wp_parse_list() simply leaves arrays as they are, not caring about the contents of the array.. It seems that leaving that as-is is okay.

Simply updating wp_parse_slug_list() to also filter for scalars results in the same outcome, and is much safer from a back-compat perspective.

  • src/wp-includes/functions.php

     
    48454845 */
    48464846function wp_parse_slug_list( $list ) {
    48474847        $list = wp_parse_list( $list );
     4848        $list = array_filter( $list, 'is_scalar' );
    48484849
    48494850        return array_unique( array_map( 'sanitize_title', $list ) );
    48504851}

Change History (7)

#1 @SergeyBiryukov
2 years ago

  • Milestone changed from Awaiting Review to 6.1

#2 @TimothyBlynJacobs
2 years ago

Updating wp_parse_slug_list makes the most sense to me.

Additionally, we should also drop the sanitize_callback that is included for the slug query parameter. The REST API schema validation will properly validate the value as an array of strings and allow the same behavior of including a comma-separated list.

This ticket was mentioned in PR #3288 on WordPress/wordpress-develop by dd32.


21 months ago
#3

This contains two changes:

  1. Removal of sanitize_callback to allow the REST API to handle the validation natively, this also causes the proper error to be output for ?slug[0][1]=2 that it's an invalid value.
  2. Ensure that wp_parse_list() only returns a single-dimensioned array, even if passed a multi-dimension array, which fits the functions expected use-case and resolves warnings in code that expects the function to return a single-dimensioned array.

Part 2 is potential for back-compat issues, as although it's not documented as such, any broken code that expects to pass wp_parse_list( [ [ 'key' => 'val' ] ] ) and have the same output as input will break.. IMHO this is not the intention of the function, and breakage should be expected.
If however, that potential back-compat is a potential issue, that line could instead be moved into wp_parse_id_list() and wp_parse_slug_list() instead.

Trac ticket: https://core.trac.wordpress.org/ticket/55838

dd32 commented on PR #3288:


21 months ago
#4

Example error in the case of scenario 1 above:
https://i0.wp.com/user-images.githubusercontent.com/767313/191193384-82cf7c13-c89a-419f-8ea0-937fc3a1d6e7.png

#5 @audrasjb
21 months ago

  • Keywords commit added
  • Owner set to audrasjb
  • Status changed from new to assigned

I tested the PR using the following endpoint: /wp-json/wp/v2/posts?slug[0][1]=2

Before patch:

<br />
<b>Warning</b>:  preg_match() expects parameter 2 to be string, array given in <b>/wp-includes/formatting.php</b> on line <b>1596</b><br />
<br />
<b>Warning</b>:  strip_tags() expects parameter 1 to be string, array given in <b>/wp-includes/formatting.php</b> on line <b>2256</b><br />
[]

After patch:

{"code":"rest_invalid_param","message":"Invalid parameter(s): slug","data":{"status":400,"params":{"slug":"slug[0] is not of type string."},"details":{"slug":{"code":"rest_invalid_type","message":"slug[0] is not of type string.","data":{"param":"slug[0]"}}}}}

Looks like it fixes the issue.
Self assigning for commit.

#6 @audrasjb
20 months ago

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

In 54476:

Posts, Post Types: Ensure all entries in the list returned by wp_parse_list() are scalar.

This changeset fixes a warning where strip_tags() expected its first parameter to be a string rather than an array. It contains the following changes:

  • Removal of sanitize_callback to allow the REST API to handle the validation natively, this also causes the proper error to be output for ?slug[0][1]=2 that it's an invalid value.
  • Ensure that wp_parse_list() only returns a single-dimensioned array, even if passed a multi-dimension array, which fits the functions expected use case and resolves warnings in code that expects the function to return a single-dimensioned array.

Props dd32, TimothyBlynJacobs.
Fixes #55838.

Note: See TracTickets for help on using tickets.