bug #28408 [VarExporter] fix exporting instances of final classes that extend internal ones (nicolas-grekas)

This PR was merged into the 4.2-dev branch. Discussion ---------- [VarExporter] fix exporting instances of final classes that extend internal ones | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Another PHP oddity found today. Commits ------- 4a16b6ca65 [VarExporter] fix exporting instances of final classes that extend internal ones
2018-09-08 20:14:47 +02:00 · 2018-09-08 20:14:47 +02:00 · f3c3c0b4d6
commit f3c3c0b4d6
parent 004c315b0c 4a16b6ca65
15 changed files with 97 additions and 43 deletions
--- a/src/Symfony/Component/VarExporter/Internal/Exporter.php
+++ b/src/Symfony/Component/VarExporter/Internal/Exporter.php
@ -283,6 +283,15 @@ class Exporter
                $serializables[$k] = $class;
                continue;
            }
+            if (!Registry::$instantiableWithoutConstructor[$class]) {
+                if (is_subclass_of($class, 'Throwable')) {
+                    $eol = is_subclass_of($class, 'Error') ? "\0Error\0" : "\0Exception\0";
+                    $serializables[$k] = 'O:'.\strlen($class).':"'.$class.'":1:{s:'.(5 + \strlen($eol)).':"'.$eol.'trace";a:0:{}}';
+                } else {
+                    $serializables[$k] = 'O:'.\strlen($class).':"'.$class.'":0:{}';
+                }
+                continue;
+            }
            $code .= $subIndent.(1 !== $k - $j ? $k.' => ' : '');
            $j = $k;
            $eol = ",\n";
@ -299,21 +308,21 @@ class Exporter
            } else {
                $seen[$class] = true;
                if (Registry::$cloneable[$class]) {
-                    $code .= 'clone ('.($prototypesAccess++ ? '$p' : '($p =& '.$r.'::$prototypes)').$c.' ?? '.$r.'::p';
+                    $code .= 'clone ('.($prototypesAccess++ ? '$p' : '($p = &'.$r.'::$prototypes)').$c.' ?? '.$r.'::p';
                } else {
-                    $code .= '('.($factoriesAccess++ ? '$f' : '($f =& '.$r.'::$factories)').$c.' ?? '.$r.'::f';
+                    $code .= '('.($factoriesAccess++ ? '$f' : '($f = &'.$r.'::$factories)').$c.' ?? '.$r.'::f';
                    $eol = '()'.$eol;
                }
-                $code .= '('.substr($c, 1, -1).', '.self::export(Registry::$instantiableWithoutConstructor[$class]).'))';
+                $code .= '('.substr($c, 1, -1).'))';
            }
            $code .= $eol;
        }

        if (1 === $prototypesAccess) {
-            $code = str_replace('($p =& '.$r.'::$prototypes)', $r.'::$prototypes', $code);
+            $code = str_replace('($p = &'.$r.'::$prototypes)', $r.'::$prototypes', $code);
        }
        if (1 === $factoriesAccess) {
-            $code = str_replace('($f =& '.$r.'::$factories)', $r.'::$factories', $code);
+            $code = str_replace('($f = &'.$r.'::$factories)', $r.'::$factories', $code);
        }
        if ('' !== $code) {
            $code = "\n".$code.$indent;
--- a/src/Symfony/Component/VarExporter/Internal/Registry.php
+++ b/src/Symfony/Component/VarExporter/Internal/Registry.php
@ -46,43 +46,50 @@ class Registry
        return $objects;
    }

-    public static function p($class, $instantiableWithoutConstructor)
+    public static function p($class)
    {
-        self::getClassReflector($class, $instantiableWithoutConstructor, true);
+        self::getClassReflector($class, true, true);

        return self::$prototypes[$class];
    }

-    public static function f($class, $instantiableWithoutConstructor)
+    public static function f($class)
    {
-        $reflector = self::$reflectors[$class] ?? self::getClassReflector($class, $instantiableWithoutConstructor, false);
+        $reflector = self::$reflectors[$class] ?? self::getClassReflector($class, true, false);

-        return self::$factories[$class] = \Closure::fromCallable(array($reflector, $instantiableWithoutConstructor ? 'newInstanceWithoutConstructor' : 'newInstance'));
+        return self::$factories[$class] = \Closure::fromCallable(array($reflector, 'newInstanceWithoutConstructor'));
    }

-    public static function getClassReflector($class, $instantiableWithoutConstructor = null, $cloneable = null)
+    public static function getClassReflector($class, $instantiableWithoutConstructor = false, $cloneable = null)
    {
        $reflector = new \ReflectionClass($class);

-        if (self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor ?? (!$reflector->isFinal() || !$reflector->isInternal())) {
+        if (self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor || !$reflector->isFinal()) {
            $proto = $reflector->newInstanceWithoutConstructor();
        } else {
-            try {
-                $proto = $reflector->newInstance();
-            } catch (\Throwable $e) {
-                throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class), 0, $e);
+            self::$instantiableWithoutConstructor[$class] = true;
+            $r = $reflector;
+            do {
+                if ($r->isInternal()) {
+                    self::$instantiableWithoutConstructor[$class] = false;
+                    if (false === $proto = @unserialize('O:'.\strlen($class).':"'.$class.'":0:{}')) {
+                        throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class));
+                    }
+                    break;
+                }
+            } while ($r = $r->getParentClass());
+
+            if (!$r) {
+                $proto = $reflector->newInstanceWithoutConstructor();
            }
        }

-        if (null !== self::$cloneable[$class] = $cloneable) {
-            // no-op
-        } elseif ($proto instanceof \Reflector || $proto instanceof \ReflectionGenerator || $proto instanceof \ReflectionType || $proto instanceof \IteratorIterator || $proto instanceof \RecursiveIteratorIterator) {
-            if (!$proto instanceof \Serializable && !\method_exists($proto, '__wakeup')) {
+        if (null === self::$cloneable[$class] = $cloneable) {
+            if (($proto instanceof \Reflector || $proto instanceof \ReflectionGenerator || $proto instanceof \ReflectionType || $proto instanceof \IteratorIterator || $proto instanceof \RecursiveIteratorIterator) && (!$proto instanceof \Serializable && !\method_exists($proto, '__wakeup'))) {
                throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class));
            }
-            self::$cloneable[$class] = false;
-        } else {
-            self::$cloneable[$class] = !$reflector->hasMethod('__clone');
+
+            self::$cloneable[$class] = $reflector->isCloneable() && !$reflector->hasMethod('__clone');
        }

        self::$prototypes[$class] = $proto;
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/array-iterator.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/array-iterator.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\ArrayIterator::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayIterator::class, true)),
+        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\ArrayIterator::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayIterator::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/array-object-custom.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/array-object-custom.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\Symfony\Component\VarExporter\Tests\MyArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyArrayObject::class, true)),
+        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\Symfony\Component\VarExporter\Tests\MyArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyArrayObject::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/array-object.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/array-object.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\ArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayObject::class, true)),
+        clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\ArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayObject::class)),
        clone $p[\ArrayObject::class],
    ],
    null,
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/clone.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/clone.php
@ -2,8 +2,8 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        (($f =& \Symfony\Component\VarExporter\Internal\Registry::$factories)[\Symfony\Component\VarExporter\Tests\MyCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyCloneable::class, true))(),
-        ($f[\Symfony\Component\VarExporter\Tests\MyNotCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyNotCloneable::class, true))(),
+        (($f = &\Symfony\Component\VarExporter\Internal\Registry::$factories)[\Symfony\Component\VarExporter\Tests\MyCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyCloneable::class))(),
+        ($f[\Symfony\Component\VarExporter\Tests\MyNotCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyNotCloneable::class))(),
    ],
    null,
    [],
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/datetime.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/datetime.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\DateTime::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\DateTime::class, true)),
+        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\DateTime::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\DateTime::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/error.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/error.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        (\Symfony\Component\VarExporter\Internal\Registry::$factories[\Error::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Error::class, true))(),
+        (\Symfony\Component\VarExporter\Internal\Registry::$factories[\Error::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Error::class))(),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php
@ -0,0 +1,22 @@
+<?php
+
+return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
+    $o = \Symfony\Component\VarExporter\Internal\Registry::unserialize([], [
+        'O:46:"Symfony\\Component\\VarExporter\\Tests\\FinalError":1:{s:12:"'."\0".'Error'."\0".'trace";a:0:{}}',
+    ]),
+    null,
+    [
+        \TypeError::class => [
+            'file' => [
+                \dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php',
+            ],
+            'line' => [
+                123,
+            ],
+        ],
+    ],
+    $o[0],
+    [
+        1 => 0,
+    ]
+);
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/hard-references.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/hard-references.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class, true)),
+        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class)),
    ],
    [
        $r = [],
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/private.php
@ -2,8 +2,8 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\Symfony\Component\VarExporter\Tests\MyPrivateValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateValue::class, true)),
-        clone ($p[\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class, true)),
+        clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\Symfony\Component\VarExporter\Tests\MyPrivateValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateValue::class)),
+        clone ($p[\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/spl-object-storage.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/spl-object-storage.php
@ -2,8 +2,8 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\SplObjectStorage::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\SplObjectStorage::class, true)),
-        clone ($p[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class, true)),
+        clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\SplObjectStorage::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\SplObjectStorage::class)),
+        clone ($p[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/var-on-sleep.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/var-on-sleep.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\Symfony\Component\VarExporter\Tests\GoodNight::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\GoodNight::class, true)),
+        clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\Symfony\Component\VarExporter\Tests\GoodNight::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\GoodNight::class)),
    ],
    null,
    [
--- a/src/Symfony/Component/VarExporter/Tests/Fixtures/wakeup.php
+++ b/src/Symfony/Component/VarExporter/Tests/Fixtures/wakeup.php
@ -2,7 +2,7 @@

 return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
    $o = [
-        clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\Symfony\Component\VarExporter\Tests\MyWakeup::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyWakeup::class, true)),
+        clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\Symfony\Component\VarExporter\Tests\MyWakeup::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyWakeup::class)),
        clone $p[\Symfony\Component\VarExporter\Tests\MyWakeup::class],
    ],
    null,
--- a/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php
+++ b/src/Symfony/Component/VarExporter/Tests/VarExporterTest.php
@ -158,17 +158,23 @@ class VarExporterTest extends TestCase

        $value = new \Error();

-        $r = new \ReflectionProperty('Error', 'trace');
-        $r->setAccessible(true);
-        $r->setValue($value, array('file' => __FILE__, 'line' => 123));
+        $rt = new \ReflectionProperty('Error', 'trace');
+        $rt->setAccessible(true);
+        $rt->setValue($value, array('file' => __FILE__, 'line' => 123));

-        $r = new \ReflectionProperty('Error', 'line');
-        $r->setAccessible(true);
-        $r->setValue($value, 234);
+        $rl = new \ReflectionProperty('Error', 'line');
+        $rl->setAccessible(true);
+        $rl->setValue($value, 234);

        yield array('error', $value);

        yield array('var-on-sleep', new GoodNight());
+
+        $value = new FinalError(false);
+        $rt->setValue($value, array());
+        $rl->setValue($value, 123);
+
+        yield array('final-error', $value);
    }
 }

@ -262,3 +268,13 @@ class GoodNight
        return array('good');
    }
 }
+
+final class FinalError extends \Error
+{
+    public function __construct(bool $throw = true)
+    {
+        if ($throw) {
+            throw new \BadMethodCallException('Should not be called.');
+        }
+    }
+}