WordPress.org
Get WordPress

Make WordPress Core

Changeset 44546


Ignore:
Timestamp:
01/10/2019 09:05:50 PM (8 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.