8000 bug #28408 [VarExporter] fix exporting instances of final classes tha… · symfony/symfony@f3c3c0b · GitHub
[go: up one dir, main page]
Skip to content

Commit f3c3c0b

Browse files
nicolas-grekasnicolas-grekas
committed
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 ------- 4a16b6c [VarExporter] fix exporting instances of final classes that extend internal ones
2 parents 004c315 + 4a16b6c commit f3c3c0b

15 files changed

+97
-43
lines changed

‎src/Symfony/Component/VarExporter/Internal/Exporter.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,15 @@ private static function exportRegistry(Registry $value, string $indent, string $
283283
$serializables[$k] = $class;
284284
continue;
285285
}
286+
if (!Registry::$instantiableWithoutConstructor[$class]) {
287+
if (is_subclass_of($class, 'Throwable')) {
288+
$eol = is_subclass_of($class, 'Error') ? "\0Error\0" : "\0Exception\0";
289+
$serializables[$k] = 'O:'.\strlen($class).':"'.$class.'":1:{s:'.(5 + \strlen($eol)).':"'.$eol.'trace";a:0:{}}';
290+
} else {
291+
$serializables[$k] = 'O:'.\strlen($class).':"'.$class.'":0:{}';
292+
}
293+
continue;
294+
}
286295
$code .= $subIndent.(1 !== $k - $j ? $k.' => ' : '');
287296
$j = $k;
288297
$eol = ",\n";
@@ -299,21 +308,21 @@ private static function exportRegistry(Registry $value, string $indent, string $
299308
} else {
300309
$seen[$class] = true;
301310
if (Registry::$cloneable[$class]) {
302-
$code .= 'clone ('.($prototypesAccess++ ? '$p' : '($p =& '.$r.'::$prototypes)').$c.' ?? '.$r.'::p';
311+
$code .= 'clone ('.($prototypesAccess++ ? '$p' : '($p = &'.$r.'::$prototypes)').$c.' ?? '.$r.'::p';
303312
} else {
304-
$code .= '('.($factoriesAccess++ ? '$f' : '($f =& '.$r.'::$factories)').$c.' ?? '.$r.'::f';
313+
$code .= '('.($factoriesAccess++ ? '$f' : '($f = &'.$r.'::$factories)').$c.' ?? '.$r.'::f';
305314
$eol = '()'.$eol;
306315
}
307-
$code .= '('.substr($c, 1, -1).', '.self::export(Registry::$instantiableWithoutConstructor[$class]).'))';
316+
$code .= '('.substr($c, 1, -1).'))';
308317
}
309318
$code .= $eol;
310319
}
311320

312321
if (1 === $prototypesAccess) {
313-
$code = str_replace('($p =& '.$r.'::$prototypes)', $r.'::$prototypes', $code);
322+
$code = str_replace('($p = &'.$r.'::$prototypes)', $r.'::$prototypes', $code);
314323
}
315324
if (1 === $factoriesAccess) {
316-
$code = str_replace('($f =& '.$r.'::$factories)', $r.'::$factories', $code);
325+
$code = str_replace('($f = &'.$r.'::$factories)', $r.'::$factories', $code);
317326
}
318327
if ('' !== $code) {
319328
$code = "\n".$code.$indent;

‎src/Symfony/Component/VarExporter/Internal/Registry.php

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,43 +46,50 @@ public static function unserialize($objects, $serializables)
4646
return $objects;
4747
}
4848

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

5353
return self::$prototypes[$class];
5454
}
5555

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

60-
return self::$factories[$class] = \Closure::fromCallable(array($reflector, $instantiable 10000 WithoutConstructor ? 'newInstanceWithoutConstructor' : 'newInstance'));
60+
return self::$factories[$class] = \Closure::fromCallable(array($reflector, 'newInstanceWithoutConstructor'));
6161
}
6262

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

67-
if (self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor ?? (!$reflector->isFinal() || !$reflector->isInternal())) {
67+
if (self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor || !$reflector->isFinal()) {
6868
$proto = $reflector->newInstanceWithoutConstructor();
6969
} else {
70-
try {
71-
$proto = $reflector->newInstance();
72-
} catch (\Throwable $e) {
73-
throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class), 0, $e);
70+
self::$instantiableWithoutConstructor[$class] = true;
71+
$r = $reflector;
72+
do {
73+
if ($r->isInternal()) {
74+
self::$instantiableWithoutConstructor[$class] = false;
75+
if (false === $proto = @unserialize('O:'.\strlen($class).':"'.$class.'":0:{}')) {
76+
throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class));
77+
}
78+
break;
79+
}
80+
} while ($r = $r->getParentClass());
81+
82+
if (!$r) {
83+
$proto = $reflector->newInstanceWithoutConstructor();
7484
}
7585
}
7686

77-
if (null !== self::$cloneable[$class] = $cloneable) {
78-
// no-op
79-
} elseif ($proto instanceof \Reflector || $proto instanceof \ReflectionGenerator || $proto instanceof \ReflectionType || $proto instanceof \IteratorIterator || $proto instanceof \RecursiveIteratorIterator) {
80-
if (!$proto instanceof \Serializable && !\method_exists($proto, '__wakeup')) {
87+
if (null === self::$cloneable[$class] = $cloneable) {
88+
if (($proto instanceof \Reflector || $proto instanceof \ReflectionGenerator || $proto instanceof \ReflectionType || $proto instanceof \IteratorIterator || $proto instanceof \RecursiveIteratorIterator) && (!$proto instanceof \Serializable && !\method_exists($proto, '__wakeup'))) {
8189
throw new \Exception(sprintf("Serialization of '%s' is not allowed", $class));
8290
}
83-
self::$cloneable[$class] = false;
84-
} else {
85-
self::$cloneable[$class] = !$reflector->hasMethod('__clone');
91+
92+
self::$cloneable[$class] = $reflector->isCloneable() && !$reflector->hasMethod('__clone');
8693
}
8794

8895
self::$prototypes[$class] = $proto;

‎src/Symfony/Component/VarExporter/Tests/Fixtures/array-iterator.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\ArrayIterator::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayIterator::class, true)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\ArrayIterator::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayIterator::class)),
66
],
77
null,
88
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/array-object-custom.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
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)),
5+
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)),
66
],
77
null,
88
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/array-object.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\ArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayObject::class, true)),
5+
clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\ArrayObject::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\ArrayObject::class)),
66
clone $p[\ArrayObject::class],
77
],
88
null,

‎src/Symfony/Component/VarExporter/Tests/Fixtures/clone.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
(($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))(),
6-
($f[\Symfony\Component\VarExporter\Tests\MyNotCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyNotCloneable::class, true))(),
5+
(($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))(),
6+
($f[\Symfony\Component\VarExporter\Tests\MyNotCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyNotCloneable::class))(),
77
],
88
null,
99
[],

‎src/Symfony/Component/VarExporter/Tests/Fixtures/datetime.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\DateTime::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\DateTime::class, true)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\DateTime::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\DateTime::class)),
66
],
77
null,
88
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/error.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
(\Symfony\Component\VarExporter\Internal\Registry::$factories[\Error::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Error::class, true))(),
5+
(\Symfony\Component\VarExporter\Internal\Registry::$factories[\Error::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Error::class))(),
66
],
77
null,
88
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/final-error.php

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
4+
$o = \Symfony\Component\VarExporter\Internal\Registry::unserialize([], [
5+
'O:46:"Symfony\\Component\\VarExporter\\Tests\\FinalError":1:{s:12:"'."\0".'Error'."\0".'trace";a:0:{}}',
6+
]),
7+
null,
8+
[
9+
\TypeError::class => [
10+
'file' => [
11+
\dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php',
12+
],
13+
'line' => [
14+
123,
15+
],
16+
],
17+
],
18+
$o[0],
19+
[
20+
1 => 0,
21+
]
22+
);

‎src/Symfony/Component/VarExporter/Tests/Fixtures/hard-references.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class, true)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class)),
66
],
77
[
88
$r = [],

‎src/Symfony/Component/VarExporter/Tests/Fixtures/private.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
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)),
6-
clone ($p[\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class, true)),
5+
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)),
6+
clone ($p[\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\Symfony\Component\VarExporter\Tests\MyPrivateChildValue::class)),
77
],
88
null,
99
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/spl-object-storage.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
clone (($p =& \Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\SplObjectStorage::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\SplObjectStorage::class, true)),
6-
clone ($p[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class, true)),
5+
clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)[\SplObjectStorage::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\SplObjectStorage::class)),
6+
clone ($p[\stdClass::class] ?? \Symfony\Component\VarExporter\Internal\Registry::p(\stdClass::class)),
77
],
88
null,
99
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/var-on-sleep.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
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)),
5+
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)),
66
],
77
null,
88
[

‎src/Symfony/Component/VarExporter/Tests/Fixtures/wakeup.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
return \Symfony\Component\VarExporter\Internal\Hydrator::hydrate(
44
$o = [
5-
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)),
5+
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)),
66
clone $p[\Symfony\Component\VarExporter\Tests\MyWakeup::class],
77
],
88
null,

‎src/Symfony/Component/VarExporter/Tests/VarExporterTest.php

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,17 +158,23 @@ public function provideMarshall()
158158

159159
$value = new \Error();
160160

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

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

169169
yield array('error', $value);
170170

171171
yield array('var-on-sleep', new GoodNight());
172+
173+
$value = new FinalError(false);
174+
$rt->setValue($value, array());
175+
$rl->setValue($value, 123);
176+
177+
yield array('final-error', $value);
172178
}
173179
}
174180

@@ -262,3 +268,13 @@ public function __sleep()
262268
return array('good');
263269
}
264270
}
271+
272+
final class FinalError extends \Error
273+
{
274+
public function __construct(bool $throw = true)
275+
{
276+
if ($throw) {
277+
throw new \BadMethodCallException('Should not be called.');
278+
}
279+
}
280+
}

0 commit comments

Comments
 (0)
0