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 | 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
@
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).
#2
@
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.
#3
@
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.
#4
@
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
.
Add unit tests for the current situation