WordPress.org
Get WordPress

Make WordPress Core

Changeset 44546


Ignore:
Timestamp:
01/10/2019 09:05:50 PM (6 years ago)
Author:
flixos90
Message:

General: Fix problematic string to array parsing.

WordPress has historically often used code like preg_split( '/[\s,]+/', $var ) to parse a string of comma-separated values into an array. However, this approach was causing an empty string to not be parsed into an empty array as expected, but rather into an array with the empty string as its sole element.

This was among other areas causing problems in the REST API where passing an empty request parameter could cause that request to fail because, instead of it being ignored, that parameter would be compared against the valid values for it, which typically do not include an empty string.

Props david.binda, sstoqnov.
Fixes #43977.

Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/wp-includes/bookmark.php

    r42343 r44546  
    172172        $r['category']      = '';
    173173        $r['category_name'] = '';
    174         $inclinks           = preg_split( '/[\s,]+/', $r['include'] );
     174        $inclinks           = wp_parse_id_list( $r['include'] );
    175175        if ( count( $inclinks ) ) {
    176176            foreach ( $inclinks as $inclink ) {
    177177                if ( empty( $inclusions ) ) {
    178                     $inclusions = ' AND ( link_id = ' . intval( $inclink ) . ' ';
     178                    $inclusions = ' AND ( link_id = ' . $inclink . ' ';
    179179                } else {
    180                     $inclusions .= ' OR link_id = ' . intval( $inclink ) . ' ';
     180                    $inclusions .= ' OR link_id = ' . $inclink . ' ';
    181181                }
    182182            }
    … …  
    189189    $exclusions = '';
    190190    if ( ! empty( $r['exclude'] ) ) {
    191         $exlinks = preg_split( '/[\s,]+/', $r['exclude'] );
     191        $exlinks = wp_parse_id_list( $r['exclude'] );
    192192        if ( count( $exlinks ) ) {
    193193            foreach ( $exlinks as $exlink ) {
    194194                if ( empty( $exclusions ) ) {
    195                     $exclusions = ' AND ( link_id <> ' . intval( $exlink ) . ' ';
     195                    $exclusions = ' AND ( link_id <> ' . $exlink . ' ';
    196196                } else {
    197                     $exclusions .= ' AND link_id <> ' . intval( $exlink ) . ' ';
     197                    $exclusions .= ' AND link_id <> ' . $exlink . ' ';
    198198                }
    199199            }
    … …  
    224224    $join           = '';
    225225    if ( ! empty( $r['category'] ) ) {
    226         $incategories = preg_split( '/[\s,]+/', $r['category'] );
     226        $incategories = wp_parse_id_list( $r['category'] );
    227227        if ( count( $incategories ) ) {
    228228            foreach ( $incategories as $incat ) {
    229229                if ( empty( $category_query ) ) {
    230                     $category_query = ' AND ( tt.term_id = ' . intval( $incat ) . ' ';
     230                    $category_query = ' AND ( tt.term_id = ' . $incat . ' ';
    231231                } else {
    232                     $category_query .= ' OR tt.term_id = ' . intval( $incat ) . ' ';
     232                    $category_query .= ' OR tt.term_id = ' . $incat . ' ';
    233233                }
    234234            }

  • trunk/src/wp-includes/class-wp-comment-query.php

    r44412 r44546  
    483483        // 'status' accepts an array or a comma-separated string.
    484484        $status_clauses = array();
    485         $statuses       = $this->query_vars['status'];
    486         if ( ! is_array( $statuses ) ) {
    487             $statuses = preg_split( '/[\s,]+/', $statuses );
     485        $statuses       = wp_parse_list( $this->query_vars['status'] );
     486
     487        // Empty 'status' should be interpreted as 'all'.
     488        if ( empty( $statuses ) ) {
     489            $statuses = array( 'all' );
    488490        }
    489491
    … …  
    518520        // User IDs or emails whose unapproved comments are included, regardless of $status.
    519521        if ( ! empty( $this->query_vars['include_unapproved'] ) ) {
    520             $include_unapproved = $this->query_vars['include_unapproved'];
    521 
    522             // Accepts arrays or comma-separated strings.
    523             if ( ! is_array( $include_unapproved ) ) {
    524                 $include_unapproved = preg_split( '/[\s,]+/', $include_unapproved );
    525             }
    526 
    527             $unapproved_ids = $unapproved_emails = array();
     522            $include_unapproved = wp_parse_list( $this->query_vars['include_unapproved'] );
     523
     524            $unapproved_ids    = array();
     525            $unapproved_emails = array();
    528526            foreach ( $include_unapproved as $unapproved_identifier ) {
    529527                // Numeric values are assumed to be user ids.

  • trunk/src/wp-includes/functions.php

    r44506 r44546  
    38243824
    38253825/**
     3826 * Cleans up an array, comma- or space-separated list of scalar values.
     3827 *
     3828 * @since 5.1.0
     3829 *
     3830 * @param array|string $list List of values.
     3831 * @return array Sanitized array of values.
     3832 */
     3833function wp_parse_list( $list ) {
     3834    if ( ! is_array( $list ) ) {
     3835        return preg_split( '/[\s,]+/', $list, -1, PREG_SPLIT_NO_EMPTY );
     3836    }
     3837
     3838    return $list;
     3839}
     3840
     3841/**
    38263842 * Clean up an array, comma- or space-separated list of IDs.
    38273843 *
    … …  
    38323848 */
    38333849function wp_parse_id_list( $list ) {
    3834     if ( ! is_array( $list ) ) {
    3835         $list = preg_split( '/[\s,]+/', $list );
    3836     }
     3850    $list = wp_parse_list( $list );
    38373851
    38383852    return array_unique( array_map( 'absint', $list ) );
    … …  
    38483862 */
    38493863function wp_parse_slug_list( $list ) {
    3850     if ( ! is_array( $list ) ) {
    3851         $list = preg_split( '/[\s,]+/', $list );
    3852     }
    3853 
    3854     foreach ( $list as $key => $value ) {
    3855         $list[ $key ] = sanitize_title( $value );
    3856     }
    3857 
    3858     return array_unique( $list );
     3864    $list = wp_parse_list( $list );
     3865
     3866    return array_unique( array_map( 'sanitize_title', $list ) );
    38593867}
    38603868

  • trunk/src/wp-includes/post.php

    r44454 r44546  
    50895089    $author_query = '';
    50905090    if ( ! empty( $r['authors'] ) ) {
    5091         $post_authors = preg_split( '/[\s,]+/', $r['authors'] );
     5091        $post_authors = wp_parse_list( $r['authors'] );
    50925092
    50935093        if ( ! empty( $post_authors ) ) {

  • trunk/src/wp-includes/rest-api.php

    r44173 r44546  
    680680    $data = $response->get_data();
    681681
    682     $fields = is_array( $request['_fields'] ) ? $request['_fields'] : preg_split( '/[\s,]+/', $request['_fields'] );
     682    $fields = wp_parse_list( $request['_fields'] );
    683683
    684684    if ( 0 === count( $fields ) ) {
    … …  
    11101110function rest_validate_value_from_schema( $value, $args, $param = '' ) {
    11111111    if ( 'array' === $args['type'] ) {
    1112         if ( ! is_array( $value ) ) {
    1113             $value = preg_split( '/[\s,]+/', $value );
     1112        if ( ! is_null( $value ) ) {
     1113            $value = wp_parse_list( $value );
    11141114        }
    11151115        if ( ! wp_is_numeric_array( $value ) ) {
    … …  
    12541254            return (array) $value;
    12551255        }
    1256         if ( ! is_array( $value ) ) {
    1257             $value = preg_split( '/[\s,]+/', $value );
    1258         }
     1256        $value = wp_parse_list( $value );
    12591257        foreach ( $value as $index => $v ) {
    12601258            $value[ $index ] = rest_sanitize_value_from_schema( $v, $args['items'] );

  • trunk/src/wp-includes/rest-api/endpoints/class-wp-rest-controller.php

    r44254 r44546  
    533533            return $fields;
    534534        }
    535         $requested_fields = is_array( $request['_fields'] ) ? $request['_fields'] : preg_split( '/[\s,]+/', $request['_fields'] );
     535        $requested_fields = wp_parse_list( $request['_fields'] );
    536536        if ( 0 === count( $requested_fields ) ) {
    537537            return $fields;

  • trunk/tests/phpunit/tests/bookmark/getBookmarks.php

    r42343 r44546  
    8585        $this->assertTrue( $num_queries < $wpdb->num_queries );
    8686    }
     87
     88    public function test_exclude_param_gets_properly_parsed_as_list() {
     89        $bookmarks = self::factory()->bookmark->create_many( 3 );
     90
     91        $found = get_bookmarks(
     92            array(
     93                'exclude' => ',,',
     94            )
     95        );
     96        $found_ids = array();
     97        foreach( $found as $bookmark ) {
     98            $found_ids[] = $bookmark->link_id;
     99        }
     100
     101        // equal sets != same order.
     102        $this->assertEqualSets( $bookmarks, $found_ids );
     103    }
     104
     105    public function test_include_param_gets_properly_parsed_as_list() {
     106        $bookmarks = self::factory()->bookmark->create_many( 3 );
     107
     108        $found = get_bookmarks(
     109            array(
     110                'include' => ',,',
     111            )
     112        );
     113        $found_ids = array();
     114        foreach( $found as $bookmark ) {
     115            $found_ids[] = $bookmark->link_id;
     116        }
     117
     118        // equal sets != same order.
     119        $this->assertEqualSets( $bookmarks, $found_ids );
     120    }
     121
     122    public function test_category_param_propelry_gets_parsed_as_list() {
     123        $bookmarks = self::factory()->bookmark->create_many( 3 );
     124        $categories = self::factory()->term->create_many( 3, array(
     125            'taxonomy' => 'link_category',
     126        ) );
     127        $add = wp_add_object_terms( $bookmarks[0], $categories[0], 'link_category' );
     128        $add = wp_add_object_terms( $bookmarks[1], $categories[1], 'link_category' );
     129        $add = wp_add_object_terms( $bookmarks[2], $categories[2], 'link_category' );
     130
     131        $found = get_bookmarks(
     132            array(
     133                'category' => ',,',
     134            )
     135        );
     136        $found_ids = array();
     137        foreach( $found as $bookmark ) {
     138            $found_ids[] = $bookmark->link_id;
     139        }
     140
     141        // equal sets != same order.
     142        $this->assertEqualSets( $bookmarks, $found_ids );
     143    }
    87144}

  • trunk/tests/phpunit/tests/functions.php

    r44481 r44546  
    533533
    534534        update_option( 'blog_charset', $orig_blog_charset );
     535    }
     536
     537    /**
     538     * @ticket 43977
     539     * @dataProvider data_wp_parse_list
     540     */
     541    function test_wp_parse_list( $expected, $actual ) {
     542        $this->assertSame( $expected, array_values( wp_parse_list( $actual ) ) );
     543    }
     544
     545    function data_wp_parse_list() {
     546        return array(
     547            array( array( '1', '2', '3', '4' ), '1,2,3,4' ),
     548            array( array( 'apple', 'banana', 'carrot', 'dog' ), 'apple,banana,carrot,dog' ),
     549            array( array( '1', '2', 'apple', 'banana' ), '1,2,apple,banana' ),
     550            array( array( '1', '2', 'apple', 'banana' ), '1, 2,apple,banana' ),
     551            array( array( '1', '2', 'apple', 'banana' ), '1,2,apple,,banana' ),
     552            array( array( '1', '2', 'apple', 'banana' ), ',1,2,apple,banana' ),
     553            array( array( '1', '2', 'apple', 'banana' ), '1,2,apple,banana,' ),
     554            array( array( '1', '2', 'apple', 'banana' ), '1,2 ,apple,banana' ),
     555            array( array(), '' ),
     556            array( array(), ',' ),
     557            array( array(), ',,' ),
     558        );
    535559    }
    536560

  • trunk/tests/phpunit/tests/rest-api/rest-controller.php

    r43986 r44546  
    204204    }
    205205
    206     public function test_get_fields_for_response() {
     206    /**
     207     * @dataProvider data_get_fields_for_response,
     208     */
     209    public function test_get_fields_for_response( $param, $expected ) {
    207210        $controller = new WP_REST_Test_Controller();
    208211        $request    = new WP_REST_Request( 'GET', '/wp/v2/testroute' );
    … …  
    222225            $fields
    223226        );
    224         $request->set_param( '_fields', 'somestring,someinteger' );
     227        $request->set_param( '_fields', $param );
    225228        $fields = $controller->get_fields_for_response( $request );
    226         $this->assertEquals(
    227             array(
    228                 'somestring',
    229                 'someinteger',
     229        $this->assertEquals( $expected, $fields );
     230    }
     231
     232    public function data_get_fields_for_response() {
     233        return array(
     234            array(
     235                'somestring,someinteger',
     236                array(
     237                    'somestring',
     238                    'someinteger',
     239                ),
    230240            ),
    231             $fields
     241            array(
     242                ',,',
     243                array(
     244                    'somestring',
     245                    'someinteger',
     246                    'someboolean',
     247                    'someurl',
     248                    'somedate',
     249                    'someemail',
     250                    'someenum',
     251                    'someargoptions',
     252                    'somedefault',
     253                ),
     254            ),
    232255        );
    233256    }

  • trunk/tests/phpunit/tests/rest-api/rest-schema-sanitization.php

    r43571 r44546  
    111111        $this->assertEquals( array( 1, 2 ), rest_sanitize_value_from_schema( '1,2', $schema ) );
    112112        $this->assertEquals( array( 1, 2, 0 ), rest_sanitize_value_from_schema( '1,2,a', $schema ) );
     113        $this->assertEquals( array( 1, 2 ), rest_sanitize_value_from_schema( '1,2,', $schema ) );
    113114    }
    114115
    … …  
    135136        $this->assertEquals( array( 'ribs', 'chicken' ), rest_sanitize_value_from_schema( 'ribs,chicken', $schema ) );
    136137        $this->assertEquals( array( 'chicken', 'coleslaw' ), rest_sanitize_value_from_schema( 'chicken,coleslaw', $schema ) );
     138        $this->assertEquals( array( 'chicken', 'coleslaw' ), rest_sanitize_value_from_schema( 'chicken,coleslaw,', $schema ) );
    137139    }
    138140

  • trunk/tests/phpunit/tests/rest-api/rest-schema-validation.php

    r43571 r44546  
    121121        $this->assertTrue( rest_validate_value_from_schema( array( 1 ), $schema ) );
    122122        $this->assertWPError( rest_validate_value_from_schema( array( true ), $schema ) );
     123        $this->assertWPError( rest_validate_value_from_schema( null, $schema ) );
    123124    }
    124125
    … …  
    146147        $this->assertTrue( rest_validate_value_from_schema( '1,2,3', $schema ) );
    147148        $this->assertWPError( rest_validate_value_from_schema( 'lol', $schema ) );
     149        $this->assertTrue( rest_validate_value_from_schema( '1,,', $schema ) );
     150        $this->assertTrue( rest_validate_value_from_schema( '', $schema ) );
    148151    }
    149152
    … …  
    170173        $this->assertTrue( rest_validate_value_from_schema( 'ribs,chicken', $schema ) );
    171174        $this->assertWPError( rest_validate_value_from_schema( 'chicken,coleslaw', $schema ) );
     175        $this->assertTrue( rest_validate_value_from_schema( 'ribs,chicken,', $schema ) );
     176        $this->assertTrue( rest_validate_value_from_schema( '', $schema ) );
    172177    }
    173178
Note: See TracChangeset for help on using the changeset viewer.