From 17d7dd28953f3d3807e3313a627a485e7d00b911 Mon Sep 17 00:00:00 2001 From: benjamineckstein <13351939+benjamineckstein@users.noreply.github.com> Date: Sat, 13 Jun 2026 22:56:22 +0200 Subject: [PATCH 1/2] feat(parser): include JSON pointer and expected shape in spec parse errors Thread an RFC 6901 JSON pointer through schema hydration so the depth-bound (#107 sibling) and node-count guards name WHERE in the document the offending node sits, e.g. 'at #/components/schemas/Deep/properties/next/...'. The pointer is escaped (~ -> ~0, / -> ~1) and degrades to no-location cleanly when the reader cannot pinpoint the node. The structural rejections (missing or mistyped openapi version, missing or mistyped info) now state the pointer, the expected-vs-found shape (a string, a number, an object, missing), and an actionable hint. The depth-bound and node-count messages also name the source file and point at --max-depth / max_depth. The pointer only ever appears in exception text, never in generated output. --- src/Parser/OpenApiReader.php | 120 +++++++++++++++++++----- tests/Unit/Parser/OpenApiReaderTest.php | 68 ++++++++++---- 2 files changed, 146 insertions(+), 42 deletions(-) diff --git a/src/Parser/OpenApiReader.php b/src/Parser/OpenApiReader.php index 218e172..41b6b50 100644 --- a/src/Parser/OpenApiReader.php +++ b/src/Parser/OpenApiReader.php @@ -96,6 +96,14 @@ final class OpenApiReader public const SUPPORTED_MATRIX = 'Supported versions: OpenAPI 3.0.x and 3.1.x (fully), 3.2.x (accepted best-effort with warnings). See '.self::VERSION_MATRIX_URL; + /** + * The human-readable label for the spec source supplied to read(), captured + * for the current read() so the deep hydration helpers (which thread a JSON + * pointer but not the source) can name the file in their error messages + * without widening every signature. Reset at the start of each read(). + */ + private string $source = 'spec'; + /** * Numeric schema keywords whose strictly-numeric string values are coerced * to int/float (issue #32). Kept here as the @@ -142,21 +150,24 @@ public function read(mixed $data, string $source = 'spec'): OpenApiDocument } $this->nodeCount = 0; + $this->source = $source; $version = $data['openapi'] ?? null; if (! is_string($version) || $version === '') { - throw new ParseException("Not an OpenAPI 3.x document ({$source}): missing 'openapi' version string. Swagger 2.0 and other formats are not supported. ".self::SUPPORTED_MATRIX); + $found = $this->describeType($version); + throw new ParseException("Not an OpenAPI 3.x document ({$source}): the root '#/openapi' member must be a version string like '3.1.0', but {$found}. Swagger 2.0 (a 'swagger' key) and other formats are not supported. ".self::SUPPORTED_MATRIX); } if (preg_match('/^3\.(\d+)(?:[.\-+]|$)/', $version, $matches) !== 1 || ! in_array((int) $matches[1], [0, 1, 2], true)) { - throw new ParseException("Unsupported OpenAPI version '{$version}' ({$source}). ".self::SUPPORTED_MATRIX); + throw new ParseException("Unsupported OpenAPI version '{$version}' at #/openapi ({$source}). ".self::SUPPORTED_MATRIX); } $rawInfo = $data['info'] ?? null; if (! is_array($rawInfo)) { - throw new ParseException("Not a valid OpenAPI document ({$source}): missing required 'info' object."); + $found = $this->describeType($rawInfo); + throw new ParseException("Not a valid OpenAPI document ({$source}): the required '#/info' object is {$found}. Add an 'info' object with at least a 'title' and a 'version'."); } $warnings = []; @@ -494,7 +505,7 @@ private function components(array $raw): ComponentsNode $rawSchemas = $raw['schemas'] ?? null; if (is_array($rawSchemas)) { foreach ($rawSchemas as $name => $value) { - $node = $this->subschema($value, 0); + $node = $this->subschema($value, 0, '#/components/schemas/'.$this->escapePointer((string) $name)); if ($node !== null) { $schemas[(string) $name] = $node; } @@ -580,7 +591,7 @@ private function components(array $raw): ComponentsNode * ("nothing"). Anything else (a mistyped value) returns null and the * caller routes the raw value to its extra bag or skips the entry. */ - private function subschema(mixed $value, int $depth): SchemaNode|ReferenceNode|null + private function subschema(mixed $value, int $depth, string $pointer = ''): SchemaNode|ReferenceNode|null { if (is_bool($value)) { return $value ? new SchemaNode : new SchemaNode(not: new SchemaNode); @@ -592,7 +603,7 @@ private function subschema(mixed $value, int $depth): SchemaNode|ReferenceNode|n return $this->isReference($value) ? $this->reference($value) - : $this->schema($value, $depth); + : $this->schema($value, $depth, $pointer); } /** @@ -604,10 +615,12 @@ private function subschema(mixed $value, int $depth): SchemaNode|ReferenceNode|n * * @param array $raw */ - private function schema(array $raw, int $depth): SchemaNode + private function schema(array $raw, int $depth, string $pointer = ''): SchemaNode { + $at = $this->locationSuffix($pointer); + if ($depth > $this->maxDepth) { - throw new ParseException("OpenAPI document exceeds the maximum schema nesting depth ({$this->maxDepth})."); + throw new ParseException("OpenAPI document ({$this->source}) exceeds the maximum schema nesting depth ({$this->maxDepth}){$at}. Flatten the nesting, factor the deep shape into a component schema referenced by \$ref, or raise the bound via --max-depth / the max_depth config key only for a trusted spec."); } // Total-node guard (issue #107): YAML alias amplification can fan a @@ -616,7 +629,7 @@ private function schema(array $raw, int $depth): SchemaNode // node and failing closed here bounds that breadth blowup before it // exhausts memory. if (++$this->nodeCount > $this->maxNodes) { - throw new ParseException("OpenAPI document exceeds the maximum hydrated schema node count ({$this->maxNodes}). This usually means the spec uses YAML anchors and aliases to amplify a small file into a very large structure. Override the bound via the constructor only for a trusted spec."); + throw new ParseException("OpenAPI document ({$this->source}) exceeds the maximum hydrated schema node count ({$this->maxNodes}){$at}. This usually means the spec uses YAML anchors and aliases to amplify a small file into a very large structure. Override the bound via the constructor only for a trusted spec."); } // Closed tuple (issue #82): `items: false` next to a non-empty @@ -785,10 +798,10 @@ private function schema(array $raw, int $depth): SchemaNode } break; case 'properties': - $properties = $this->schemaMap($value, $depth, $extra, 'properties'); + $properties = $this->schemaMap($value, $depth, $extra, 'properties', $pointer); break; case 'patternProperties': - $patternProperties = $this->schemaMap($value, $depth, $extra, 'patternProperties'); + $patternProperties = $this->schemaMap($value, $depth, $extra, 'patternProperties', $pointer); break; case 'additionalProperties': if (is_bool($value)) { @@ -796,7 +809,7 @@ private function schema(array $raw, int $depth): SchemaNode $hasAdditionalProperties = true; break; } - $node = $this->subschema($value, $depth + 1); + $node = $this->subschema($value, $depth + 1, $this->childPointer($pointer, 'additionalProperties')); if ($node === null) { $extra['additionalProperties'] = $value; } else { @@ -807,7 +820,7 @@ private function schema(array $raw, int $depth): SchemaNode case 'allOf': case 'oneOf': case 'anyOf': - $list = $this->schemaList($value, $depth); + $list = $this->schemaList($value, $depth, $this->childPointer($pointer, (string) $key)); if ($list === null) { $extra[(string) $key] = $value; } else { @@ -821,7 +834,7 @@ private function schema(array $raw, int $depth): SchemaNode // empty placeholder node (which emits no rules, exactly // like the skipped null position on the old cebe path) // instead of being dropped. - $list = $this->prefixItemList($value, $depth); + $list = $this->prefixItemList($value, $depth, $this->childPointer($pointer, 'prefixItems')); if ($list === null) { $extra['prefixItems'] = $value; } else { @@ -829,7 +842,7 @@ private function schema(array $raw, int $depth): SchemaNode } break; case 'not': - $not = $this->subschema($value, $depth + 1); + $not = $this->subschema($value, $depth + 1, $this->childPointer($pointer, 'not')); if ($not === null) { $extra['not'] = $value; } @@ -841,7 +854,7 @@ private function schema(array $raw, int $depth): SchemaNode if ($value === true) { $items = new SchemaNode; } elseif ($value !== false) { - $items = $this->subschema($value, $depth + 1); + $items = $this->subschema($value, $depth + 1, $this->childPointer($pointer, 'items')); if ($items === null) { $extra['items'] = $value; } @@ -946,7 +959,7 @@ enum: $enum, * @param array $extra * @return array|null */ - private function schemaMap(mixed $value, int $depth, array &$extra, string $key): ?array + private function schemaMap(mixed $value, int $depth, array &$extra, string $key, string $pointer = ''): ?array { if (! is_array($value)) { $extra[$key] = $value; @@ -954,9 +967,11 @@ private function schemaMap(mixed $value, int $depth, array &$extra, string $key) return null; } + $base = $this->childPointer($pointer, $key); + $map = []; foreach ($value as $name => $entry) { - $node = $this->subschema($entry, $depth + 1); + $node = $this->subschema($entry, $depth + 1, $this->childPointer($base, (string) $name)); if ($node !== null) { $map[(string) $name] = $node; } @@ -972,15 +987,15 @@ private function schemaMap(mixed $value, int $depth, array &$extra, string $key) * * @return list|null */ - private function schemaList(mixed $value, int $depth): ?array + private function schemaList(mixed $value, int $depth, string $pointer = ''): ?array { if (! is_array($value) || ! array_is_list($value)) { return null; } $list = []; - foreach ($value as $entry) { - $node = $this->subschema($entry, $depth + 1); + foreach ($value as $index => $entry) { + $node = $this->subschema($entry, $depth + 1, $this->childPointer($pointer, (string) $index)); if ($node !== null) { $list[] = $node; } @@ -1000,15 +1015,15 @@ private function schemaList(mixed $value, int $depth): ?array * * @return list|null */ - private function prefixItemList(mixed $value, int $depth): ?array + private function prefixItemList(mixed $value, int $depth, string $pointer = ''): ?array { if (! is_array($value) || ! array_is_list($value)) { return null; } $list = []; - foreach ($value as $entry) { - $list[] = $this->subschema($entry, $depth + 1) ?? new SchemaNode; + foreach ($value as $index => $entry) { + $list[] = $this->subschema($entry, $depth + 1, $this->childPointer($pointer, (string) $index)) ?? new SchemaNode; } return $list; @@ -1282,4 +1297,61 @@ private function numericFromString(string $value): int|float return $float; } + + /** + * Append a single member or array index to a JSON pointer (RFC 6901), + * escaping the segment. An empty parent pointer (a schema reached from a + * position the reader does not pinpoint, e.g. an inline parameter or media + * type schema) yields an empty pointer too, so the message degrades to "no + * location" cleanly rather than producing a misleading partial pointer. + */ + private function childPointer(string $parent, string $segment): string + { + if ($parent === '') { + return ''; + } + + return $parent.'/'.$this->escapePointer($segment); + } + + /** + * Escape a single JSON pointer reference token (RFC 6901): `~` becomes `~0` + * and `/` becomes `~1`, in that order, so a property or component name that + * contains either character produces a valid, unambiguous pointer. + */ + private function escapePointer(string $segment): string + { + return str_replace(['~', '/'], ['~0', '~1'], $segment); + } + + /** + * Render the ` at ` location suffix for an error message, or the + * empty string when the reader could not pinpoint the offending node (the + * pointer is threaded only from positions the reader tracks). Keeping the + * caller's message identical when there is no pointer avoids a dangling + * "at " with nothing after it. + */ + private function locationSuffix(string $pointer): string + { + return $pointer === '' ? '' : " at {$pointer}"; + } + + /** + * A short human-readable description of a found value's type, for the + * expected-vs-found half of a structural-rejection message: "missing" + * (null/absent), "a string", "a number", "a boolean", "an array", or "an + * object". Never echoes the value itself, which is untrusted input. + */ + private function describeType(mixed $value): string + { + return match (true) { + $value === null => 'missing', + is_string($value) => 'a string', + is_int($value) || is_float($value) => 'a number', + is_bool($value) => 'a boolean', + is_array($value) && array_is_list($value) => 'an array', + is_array($value) => 'an object', + default => 'an unexpected value', + }; + } } diff --git a/tests/Unit/Parser/OpenApiReaderTest.php b/tests/Unit/Parser/OpenApiReaderTest.php index b59eb3d..93c19be 100644 --- a/tests/Unit/Parser/OpenApiReaderTest.php +++ b/tests/Unit/Parser/OpenApiReaderTest.php @@ -46,26 +46,37 @@ function readSchema(array|bool $schema): SchemaNode|ReferenceNode // --- Structural rejection and version gating (#103 parity) ----------------- -it('rejects non-array input as a missing openapi version string', function () { - (new OpenApiReader)->read('not a document', 'bad.yaml'); -})->throws(ParseException::class, "missing 'openapi' version string"); +it('rejects non-array input as a missing openapi version string, naming the pointer and expectation', function () { + expect(fn () => (new OpenApiReader)->read('not a document', 'bad.yaml')) + ->toThrow(ParseException::class, 'Not an OpenAPI 3.x document (bad.yaml)') + ->toThrow(ParseException::class, "the root '#/openapi' member must be a version string like '3.1.0'"); +}); + +it('names the found type in the missing-openapi message', function () { + expect(fn () => (new OpenApiReader)->read(['openapi' => 30, 'info' => []], 's')) + ->toThrow(ParseException::class, 'but a number') + ->toThrow(ParseException::class, "Swagger 2.0 (a 'swagger' key)"); +}); it('rejects a document without an openapi key, naming the supported matrix', function () { (new OpenApiReader)->read(['swagger' => '2.0'], 'swagger.json'); })->throws(ParseException::class, 'Supported versions: OpenAPI 3.0.x and 3.1.x'); -it('rejects unsupported versions exactly', function (string $version) { +it('rejects unsupported versions exactly, naming the #/openapi pointer', function (string $version) { expect(fn () => (new OpenApiReader)->read(['openapi' => $version, 'info' => ['title' => 'T', 'version' => '1']], 's')) - ->toThrow(ParseException::class, "Unsupported OpenAPI version '{$version}'"); + ->toThrow(ParseException::class, "Unsupported OpenAPI version '{$version}' at #/openapi (s)"); })->with(['2.0', '3.3.0', '4.0.0', '3.10.1', '30.0.0']); -it('rejects a document missing the info object', function () { - (new OpenApiReader)->read(['openapi' => '3.1.0'], 's'); -})->throws(ParseException::class, "missing required 'info' object"); +it('rejects a document missing the info object, naming the pointer and a hint', function () { + expect(fn () => (new OpenApiReader)->read(['openapi' => '3.1.0'], 's')) + ->toThrow(ParseException::class, "the required '#/info' object is missing") + ->toThrow(ParseException::class, "Add an 'info' object with at least a 'title' and a 'version'"); +}); -it('rejects a mistyped info value as a missing info object', function () { - (new OpenApiReader)->read(['openapi' => '3.1.0', 'info' => 'Petstore'], 's'); -})->throws(ParseException::class, "missing required 'info' object"); +it('rejects a mistyped info value naming the found type', function () { + expect(fn () => (new OpenApiReader)->read(['openapi' => '3.1.0', 'info' => 'Petstore'], 's')) + ->toThrow(ParseException::class, "the required '#/info' object is a string"); +}); it('accepts every supported version band without warnings', function (string $version) { $document = (new OpenApiReader)->read(['openapi' => $version, 'info' => ['title' => 'T', 'version' => '1']], 's'); @@ -667,18 +678,36 @@ function readSchema(array|bool $schema): SchemaNode|ReferenceNode // --- Depth bound ------------------------------------------------------------- -it('rejects schema nesting beyond the configured depth bound', function () { +it('rejects schema nesting beyond the configured depth bound, naming the source and pointer', function () { $schema = ['type' => 'string']; for ($i = 0; $i < 12; $i++) { $schema = ['type' => 'object', 'properties' => ['next' => $schema]]; } - (new OpenApiReader(maxDepth: 10))->read([ + expect(fn () => (new OpenApiReader(maxDepth: 10))->read([ 'openapi' => '3.1.0', 'info' => ['title' => 'T', 'version' => '1'], 'components' => ['schemas' => ['Deep' => $schema]], - ], 's'); -})->throws(ParseException::class, 'maximum schema nesting depth (10)'); + ], 'deep.yaml')) + ->toThrow(ParseException::class, 'maximum schema nesting depth (10)') + ->toThrow(ParseException::class, '(deep.yaml)') + ->toThrow(ParseException::class, 'at #/components/schemas/Deep/properties/next') + ->toThrow(ParseException::class, '--max-depth'); +}); + +it('escapes slashes and tildes in the JSON pointer of a depth-bound error', function () { + $schema = ['type' => 'string']; + for ($i = 0; $i < 4; $i++) { + $schema = ['type' => 'object', 'properties' => ['a/b~c' => $schema]]; + } + + expect(fn () => (new OpenApiReader(maxDepth: 2))->read([ + 'openapi' => '3.1.0', + 'info' => ['title' => 'T', 'version' => '1'], + 'components' => ['schemas' => ['Deep' => $schema]], + ], 's')) + ->toThrow(ParseException::class, 'a~1b~0c'); +}); it('hydrates nesting within the depth bound', function () { $schema = ['type' => 'string']; @@ -711,12 +740,15 @@ function readSchema(array|bool $schema): SchemaNode|ReferenceNode ]; } - (new OpenApiReader(maxNodes: 20))->read([ + expect(fn () => (new OpenApiReader(maxNodes: 20))->read([ 'openapi' => '3.1.0', 'info' => ['title' => 'T', 'version' => '1'], 'components' => ['schemas' => $schemas], - ], 's'); -})->throws(ParseException::class, 'maximum hydrated schema node count (20)'); + ], 'wide.yaml')) + ->toThrow(ParseException::class, 'maximum hydrated schema node count (20)') + ->toThrow(ParseException::class, '(wide.yaml)') + ->toThrow(ParseException::class, 'at #/components/schemas/'); +}); it('rejects a YAML alias-fanout bomb that stays under the byte and depth bounds', function () { // A classic alias-amplification bomb: each level references the previous From 713f79237e515e1c10918c4040b3478bd617bcd5 Mon Sep 17 00:00:00 2001 From: benjamineckstein <13351939+benjamineckstein@users.noreply.github.com> Date: Sat, 13 Jun 2026 22:56:27 +0200 Subject: [PATCH 2/2] feat(parser): name the attempted format and source file in spec file errors The decode-failure wrapper now names whether the file was parsed as JSON or YAML (taken from the extension, falling back to the first-byte sniff) and adds a hint about that detection, so a YAML file with a .json extension no longer produces an opaque 'Failed to parse' message. The size-guard message names the offending file and points at both the --max-bytes flag and the max_bytes config key. Output text only; no generated output changes. --- src/Parser/SpecParser.php | 21 ++++++++++++++++----- tests/Unit/Parser/SpecParserTest.php | 13 ++++++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/Parser/SpecParser.php b/src/Parser/SpecParser.php index aa19430..3ef1d41 100644 --- a/src/Parser/SpecParser.php +++ b/src/Parser/SpecParser.php @@ -91,10 +91,12 @@ public function parseFileToDocument(string $path): OpenApiDocument // (issue #17). The guard is disarmed as soon as the parse step returns. MemoryGuard::arm($absolute, $this->maxBytes); + $isYaml = $this->isYaml($absolute); + try { $contents = (string) file_get_contents($absolute); - $data = $this->isYaml($absolute) + $data = $isYaml ? Yaml::parse($contents) : json_decode($contents, true, flags: JSON_THROW_ON_ERROR); @@ -102,7 +104,14 @@ public function parseFileToDocument(string $path): OpenApiDocument } catch (ParseException $e) { throw $e; } catch (Throwable $e) { - throw new ParseException("Failed to parse OpenAPI spec ({$path}): {$e->getMessage()}", 0, $e); + $format = $isYaml ? 'YAML' : 'JSON'; + throw new ParseException( + "Failed to parse OpenAPI spec ({$path}) as {$format}: {$e->getMessage()}. " + ."Check that the file is well-formed {$format} (the format is taken from the file extension, " + .'falling back to sniffing the first non-whitespace byte: { or [ means JSON, anything else YAML).', + 0, + $e, + ); } finally { MemoryGuard::disarm(); } @@ -122,9 +131,11 @@ private function guardSize(string $absolute): void if ($size !== false && $size > $this->maxBytes) { throw new ParseException(sprintf( - 'OpenAPI spec is too large (%d bytes, limit %d bytes). Raise --max-bytes only for trusted ' - .'specs, and note that a larger spec needs a proportionally larger PHP memory_limit to parse ' - .'(it can otherwise exhaust memory mid-parse). Or run the generator under OS-level resource limits.', + 'OpenAPI spec is too large (%s: %d bytes, limit %d bytes). Raise --max-bytes / the max_bytes ' + .'config key only for trusted specs, and note that a larger spec needs a proportionally larger ' + .'PHP memory_limit to parse (it can otherwise exhaust memory mid-parse). Or run the generator ' + .'under OS-level resource limits.', + $absolute, $size, $this->maxBytes, )); diff --git a/tests/Unit/Parser/SpecParserTest.php b/tests/Unit/Parser/SpecParserTest.php index b9b3848..205464a 100644 --- a/tests/Unit/Parser/SpecParserTest.php +++ b/tests/Unit/Parser/SpecParserTest.php @@ -44,11 +44,14 @@ function writeTempSpec(string $name, string $contents): string (new SpecParser)->parseFileToDocument('/no/such/spec.json'); })->throws(ParseException::class, 'not found'); -it('wraps malformed content in a ParseException', function () { +it('wraps malformed content in a ParseException naming the attempted format', function () { $path = writeTempSpec('broken.json', '{not valid json'); - (new SpecParser)->parseFileToDocument($path); -})->throws(ParseException::class, 'Failed to parse OpenAPI spec'); + expect(fn () => (new SpecParser)->parseFileToDocument($path)) + ->toThrow(ParseException::class, 'Failed to parse OpenAPI spec') + ->toThrow(ParseException::class, 'as JSON') + ->toThrow(ParseException::class, 'Check that the file is well-formed JSON'); +}); // A-1: a non-OpenAPI-3.x document must fail loudly, not parse into a // null-filled graph that silently produces nothing. @@ -69,13 +72,13 @@ function writeTempSpec(string $name, string $contents): string $path = writeTempSpec('noinfo.json', '{"openapi":"3.0.3","paths":{}}'); (new SpecParser)->parseFileToDocument($path); -})->throws(ParseException::class, "missing required 'info'"); +})->throws(ParseException::class, "the required '#/info' object is missing"); it('rejects an empty file with the structural error, not a silent success', function () { $path = writeTempSpec('empty.yaml', ''); (new SpecParser)->parseFileToDocument($path); -})->throws(ParseException::class, "missing 'openapi' version string"); +})->throws(ParseException::class, "the root '#/openapi' member must be a version string"); // #103: exact version gating instead of the old `3.` prefix check. 3.0.x and // 3.1.x are fully supported (no warnings); 3.2.x is accepted best-effort with