[wp-trac] [WordPress Trac] #47164: map_deep in formatting.php do not handle null-byte

[WordPress Trac]

#47164: map_deep in formatting.php do not handle null-byte
---------------------------------------------------+---------------------
 Reporter:  bitcomplex                             |       Owner:  (none)
     Type:  defect (bug)                           |      Status:  new
 Priority:  normal                                 |   Milestone:  6.2
Component:  Formatting                             |     Version:  5.2.2
 Severity:  critical                               |  Resolution:
 Keywords:  has-patch has-unit-tests dev-feedback  |     Focuses:
---------------------------------------------------+---------------------
Changes (by costdev):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests dev-
     feedback


Comment:

 I saw the failing test on PHP 5.6 on the PR and starting looking into why
 it's failing.

 @hellofromTonya @jrf @SergeyBiryukov Would appreciate a confidence check
 on the below.

 -----

 Sample:
 {{{#!php
 <?php
 class MyClass {
     public    $pub  = 0;
     protected $prot = 1;
     private   $priv = 2;
 }

 // Produce an object with null byte in the property name.
 $obj      = (object) (array) new MyClass();
 $obj_vars = get_object_vars( $obj );
 }}}

 - In PHP 5.6,  `$obj_vars` is `[ 'pub' => 0, 'prot' => 1 ];`
 - In PHP 7.0+, `$obj_vars` is `[ 'pub' => 0, '\0*\0prot' => 1,
 '\0MyClass\0priv' => 2 ];`
 [https://3v4l.org/STWer 3v4l]

 -----

 {{{#!php
 <?php
 foreach ( $obj_vars as $name => $value ) {
     $obj->$name = $value;
 }
 }}}

 - PHP 5.6 will not throw an error.
 - PHP 7.0+ will throw an error: `Fatal error: Cannot access property
 started with '\0'`
 [https://3v4l.org/ZLvSd 3v4l]

 -----

 If we then clean up the property name with, for example, `trim( $name,
 "\0*" )`:
 - PHP 5.6 adds one new `public` property with the same name as the
 `protected` property.
 - PHP 7.0+ adds two new `public` properties with the same names as the
 `protected` and `private` properties.
 [https://3v4l.org/WXASh 3v4l]

 -----

 Provided there's no issues I've missed/created along the way, could I get
 a confidence check?

 When writing a test to cover this, we ''must'' test different values
 depending on the PHP version. Correct, or incorrect?

 i.e.
 {{{#!php
 <?php

 // PHP 5.6
 $expected = array(
     0, // public $pub
     1, // protected $prot
     1, // public $prot
     2  // private $priv
 );

 // PHP 7.0+ adjustment
 if ( version_compare( PHP_VERSION, '7.0', '>=' ) ) {
         $expected[] = 2; // public $priv
 }

 // Assertion.
 }}}

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