[wp-trac] [WordPress Trac] #43312: PHP 7.2 warning in wp_kses_attr()

[WordPress Trac]

#43312: PHP 7.2 warning in wp_kses_attr()
---------------------------------------------+-----------------------------
 Reporter:  andrei0x309                      |       Owner:  SergeyBiryukov
     Type:  defect (bug)                     |      Status:  reopened
 Priority:  normal                           |   Milestone:  4.9.5
Component:  Formatting                       |     Version:
 Severity:  normal                           |  Resolution:
 Keywords:  has-patch has-unit-tests commit  |     Focuses:
---------------------------------------------+-----------------------------
Changes (by apokalyptik):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 In r42861 the new conditional logic is not equivalent to the old
 conditional logic.

 {{{
   $ php -v
     PHP 7.0.27-0+deb9u1 (cli) (built: Jan  5 2018 13:51:52) ( NTS )

   $ cat test.php
     <?php
     $element = 'foo';
     $allowed_html = array( 'foo' => false );
     var_dump( ( ! isset( $allowed_html[ strtolower( $element ) ] ) || true
 === $allowed_html[ strtolower( $element ) ] || count(
     $allowed_html[ strtolower( $element ) ] ) == 0 ) );
     $element_low = strtolower( $element );
     var_dump( ( empty( $allowed_html[ $element_low ] ) || true ===
 $allowed_html[ $element_low ] ) );

   $ php ./test.php
     bool(false)
     bool(true)
 }}}

 as you can see above empty() returns true in places where isset() does
 not.

 ironically this use of empty covers the omission of the empty array case.
 but once that is fixed you'll want to check if it's an array and if so
 then count and check to keep the logic consistent.

 if the logic needs to be changed for what is and is not supported, then
 that's fine but kses is pretty important and so it shouldn't be hidden in
 a ticket about fixing warnings (which to me implies that it's about the
 warning and not the logic/functionality and therefor tends to get less
 scrutiny)

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