Skip to content

Commit bc3f595

Browse files
committed
Walk bare $ref chains iteratively, not by mutual recursion
A chain of {"$ref": ...} nodes is meant to cost nothing regardless of length, but cost()/refCost() resolved it by mutual recursion - one native call frame per link. A chain long enough (~17-20k links, well within default maxProperties/maxDepth combined across sibling maps) exhausted the stack or its backing memory before the subschema budget or depth ceiling ever got a chance to refuse it: the guard was bypassable by the exact class of input it exists to stop. refCost() now walks the chain in a loop at constant stack depth, handing only the schema found at the end of it to cost() for its own already depth-bounded recursion.
1 parent 802b24e commit bc3f595

2 files changed

Lines changed: 75 additions & 20 deletions

File tree

src/Capability/Discovery/SchemaComplexityGuard.php

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -171,35 +171,69 @@ private function cost(array $node, array $root, array $stack, int $depth, object
171171
}
172172

173173
/**
174+
* Chases a same-document `$ref`, and every bare `$ref` it in turn points
175+
* to, without recursing: a node that is only `{"$ref": ...}` contributes
176+
* nothing of its own, so a schema chaining many of them (a "flat" `$defs`
177+
* indirection) is meant to be free regardless of length. Resolving that
178+
* chain by mutual recursion with {@see cost()} spent one native call
179+
* frame per link, so a chain long enough — a size none of the other
180+
* bounds catch, since a chain's cost is deliberately independent of its
181+
* length — exhausted the stack or the memory backing it before this
182+
* class ever got to refuse anything. Walking the chain in a loop keeps
183+
* this at constant stack depth; only the schema found at the end of it,
184+
* if any, is handed to cost() for its own depth-bounded recursion.
185+
*
174186
* @param array<string, mixed> $root
175-
* @param list<string> $stack
187+
* @param list<string> $stack pointers being resolved by an enclosing call
176188
*/
177189
private function refCost(string $pointer, array $root, array $stack, int $depth, object $memo): int
178190
{
179-
// A back-edge: recursive schemas are legitimate, and how far one
180-
// unrolls is decided by the data, not the schema.
181-
if (\in_array($pointer, $stack, true)) {
182-
return 1;
183-
}
191+
$visited = [];
184192

185-
if (isset($memo->{$pointer})) {
186-
return $memo->{$pointer};
187-
}
193+
while (true) {
194+
// A back-edge: recursive schemas are legitimate, and how far one
195+
// unrolls is decided by the data, not the schema.
196+
if (\in_array($pointer, $stack, true) || isset($visited[$pointer])) {
197+
return $this->memoizeAll($visited, 1, $memo);
198+
}
199+
200+
if (isset($memo->{$pointer})) {
201+
return $this->memoizeAll($visited, $memo->{$pointer}, $memo);
202+
}
203+
204+
$target = self::resolve($pointer, $root);
205+
206+
if (null === $target) {
207+
// Unresolvable same-document pointers are the validator's
208+
// business to report; nothing here can be expensive.
209+
return $this->memoizeAll($visited, 1, $memo);
210+
}
188211

189-
$target = self::resolve($pointer, $root);
212+
$visited[$pointer] = true;
190213

191-
if (null === $target) {
192-
// Unresolvable same-document pointers are the validator's business
193-
// to report; nothing here can be expensive.
194-
return 1;
214+
if (!isset($target['$ref']) || !\is_string($target['$ref'])) {
215+
// Depth is lexical nesting, which following a reference is
216+
// not: a long chain of `$defs` referring to one another is
217+
// flat and cheap. What bounds this is the subschema budget
218+
// and the cycle check above, and the pointer set is finite,
219+
// so the walk is too.
220+
$cost = $this->cost($target, $root, [...$stack, ...array_keys($visited)], $depth, $memo);
221+
222+
return $this->memoizeAll($visited, $cost, $memo);
223+
}
224+
225+
$pointer = $target['$ref'];
195226
}
227+
}
196228

197-
// Depth is lexical nesting, which following a reference is not: a long
198-
// chain of `$defs` referring to one another is flat and cheap. What
199-
// bounds this is the subschema budget and the cycle check above, and
200-
// the pointer set is finite, so the recursion is too.
201-
$cost = $this->cost($target, $root, [...$stack, $pointer], $depth, $memo);
202-
$memo->{$pointer} = $cost;
229+
/**
230+
* @param array<string, true> $pointers
231+
*/
232+
private function memoizeAll(array $pointers, int $cost, object $memo): int
233+
{
234+
foreach ($pointers as $pointer => $_) {
235+
$memo->{$pointer} = $cost;
236+
}
203237

204238
return $cost;
205239
}

tests/Unit/Capability/Discovery/SchemaComplexityGuardTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,27 @@ public function testLongLocalRefChainIsAllowed(): void
160160
$this->assertNull($this->guard->check(['$defs' => $defs, '$ref' => '#/$defs/a60']));
161161
}
162162

163+
#[TestDox('a long chain of bare $refs is walked without recursing per link')]
164+
public function testLongLocalRefChainDoesNotRecursePerLink(): void
165+
{
166+
// Bare {"$ref": ...} nodes chained together are meant to be free
167+
// regardless of length, and used to be resolved by mutual recursion
168+
// between cost() and refCost(): one native call frame per link. A
169+
// chain long enough exhausted the stack, or the memory backing it,
170+
// long before the subschema budget below ever got a chance to fire —
171+
// 20,000 links reliably faulted with the old implementation. This
172+
// uses a guard with a raised budget so the chain is not refused for
173+
// an unrelated reason, and asserts it resolves at all.
174+
$defs = ['a0' => ['type' => 'string']];
175+
for ($i = 1; $i < 20_000; ++$i) {
176+
$defs['a'.$i] = ['$ref' => '#/$defs/a'.($i - 1)];
177+
}
178+
179+
$guard = new SchemaComplexityGuard(maxSubschemas: 1_000_000, maxProperties: 1_000_000);
180+
181+
$this->assertNull($guard->check(['$defs' => $defs, '$ref' => '#/$defs/a19999']));
182+
}
183+
163184
#[TestDox('a recursive schema is allowed: how far it unrolls is the data\'s doing')]
164185
public function testRecursiveSchemaIsAllowed(): void
165186
{

0 commit comments

Comments
 (0)