Opened 7 years ago
Last modified 7 years ago
#44212 new defect (bug)
Add a new helper function for comma- or space-separated lists
| Reported by: |
|
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)
Change History (6)
#1
@
7 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).
#2
@
7 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.
#3
@
7 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.
#4
@
7 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.
Add unit tests for the current situation