Uh oh!
There was an error while loading. Please reload this page.
feat(compilers/openapi): bound input size, honour cancellation - #349
Merged
Conversation
Three production conflicts. load.go took both sides' Options fields and kept the byte budget where it was, before the parse, dropping this branch's copy of the pre-parse cycle scan that main moved onto the decoded tree. compose.go and architecture.md each had one side add a paragraph while the other rewrote the one beside it. The rest were the test helpers main moved into openapitest, which also gained a lowerer constructor this branch's fixtures had to adopt while keeping the Limits argument they pass. The merge also put the enum budget and the unhomed-keyword census (#348) in each other's way, which neither could see alone. A member set past the budget degrades to the top type, and ir.Any has a field for nothing, so the census preserved the whole set verbatim: 5,000 members refused at a budget of 10 came back as 50,001 bytes under Unmodeled, more than the source they were read from. The budget bounded the per-member amplification and nothing else. keywordHome now counts Any a home for `enum`, exactly as it already did for `const` and for the same reason — lowerEnum read the members and announced the degradation, so claiming them again both double-reports and undoes the refusal. typeShapedBy counts it for `type` too: the top type contradicts no declared shape, and a position only arrives there by a degradation that has already been reported. The budget test now also pins that the refused node keeps nothing, which is the half a diagnostic count does not state.
TestRun_RefusesAtEveryPhaseBoundaryOnCancellation had a row for each of run's two boundaries, but only the second one was pinned. Every boundary returns the same nil document, the same context.Canceled and the same empty diagnostics, so the first row passed just as well when the boundary it names was reached and fell through to the one after it — deleting the check after the component walk left the whole suite green. The assertion meant to separate them could not: left starts at zero on that row, so asserting it ends at zero says nothing. The context now counts the calls it answers, and each row asserts the count its boundary implies — a boundary that does not stop the compile is one the next boundary asks after, which is one call too many. With that, all five ctx.Err() checks redden when reverted individually; before it, four did.
Uh oh!
There was an error while loading. Please reload this page.
OmarAlJarrah added a commit
that referenced
this pull request
Aug 10, 2026
lowerEnum grew a guard on each side: the empty member list here, the member budget on main (#349). They cannot both hold — a count of zero exceeds no positive budget — so both are kept, the degenerate list read first, beside the members it lacks. The conformance table gained a third column on main (#342), naming the matrix rows a case witnesses. empty-enum names none: its subject is the degenerate list rather than any enum capability, and that table's own comment warns that reaching for a construct on the way to a different subject is not witnessing it. The empty-enum golden moved, because the unhomed-keyword census (#348) reaches this branch for the first time here. `type: object` beside an Enum now lands under Unmodeled with the property set that was already kept — an Enum carries neither — so the case preserves strictly more than it did, and its diagnostic names both keywords.
OmarAlJarrah added a commit
that referenced
this pull request
Aug 11, 2026
lowering.New gained a parameter on each side — the budgets on main (#349), the streaming media policy here — so the merged signature takes both, and the field comment that called Grouping "one of the two" caller policies no longer counts them: it names the other two instead, since a count in prose is what just went stale. The conformance table gained its capability column on main (#342), so both streaming rows come back with the matrix rows they witness. streaming-media-31 witnesses more than it may claim. Its NDJSON request body lowers to a populated RequestStream and the golden reads streaming=bidi, but ir-spec-matrix.md marks streaming-client absent for OpenAPI, and the matrix contract refuses a witness for a row a format is said not to reach. The row claims streaming-server alone, which is true and checkable; whether the matrix should now say OpenAPI reaches client streaming is a question about that document rather than about this merge.
OmarAlJarrah added a commit
that referenced
this pull request
Aug 11, 2026
lowering.New now takes three caller policies: the budgets and the streaming media list arrived on main (#349, #352), the promotion mapping here. The field comment naming Grouping "one of the two" no longer counts them, and both policy types had called themselves "the second injectable-policy seam" — with Grouping there are three, so neither is second. Two lowerings this branch hooked into were refactored under it. An operation's extensions are gathered by applyOperationExtensions now, and a security scheme's by applySchemeExtensions, so the promotion runs after each rather than beside the inline reads it used to follow — it reads the assembled map, so it has to. The conformance table gained its capability column on main (#342); extension-promotion names the deprecation row, whose members are the fields it fills. It does not name vendor-extensions: extensions-x witnesses that already, and the x-* here are the means rather than the subject.
OmarAlJarrah added a commit
that referenced
this pull request
Aug 13, 2026
The branch forked before #310, #349 and #356 landed, so three conflicts needed resolving rather than taking a side: - census() keeps main's provenance-aware unrecorded() filter, its keep() helper and its filter-before-bound order, under this branch's (keys, root) signature. Taking the branch's body would have dropped the collision and unreachable-key reports and spent budget slots on keys another reader had already kept. - applyPathItem() absorbs applyPathItemDocs() rather than replacing applyPathItemResidue() outright, so a path item's summary and description are still kept on each operation it mounts (#292/#310). carrier.keepsDocs marks where that pair has a home: only an operation, since pathItemDocFields keys by bare keyword and one service holds every unmounted item. #383 keeps the unmounted half. - lowerPaths/lowerWebhooks keep main's ctx cancellation and pathOperations walk beside this branch's svc carrier.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The compiler already carries two dozen named caps, and the ones a pathological
input can reach do fire: schema nesting past 256 and YAML alias amplification
both produce a coded, provenanced diagnostic and exit 1. What none of them
bounds is cardinality or aggregate size. A legal OpenAPI 3.1 document whose
one component declares a 1,000,000-member enum turned 10 MB of source into
2.6 GB of peak RSS and 318 MB of IR JSON, exiting 0 with an empty stderr — the
failure mode on that axis was OOM-or-succeed, never a diagnostic.
This adds budgets for that axis, as options rather than constants, because what
counts as too large depends on the machine doing the compiling:
openapi.Options.Limits.MaxSourceBytes(default 64 MiB), checked before thesource is parsed — the only bound that can be taken before anything about the
document is known, and what bounds the parse itself.
Limits.MaxSourceNodes(default 2,097,152), checked on the parsed tree afterany overlay is applied, before the typed model is built from it. Node count is
what the phases after the parse cost.
Limits.MaxEnumMembers(default 65,536). An enum is the one construct whoseIR cost per source node is disproportionate — a canonical word sequence per
member, or a hoisted literal type and a union variant per member — which is
why the node budget does not cover it.
Each default is calibrated against measured documents rather than chosen round;
the constants carry the numbers and the margins. The largest public description
measured (GitHub's REST API, 12.9 MB, 471,735 nodes, largest enum 53 members)
sits 4.4x inside the tightest of them. Zero takes the default in every field and
a negative value turns the budget off, so a caller with the memory for a huge
input keeps the old behaviour. Crossing a budget is a spec problem, so it is an
openapi/budget-exceedederror diagnostic, not a Go error; the two load-phasebudgets refuse the document, the enum budget degrades that one node to the top
type and lets the rest of the document lower.
Time was not a bound either.
ctxwas threaded throughCompileand neverconsulted anywhere in the compiler, so a caller could not stop a runaway compile
with a deadline. The two walks that do work proportional to the document — the
component-schema loop and the path/webhook loops — now check
ctx.Err()betweenitems, and
runrefuses at the phase boundary after each rather than assemblinga document from a half-filled registry. Cancellation reaches the caller as the
wrapped
context.Canceled/DeadlineExceeded, since nothing about the documentis wrong; the diagnostics gathered before the stop are still returned.
Measured on the reproduction above: exit 0 with no diagnostics and 2.59 GB peak
RSS becomes exit 1 with one diagnostic naming the enum, its pointer, the count
and the budget, at 896 MB and 3.5 s. GitHub's spec compiles to byte-identical IR
with no budget diagnostic.
Test plan
the limit the document lowers whole, one past it is refused. Bytes and enum
members are pinned through the public
Compile; the node budget is pinnedexactly in the load package, where the count the loader takes is visible.
asserts every member reaches the IR and no budget diagnostic is raised, so the
budgets cannot pass by refusing anything big.
table over
Limits.withDefaultspins which spellings mean default, set andoff.
interns nothing, a cancelled service walk produces no group — since the phase
boundary above would report the same error with neither loop checking. The
boundary checks themselves are driven by a context that reports itself live for
a chosen number of calls, which is what places the cancellation at a specific
boundary deterministically instead of racing the walk against a timer.
ctx.Err()check was reverted in turn with the tests leftin place; each went red, and all six were restored before the gate ran.
specs are generated in the tests, and no golden changed.
Closes#75