Make WordPress Core

Opened 6 years ago

Last modified 6 years ago

#44212 new defect (bug)

Add a new helper function for comma- or space-separated lists

Reported by: keesiemeijer's profile keesiemeijer Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

The functions wp_parse_id_list() and wp_parse_slug_list() both use preg_split() to convert strings to an array. Strings with leading or trailing spaces will result in an added empty value '' or 0 returned by these functions.

Example

// leading space
$ids = wp_parse_id_list( ' 1,2,3' );

// result

array(
    0 => 0
    1 => 1
    2 => 2
    3 => 3
)

This is because the PREG_SPLIT_NO_EMPTY flag is not used withpreg_split(). The same regex '/[\s,]+/' used by these functions is also used in other places in WordPress core (also without using the flag). That's why I propose a new helper function for comma- or space-separated lists (that uses the flag by default).

That function could then be used by wp_parse_id_list() and wp_parse_slug_list() and all other instances where this regex is used.

Attachments (2)

44212.patch (1.0 KB) - added by keesiemeijer 6 years ago.
Add unit tests for the current situation
44212.2.patch (3.3 KB) - added by keesiemeijer 6 years ago.
Add new function wp_parse_list()

Download all attachments as: .zip

Change History (6)

@keesiemeijer
6 years ago

Add unit tests for the current situation

@keesiemeijer
6 years ago

Add new function wp_parse_list()

#1 @keesiemeijer
6 years ago

In 44212.2.patch the function wp_parse_list() is introduced and is used by wp_parse_id_list() and wp_parse_slug_list().

With this function it now removes the empty values returned by preg_split(). You can also remove all the values that equal to false by setting the $format parameter to filter_false. We could use this filter for wp_parse_id_list() as IDs are not supposed to have a value of 0. I've left it as is for back compatibility.

Empty strings

There are differences between what is returned for preg_split() (without the PREG_SPLIT_NO_EMPTY) flag and the wp_parse_id_list() function if you use it with an empty string

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

// non empty array
array(
    0 => ''
)

$array = wp_parse_list('');

// empty array
array(

)

New tickets should be created for the other instances where preg_split() is used (with the same regex) that expect an empty array (and are now iterating over it).

Last edited 6 years ago by keesiemeijer (previous) (diff)

#2 @keesiemeijer
6 years ago

  • Type changed from enhancement to defect (bug)

I'm changing this from enhancement to defect as the wp_parse_id_list() and wp_parse_slug_list() are not supposed to return non empty arrays for empty strings.

Last edited 6 years ago by keesiemeijer (previous) (diff)

#3 @subrataemfluence
6 years ago

Curious to know what if we just wrap the $list within a trim() function to clean it up frm any leading or trailing spaces before using preg_split on to it in the core functions.

Like:

<?php
function wp_parse_id_list( $list ) {
   if ( ! is_array( $list ) ) {
      $list = preg_split( '/[\s,]+/', trim( $list ) );
   }

   return array_unique( array_map( 'absint', $list ) );
}

and

<?php
function wp_parse_slug_list( $list ) {
   if ( ! is_array( $list ) ) {
      $list = preg_split( '/[\s,]+/', trim( $list ) );
   }

   foreach ( $list as $key => $value ) {
      $list[ $key ] = sanitize_title( $value );
   }

   return array_unique( $list );
}

Do you think if could create any impact in other areas? Please correct me if I am wrong.

Last edited 6 years ago by subrataemfluence (previous) (diff)

#4 @keesiemeijer
6 years ago

Yes, it could be used with trim(), but you still have to use the PREG_SPLIT_NO_EMPTY flag otherwise it returns a non empty array for empty strings.

<?php
$list = preg_split( '/[\s,]+/', trim( $list ), -1, PREG_SPLIT_NO_EMPTY );

The wp_parse_list() function is there so you can get values from comma- or space-separated lists without the sanitation from wp_parse_slug_list() or wp_parse_id_list.

Last edited 6 years ago by keesiemeijer (previous) (diff)
Note: See TracTickets for help on using tickets.