Skip to content

Commit 2879baf

Browse files
nicolas-grekasnicolas-grekas
committed
bug symfony#28437 [VarExporter] fix more edge cases (nicolas-grekas)
This PR was merged into the 4.2-dev branch. Discussion ---------- [VarExporter] fix more edge cases | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - As found while preparing symfony#28417 Commits ------- 443bd11 [VarExporter] fix more edge cases
2 parents deae538 + 443bd11 commit 2879baf

File tree

17 files changed

+100
-69
lines changed

17 files changed

+100
-69
lines changed

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

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -77,19 +77,7 @@ public static function prepare($values, $objectsPool, &$refsPool, &$objectsCount
7777
$properties = array();
7878
$sleep = null;
7979
$arrayValue = (array) $value;
80-
81-
if (!isset(Registry::$reflectors[$class])) {
82-
Registry::getClassReflector($class);
83-
try {
84-
serialize(Registry::$prototypes[$class]);
85-
} catch (\Exception $e) {
86-
throw new NotInstantiableTypeException($class, $e);
87-
}
88-
if (\method_exists($class, '__sleep')) {
89-
Registry::getClassReflector($class, Registry::$instantiableWithoutConstructor[$class], Registry::$cloneable[$class]);
90-
}
91-
}
92-
$reflector = Registry::$reflectors[$class];
80+
$reflector = Registry::$reflectors[$class] ?? Registry::getClassReflector($class);
9381
$proto = Registry::$prototypes[$class];
9482

9583
if (($value instanceof \ArrayIterator || $value instanceof \ArrayObject) && null !== $proto) {
@@ -133,7 +121,7 @@ public static function prepare($values, $objectsPool, &$refsPool, &$objectsCount
133121
foreach ($arrayValue as $name => $v) {
134122
$n = (string) $name;
135123
if ('' === $n || "\0" !== $n[0]) {
136-
$c = '*';
124+
$c = 'stdClass';
137125
$properties[$c][$n] = $v;
138126
unset($sleep[$n]);
139127
} elseif ('*' === $n[1]) {
@@ -305,7 +293,7 @@ private static function exportRegistry(Registry $value, string $indent, string $
305293
$code .= $subIndent.(1 !== $k - $j ? $k.' => ' : '');
306294
$j = $k;
307295
$eol = ",\n";
308-
$c = '[\\'.$class.'::class]';
296+
$c = '['.self::export($class).']';
309297

310298
if ($seen[$class] ?? false) {
311299
if (Registry::$cloneable[$class]) {
@@ -351,8 +339,7 @@ private static function exportHydrator(Hydrator $value, string $indent, string $
351339
{
352340
$code = '';
353341
foreach ($value->properties as $class => $properties) {
354-
$c = '*' !== $class ? '\\'.$class.'::class' : "'*'";
355-
$code .= $subIndent.' '.$c.' => '.self::export($properties, $subIndent.' ').",\n";
342+
$code .= $subIndent.' '.self::export($class).' => '.self::export($properties, $subIndent.' ').",\n";
356343
}
357344

358345
$code = array(

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

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public static function hydrate($objects, $values, $properties, $value, $wakeups)
4949

5050
public static function getHydrator($class)
5151
{
52-
if ('*' === $class) {
52+
if ('stdClass' === $class) {
5353
return self::$hydrators[$class] = static function ($properties, $objects) {
5454
foreach ($properties as $name => $values) {
5555
foreach ($values as $i => $v) {
@@ -62,13 +62,17 @@ public static function getHydrator($class)
6262
$classReflector = Registry::$reflectors[$class] ?? Registry::getClassReflector($class);
6363

6464
if (!$classReflector->isInternal()) {
65-
return self::$hydrators[$class] = (self::$hydrators['*'] ?? self::getHydrator('*'))->bindTo(null, $class);
65+
return self::$hydrators[$class] = (self::$hydrators['stdClass'] ?? self::getHydrator('stdClass'))->bindTo(null, $class);
66+
}
67+
68+
if ($classReflector->name !== $class) {
69+
return self::$hydrators[$classReflector->name] ?? self::getHydrator($classReflector->name);
6670
}
6771

6872
switch ($class) {
6973
case 'ArrayIterator':
7074
case 'ArrayObject':
71-
$constructor = $classReflector->getConstructor();
75+
$constructor = \Closure::fromCallable(array($classReflector->getConstructor(), 'invokeArgs'));
7276

7377
return self::$hydrators[$class] = static function ($properties, $objects) use ($constructor) {
7478
foreach ($properties as $name => $values) {
@@ -78,17 +82,17 @@ public static function getHydrator($class)
7882
}
7983
}
8084
}
81-
foreach ($properties["\0"] as $i => $v) {
82-
$constructor->invokeArgs($objects[$i], $v);
85+
foreach ($properties["\0"] ?? array() as $i => $v) {
86+
$constructor($objects[$i], $v);
8387
}
8488
};
8589

8690
case 'ErrorException':
87-
return self::$hydrators[$class] = (self::$hydrators['*'] ?? self::getHydrator('*'))->bindTo(null, new class() extends \ErrorException {
91+
return self::$hydrators[$class] = (self::$hydrators['stdClass'] ?? self::getHydrator('stdClass'))->bindTo(null, new class() extends \ErrorException {
8892
});
8993

9094
case 'TypeError':
91-
return self::$hydrators[$class] = (self::$hydrators['*'] ?? self::getHydrator('*'))->bindTo(null, new class() extends \Error {
95+
return self::$hydrators[$class] = (self::$hydrators['stdClass'] ?? self::getHydrator('stdClass'))->bindTo(null, new class() extends \Error {
9296
});
9397

9498
case 'SplObjectStorage':
@@ -109,19 +113,28 @@ public static function getHydrator($class)
109113
};
110114
}
111115

112-
$propertyReflectors = array();
116+
$propertySetters = array();
113117
foreach ($classReflector->getProperties() as $propertyReflector) {
114118
if (!$propertyReflector->isStatic()) {
115119
$propertyReflector->setAccessible(true);
116-
$propertyReflectors[$propertyReflector->name] = $propertyReflector;
120+
$propertySetters[$propertyReflector->name] = \Closure::fromCallable(array($propertyReflector, 'setValue'));
117121
}
118122
}
119123

120-
return self::$hydrators[$class] = static function ($properties, $objects) use ($propertyReflectors) {
124+
if (!$propertySetters) {
125+
return self::$hydrators[$class] = self::$hydrators['stdClass'] ?? self::getHydrator('stdClass');
126+
}
127+
128+
return self::$hydrators[$class] = static function ($properties, $objects) use ($propertySetters) {
121129
foreach ($properties as $name => $values) {
122-
$p = $propertyReflectors[$name];
130+
if ($setValue = $propertySetters[$name] ?? null) {
131+
foreach ($values as $i => $v) {
132+
$setValue($objects[$i], $v);
133+
}
134+
continue;
135+
}
123136
foreach ($values as $i => $v) {
124-
$p->setValue($objects[$i], $v);
137+
$objects[$i]->$name = $v;
125138
}
126139
}
127140
};

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

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,17 +65,26 @@ public static function f($class)
6565

6666
public static function getClassReflector($class, $instantiableWithoutConstructor = false, $cloneable = null)
6767
{
68-
if (!\class_exists($class)) {
68+
if (!\class_exists($class) && !\interface_exists($class, false) && !\trait_exists($class, false)) {
6969
throw new ClassNotFoundException($class);
7070
}
7171
$reflector = new \ReflectionClass($class);
7272

73-
if (self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor || !$reflector->isFinal()) {
73+
if ($instantiableWithoutConstructor) {
7474
$proto = $reflector->newInstanceWithoutConstructor();
75+
} elseif (!$reflector->isInstantiable()) {
76+
throw new NotInstantiableTypeException($class);
77+
} elseif ($reflector->name !== $class) {
78+
$reflector = self::$reflectors[$name = $reflector->name] ?? self::getClassReflector($name, $instantiableWithoutConstructor, $cloneable);
79+
self::$cloneable[$class] = self::$cloneable[$name];
80+
self::$instantiableWithoutConstructor[$class] = self::$instantiableWithoutConstructor[$name];
81+
self::$prototypes[$class] = self::$prototypes[$name];
82+
83+
return self::$reflectors[$class] = $reflector;
7584
} else {
7685
try {
7786
$proto = $reflector->newInstanceWithoutConstructor();
78-
self::$instantiableWithoutConstructor[$class] = true;
87+
$instantiableWithoutConstructor = true;
7988
} catch (\ReflectionException $e) {
8089
$proto = $reflector->implementsInterface('Serializable') ? 'C:' : 'O:';
8190
if ('C:' === $proto && !$reflector->getMethod('unserialize')->isInternal()) {
@@ -84,31 +93,48 @@ public static function getClassReflector($class, $instantiableWithoutConstructor
8493
throw new NotInstantiableTypeException($class);
8594
}
8695
}
96+
if (null !== $proto && !$proto instanceof \Throwable) {
97+
try {
98+
if (!$proto instanceof \Serializable && !\method_exists($class, '__sleep')) {
99+
serialize($proto);
100+
} elseif ($instantiableWithoutConstructor) {
101+
serialize($reflector->newInstanceWithoutConstructor());
102+
} else {
103+
serialize(unserialize(($proto instanceof \Serializable ? 'C:' : 'O:').\strlen($class).':"'.$class.'":0:{}'));
104+
}
105+
} catch (\Exception $e) {
106+
throw new NotInstantiableTypeException($class, $e);
107+
}
108+
}
87109
}
88110

89-
if (null === self::$cloneable[$class] = $cloneable) {
111+
if (null === $cloneable) {
90112
if (($proto instanceof \Reflector || $proto instanceof \ReflectionGenerator || $proto instanceof \ReflectionType || $proto instanceof \IteratorIterator || $proto instanceof \RecursiveIteratorIterator) && (!$proto instanceof \Serializable && !\method_exists($proto, '__wakeup'))) {
91113
throw new NotInstantiableTypeException($class);
92114
}
93115

94-
self::$cloneable[$class] = $reflector->isCloneable() && !$reflector->hasMethod('__clone');
116+
$cloneable = $reflector->isCloneable() && !$reflector->hasMethod('__clone');
95117
}
96118

119+
self::$cloneable[$class] = $cloneable;
120+
self::$instantiableWithoutConstructor[$class] = $instantiableWithoutConstructor;
97121
self::$prototypes[$class] = $proto;
98122

99123
if ($proto instanceof \Throwable) {
100-
static $trace;
124+
static $setTrace;
101125

102-
if (null === $trace) {
103-
$trace = array(
126+
if (null === $setTrace) {
127+
$setTrace = array(
104128
new \ReflectionProperty(\Error::class, 'trace'),
105129
new \ReflectionProperty(\Exception::class, 'trace'),
106130
);
107-
$trace[0]->setAccessible(true);
108-
$trace[1]->setAccessible(true);
131+
$setTrace[0]->setAccessible(true);
132+
$setTrace[1]->setAccessible(true);
133+
$setTrace[0] = \Closure::fromCallable(array($setTrace[0], 'setValue'));
134+
$setTrace[1] = \Closure::fromCallable(array($setTrace[1], 'setValue'));
109135
}
110136

111-
$trace[$proto instanceof \Exception]->setValue($proto, array());
137+
$setTrace[$proto instanceof \Exception]($proto, array());
112138
}
113139

114140
return self::$reflectors[$class] = $reflector;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
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)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes['ArrayIterator'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('ArrayIterator')),
66
],
77
null,
88
[
9-
\ArrayIterator::class => [
9+
'ArrayIterator' => [
1010
"\0" => [
1111
[
1212
[

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
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)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes['Symfony\\Component\\VarExporter\\Tests\\MyArrayObject'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('Symfony\\Component\\VarExporter\\Tests\\MyArrayObject')),
66
],
77
null,
88
[
9-
\ArrayObject::class => [
9+
'ArrayObject' => [
1010
"\0" => [
1111
[
1212
[

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
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)),
6-
clone $p[\ArrayObject::class],
5+
clone (($p = &\Symfony\Component\VarExporter\Internal\Registry::$prototypes)['ArrayObject'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('ArrayObject')),
6+
clone $p['ArrayObject'],
77
],
88
null,
99
[
10-
\ArrayObject::class => [
10+
'ArrayObject' => [
1111
"\0" => [
1212
[
1313
[
@@ -18,7 +18,7 @@
1818
],
1919
],
2020
],
21-
'*' => [
21+
'stdClass' => [
2222
'foo' => [
2323
$o[1],
2424
],

‎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))(),
6-
($f[\Symfony\Component\VarExporter\Tests\MyNotCloneable::class] ?? \Symfony\Component\VarExporter\Internal\Registry::f(\Symfony\Component\VarExporter\Tests\MyNotCloneable::class))(),
5+
(($f = &\Symfony\Component\VarExporter\Internal\Registry::$factories)['Symfony\\Component\\VarExporter\\Tests\\MyCloneable'] ?? \Symfony\Component\VarExporter\Internal\Registry::f('Symfony\\Component\\VarExporter\\Tests\\MyCloneable'))(),
6+
($f['Symfony\\Component\\VarExporter\\Tests\\MyNotCloneable'] ?? \Symfony\Component\VarExporter\Internal\Registry::f('Symfony\\Component\\VarExporter\\Tests\\MyNotCloneable'))(),
77
],
88
null,
99
[],

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
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)),
5+
clone (\Symfony\Component\VarExporter\Internal\Registry::$prototypes['DateTime'] ?? \Symfony\Component\VarExporter\Internal\Registry::p('DateTime')),
66
],
77
null,
88
[
9-
'*' => [
9+
'stdClass' => [
1010
'date' => [
1111
'1970-01-01 00:00:00.000000',
1212
],

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,19 @@
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))(),
5+
(\Symfony\Component\VarExporter\Internal\Registry::$factories['Error'] ?? \Symfony\Component\VarExporter\Internal\Registry::f('Error'))(),
66
],
77
null,
88
[
9-
\TypeError::class => [
9+
'TypeError' => [
1010
'file' => [
1111
\dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php',
1212
],
1313
'line' => [
1414
234,
1515
],
1616
],
17-
\Error::class => [
17+
'Error' => [
1818
'trace' => [
1919
[
2020
'file' => \dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php',

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
]),
77
null,
88
[
9-
\TypeError::class => [
9+
'TypeError' => [
1010
'file' => [
1111
\dirname(__DIR__).\DIRECTORY_SEPARATOR.'VarExporterTest.php',
1212
],

0 commit comments

Comments
 (0)