Skip to content

Commit 5766ba6

Browse files
jnvsorjnvsor
committed
Parser: Use try/finally to unset recursion stacks
I was concerned this would cost a few % performance but individual tests fluctuate more than the difference so it's negligable at best
1 parent 1fcde3e commit 5766ba6

File tree

3 files changed

+162
-123
lines changed

3 files changed

+162
-123
lines changed

‎build/kint.phar

350 Bytes
Binary file not shown.

‎src/Parser/Parser.php

Lines changed: 116 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -265,45 +265,41 @@ private function parseArray(array &$var, ContextInterface $c): AbstractValue
265265
return $this->applyPluginsComplete($var, $array, self::TRIGGER_RECURSION);
266266
}
267267

268-
$this->array_ref_stack[$parentRef] = true;
268+
try {
269+
$this->array_ref_stack[$parentRef] = true;
269270

270-
$cdepth = $c->getDepth();
271-
$ap = $c->getAccessPath();
271+
$cdepth = $c->getDepth();
272+
$ap = $c->getAccessPath();
272273

273-
if ($size > 0 && $this->depth_limit && $cdepth >= $this->depth_limit) {
274-
$array = new ArrayValue($c, $size, $contents);
275-
$array->flags |= AbstractValue::FLAG_DEPTH_LIMIT;
276-
277-
$array = $this->applyPluginsComplete($var, $array, self::TRIGGER_DEPTH_LIMIT);
274+
if ($size > 0 && $this->depth_limit && $cdepth >= $this->depth_limit) {
275+
$array = new ArrayValue($c, $size, $contents);
276+
$array->flags |= AbstractValue::FLAG_DEPTH_LIMIT;
278277

279-
unset($this->array_ref_stack[$parentRef]);
278+
return $this->applyPluginsComplete($var, $array, self::TRIGGER_DEPTH_LIMIT);
279+
}
280280

281-
return $array;
282-
}
281+
foreach ($var as $key => $_) {
282+
$child = new ArrayContext($key);
283+
$child->depth = $cdepth + 1;
284+
$child->reference = null !== ReflectionReference::fromArrayElement($var, $key);
283285

284-
foreach ($var as $key => $_) {
285-
$child = new ArrayContext($key);
286-
$child->depth = $cdepth + 1;
287-
$child->reference = null !== ReflectionReference::fromArrayElement($var, $key);
286+
if (null !== $ap) {
287+
$child->access_path = $ap.'['.\var_export($key, true).']';
288+
}
288289

289-
if (null !== $ap) {
290-
$child->access_path = $ap.'['.\var_export($key, true).']';
290+
$contents[$key] = $this->parse($var[$key], $child);
291291
}
292292

293-
$contents[$key] = $this->parse($var[$key], $child);
294-
}
293+
$array = new ArrayValue($c, $size, $contents);
295294

296-
$array = new ArrayValue($c, $size, $contents);
295+
if ($contents) {
296+
$array->addRepresentation(new ContainerRepresentation('Contents', $contents, null, true));
297+
}
297298

298-
if ($contents) {
299-
$array->addRepresentation(new ContainerRepresentation('Contents', $contents, null, true));
299+
return $this->applyPluginsComplete($var, $array, self::TRIGGER_SUCCESS);
300+
} finally {
301+
unset($this->array_ref_stack[$parentRef]);
300302
}
301-
302-
$array = $this->applyPluginsComplete($var, $array, self::TRIGGER_SUCCESS);
303-
304-
unset($this->array_ref_stack[$parentRef]);
305-
306-
return $array;
307303
}
308304

309305
/**
@@ -374,129 +370,126 @@ private function parseObject(object &$var, ContextInterface $c): AbstractValue
374370
return $this->applyPluginsComplete($var, $object, self::TRIGGER_RECURSION);
375371
}
376372

377-
$this->object_hashes[$hash] = true;
378-
379-
$cdepth = $c->getDepth();
380-
$ap = $c->getAccessPath();
381-
382-
if ($this->depth_limit && $cdepth >= $this->depth_limit) {
383-
$object = new InstanceValue($c, $classname, $hash, \spl_object_id($var));
384-
$object->flags |= AbstractValue::FLAG_DEPTH_LIMIT;
373+
try {
374+
$this->object_hashes[$hash] = true;
385375

386-
$object = $this->applyPluginsComplete($var, $object, self::TRIGGER_DEPTH_LIMIT);
376+
$cdepth = $c->getDepth();
377+
$ap = $c->getAccessPath();
387378

388-
unset($this->object_hashes[$hash]);
379+
if ($this->depth_limit && $cdepth >= $this->depth_limit) {
380+
$object = new InstanceValue($c, $classname, $hash, \spl_object_id($var));
381+
$object->flags |= AbstractValue::FLAG_DEPTH_LIMIT;
389382

390-
return $object;
391-
}
392-
393-
if (KINT_PHP81) {
394-
$props = $this->getPropsOrdered(new ReflectionObject($var));
395-
} else {
396-
$props = $this->getPropsOrderedOld(new ReflectionObject($var)); // @codeCoverageIgnore
397-
}
398-
399-
$values = (array) $var;
400-
$properties = [];
401-
402-
foreach ($props as $rprop) {
403-
$rprop->setAccessible(true);
404-
$name = $rprop->getName();
405-
406-
// Casting object to array:
407-
// private properties show in the form "\0$owner_class_name\0$property_name";
408-
// protected properties show in the form "\0*\0$property_name";
409-
// public properties show in the form "$property_name";
410-
// http://www.php.net/manual/en/language.types.array.php#language.types.array.casting
411-
$key = $name;
412-
if ($rprop->isProtected()) {
413-
$key = "\0*\0".$name;
414-
} elseif ($rprop->isPrivate()) {
415-
$key = "\0".$rprop->getDeclaringClass()->getName()."\0".$name;
383+
return $this->applyPluginsComplete($var, $object, self::TRIGGER_DEPTH_LIMIT);
416384
}
417-
$initialized = \array_key_exists($key, $values);
418-
if ($key === (string) (int) $key) {
419-
$key = (int) $key;
385+
386+
if (KINT_PHP81) {
387+
$props = $this->getPropsOrdered(new ReflectionObject($var));
388+
} else {
389+
$props = $this->getPropsOrderedOld(new ReflectionObject($var)); // @codeCoverageIgnore
420390
}
421391

422-
if ($rprop->isDefault()) {
423-
$child = new PropertyContext(
424-
$name,
425-
$rprop->getDeclaringClass()->getName(),
426-
ClassDeclaredContext::ACCESS_PUBLIC
427-
);
392+
$values = (array) $var;
393+
$properties = [];
428394

429-
$child->readonly = KINT_PHP81 && $rprop->isReadOnly();
395+
foreach ($props as $rprop) {
396+
$rprop->setAccessible(true);
397+
$name = $rprop->getName();
430398

399+
// Casting object to array:
400+
// private properties show in the form "\0$owner_class_name\0$property_name";
401+
// protected properties show in the form "\0*\0$property_name";
402+
// public properties show in the form "$property_name";
403+
// http://www.php.net/manual/en/language.types.array.php#language.types.array.casting
404+
$key = $name;
431405
if ($rprop->isProtected()) {
432-
$child->access = ClassDeclaredContext::ACCESS_PROTECTED;
406+
$key = "\0*\0".$name;
433407
} elseif ($rprop->isPrivate()) {
434-
$child->access = ClassDeclaredContext::ACCESS_PRIVATE;
408+
$key = "\0".$rprop->getDeclaringClass()->getName()."\0".$name;
409+
}
410+
$initialized = \array_key_exists($key, $values);
411+
if ($key === (string) (int) $key) {
412+
$key = (int) $key;
435413
}
436414

437-
if (KINT_PHP84) {
438-
if ($rprop->isProtectedSet()) {
439-
$child->access_set = ClassDeclaredContext::ACCESS_PROTECTED;
440-
} elseif ($rprop->isPrivateSet()) {
441-
$child->access_set = ClassDeclaredContext::ACCESS_PRIVATE;
415+
if ($rprop->isDefault()) {
416+
$child = new PropertyContext(
417+
$name,
418+
$rprop->getDeclaringClass()->getName(),
419+
ClassDeclaredContext::ACCESS_PUBLIC
420+
);
421+
422+
$child->readonly = KINT_PHP81 && $rprop->isReadOnly();
423+
424+
if ($rprop->isProtected()) {
425+
$child->access = ClassDeclaredContext::ACCESS_PROTECTED;
426+
} elseif ($rprop->isPrivate()) {
427+
$child->access = ClassDeclaredContext::ACCESS_PRIVATE;
442428
}
443429

444-
$hooks = $rprop->getHooks();
445-
if (isset($hooks['get'])) {
446-
$child->hooks |= PropertyContext::HOOK_GET;
447-
if ($hooks['get']->returnsReference()) {
448-
$child->hooks |= PropertyContext::HOOK_GET_REF;
430+
if (KINT_PHP84) {
431+
if ($rprop->isProtectedSet()) {
432+
$child->access_set = ClassDeclaredContext::ACCESS_PROTECTED;
433+
} elseif ($rprop->isPrivateSet()) {
434+
$child->access_set = ClassDeclaredContext::ACCESS_PRIVATE;
449435
}
450-
}
451-
if (isset($hooks['set'])) {
452-
$child->hooks |= PropertyContext::HOOK_SET;
453-
454-
$child->hook_set_type = (string) $rprop->getSettableType();
455-
if ($child->hook_set_type !== (string) $rprop->getType()) {
456-
$child->hooks |= PropertyContext::HOOK_SET_TYPE;
457-
} elseif ('' === $child->hook_set_type) {
458-
$child->hook_set_type = null;
436+
437+
$hooks = $rprop->getHooks();
438+
if (isset($hooks['get'])) {
439+
$child->hooks |= PropertyContext::HOOK_GET;
440+
if ($hooks['get']->returnsReference()) {
441+
$child->hooks |= PropertyContext::HOOK_GET_REF;
442+
}
443+
}
444+
if (isset($hooks['set'])) {
445+
$child->hooks |= PropertyContext::HOOK_SET;
446+
447+
$child->hook_set_type = (string) $rprop->getSettableType();
448+
if ($child->hook_set_type !== (string) $rprop->getType()) {
449+
$child->hooks |= PropertyContext::HOOK_SET_TYPE;
450+
} elseif ('' === $child->hook_set_type) {
451+
$child->hook_set_type = null;
452+
}
459453
}
460454
}
455+
} else {
456+
$child = new ClassOwnedContext($name, $rprop->getDeclaringClass()->getName());
461457
}
462-
} else {
463-
$child = new ClassOwnedContext($name, $rprop->getDeclaringClass()->getName());
464-
}
465458

466-
$child->reference = $initialized && null !== ReflectionReference::fromArrayElement($values, $key);
467-
$child->depth = $cdepth + 1;
459+
$child->reference = $initialized && null !== ReflectionReference::fromArrayElement($values, $key);
460+
$child->depth = $cdepth + 1;
468461

469-
if (null !== $ap && $child->isAccessible($this->caller_class)) {
470-
/** @psalm-var string $child->name */
471-
if (Utils::isValidPhpName($child->name)) {
472-
$child->access_path = $ap.'->'.$child->name;
462+
if (null !== $ap && $child->isAccessible($this->caller_class)) {
463+
/** @psalm-var string $child->name */
464+
if (Utils::isValidPhpName($child->name)) {
465+
$child->access_path = $ap.'->'.$child->name;
466+
} else {
467+
$child->access_path = $ap.'->{'.\var_export($child->name, true).'}';
468+
}
469+
}
470+
471+
if (KINT_PHP84 && $rprop->isVirtual()) {
472+
$properties[] = new VirtualValue($child);
473+
} elseif (!$initialized) {
474+
$properties[] = new UninitializedValue($child);
473475
} else {
474-
$child->access_path = $ap.'->{'.\var_export($child->name, true).'}';
476+
$properties[] = $this->parse($values[$key], $child);
475477
}
476478
}
477479

478-
if (KINT_PHP84 && $rprop->isVirtual()) {
479-
$properties[] = new VirtualValue($child);
480-
} elseif (!$initialized) {
481-
$properties[] = new UninitializedValue($child);
482-
} else {
483-
$properties[] = $this->parse($values[$key], $child);
480+
$object = new InstanceValue($c, $classname, $hash, \spl_object_id($var));
481+
if ($props) {
482+
$object->setChildren($properties);
484483
}
485-
}
486484

487-
$object = new InstanceValue($c, $classname, $hash, \spl_object_id($var));
488-
if ($props) {
489-
$object->setChildren($properties);
490-
}
485+
if ($properties) {
486+
$object->addRepresentation(new ContainerRepresentation('Properties', $properties));
487+
}
491488

492-
if ($properties) {
493-
$object->addRepresentation(new ContainerRepresentation('Properties', $properties));
489+
return $this->applyPluginsComplete($var, $object, self::TRIGGER_SUCCESS);
490+
} finally {
491+
unset($this->object_hashes[$hash]);
494492
}
495-
496-
$object = $this->applyPluginsComplete($var, $object, self::TRIGGER_SUCCESS);
497-
unset($this->object_hashes[$hash]);
498-
499-
return $object;
500493
}
501494

502495
/**

‎tests/Parser/ParserTest.php

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1425,4 +1425,50 @@ function () {
14251425

14261426
$this->fail("Didn't get warning");
14271427
}
1428+
1429+
public function testPluginsExceptionCleansUpRecursionMarkers()
1430+
{
1431+
$p = new Parser();
1432+
$b = new BaseContext('$v');
1433+
$v = new stdClass();
1434+
$v->v = $v;
1435+
1436+
$pl = new ProxyPlugin(
1437+
['object', 'array'],
1438+
Parser::TRIGGER_COMPLETE,
1439+
static function (&$var, AbstractValue $v) {
1440+
if ($v->getContext()->getDepth()) {
1441+
throw new Exception();
1442+
}
1443+
}
1444+
);
1445+
$p->addPlugin($pl);
1446+
1447+
try {
1448+
$o = $p->parse($v, clone $b);
1449+
} catch (Exception $e) {
1450+
}
1451+
1452+
$p->clearPlugins();
1453+
1454+
$o = $p->parse($v, clone $b);
1455+
$this->assertEquals(false, $o->flags & AbstractValue::FLAG_RECURSION);
1456+
$this->assertEquals(true, $o->getChildren()[0]->flags & AbstractValue::FLAG_RECURSION);
1457+
1458+
$v = [];
1459+
$v[] = &$v;
1460+
1461+
$p->addPlugin($pl);
1462+
1463+
try {
1464+
$o = $p->parse($v, clone $b);
1465+
} catch (Exception $e) {
1466+
}
1467+
1468+
$p->clearPlugins();
1469+
1470+
$o = $p->parse($v, clone $b);
1471+
$this->assertEquals(false, $o->flags & AbstractValue::FLAG_RECURSION);
1472+
$this->assertEquals(true, $o->getContents()[0]->flags & AbstractValue::FLAG_RECURSION);
1473+
}
14281474
}

0 commit comments

Comments
 (0)