[wp-trac] [WordPress Trac] #54160: sanitize_key() / _wp_customize_include() is not able to handle non-scalar values

WordPress Trac noreply at wordpress.org
Fri Dec 10 23:18:23 UTC 2021


#54160: sanitize_key() / _wp_customize_include() is not able to handle non-scalar
values
--------------------------+-----------------------------
 Reporter:  dd32          |       Owner:  hellofromTonya
     Type:  defect (bug)  |      Status:  reopened
 Priority:  normal        |   Milestone:  5.9
Component:  Formatting    |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:
--------------------------+-----------------------------
Changes (by hellofromTonya):

 * keywords:  has-patch needs-unit-tests => needs-patch
 * milestone:  6.0 => 5.9


Comment:

 Capturing notes:

 PHP is getting more and more strict with documented data types being
 passed into native PHP functions where deprecation notices are thrown in
 PHP 8.1, but will be fatal errors in PHP 9. However, currently
 `strtolower()` still handles scalars, though it's documented as only
 accepting strings. It's probable that a PHP 8.x version will add a
 deprecation to fix the bug internal to `strtolower()`. This is not the
 case today or in WordPress 5.9.

 A choice needs to be made. Deprecate non-strings scalars now, or allow
 them until PHP deprecates it in `strtolower()`.

 `sanitize_key()` is clearly identified to only work with string keys and
 has been since it was introduced. However, it had a bug in it that allowed
 non-strings to pass through. Non-scalars would throw a PHP Warning,
 whereas a non-string scalar would be handled by `strtolower()` and
 converted into a string.

 What input validation and error messaging should happen here?

 I agree with @wppunk that silently passing it through makes it difficult
 to debug.

 I'm also leaning towards allowing non-string scalars. If in the future PHP
 deprecates passing non-strings to `strtolower()`, then go further with the
 input validation. For non-scalars, a `E_USER_WARNING` needs to be thrown
 to avoid silently passing an issue through the function.

 Pulling this back into 5.9 to make adjustments. Patch with tests are
 coming.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/54160#comment:25>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list