Revisit the strict-ingest preflight: six memory findings on forty lines, and one residual that cannot be closed locally

[bomly-guy]

Filed from review on #431. This is deliberately a design question rather than another point fix — it is the sixth memory finding on requireUnambiguousJSON, and the pattern is more informative than any one of them.

The immediate finding

Reported by Codex on internal/sbom/strict.go:194: the name-byte bound is checked after ReadToken has already unquoted and retained the name, so a single name larger than the bound is retained once before being refused.

Measured: a 200 MiB member name retains 200 MiB — 1.00× input.

Why it was not fixed in #431

Two reasons, both recorded in the code beside the bound:

1. No library option exists. jsontext.Options offers AllowDuplicateNames, AllowInvalidUTF8, and formatting options. Nothing bounds a token's size.
2. The only remaining route is prohibited. Sizing the name before the decoder reads it means scanning raw bytes for the string's closing quote with escape handling — mirroring the library's tokenizer, which this project's delegation rule names explicitly as an anti-pattern.

The residual is also the floor, not a multiplier: the name is bounded by an input already capped at 256 MiB and already resident. That is qualitatively different from what the earlier fixes removed.

The pattern, which is the actual point

Six findings against one ~40-line function across #431's review rounds:

#	Shape	Worst case	Fix
1	One very wide object	6.7× input	bound the widest object
2	Nested objects, each under that bound	2.5 GiB from 411 MiB	bound the aggregate across open objects
3	Aggregate names within bounds, but long	900 MiB from 194 MiB	bound name bytes, not count
4	Large values, which no name bound reaches	447 MiB from 150 MiB	read in place (*bytes.Buffer)
5	Byte bound counted indentation (my regression)	6.4× over-count	measure the token, not the span
6	One name larger than the bound	1.00× input	open — see above

Two observations worth carrying:

• The two fixes that closed whole classes were replacements, not tunings. (4) and (5) removed an approximation — a generic reader instead of the in-place path, offset arithmetic instead of the token. The three that added or adjusted bounds each left a next shape.
• Every doc comment written about this function's memory was wrong until measured. "Constant memory", then a bound on the wrong quantity, then an estimate off by 40%. The current comment states measured figures only.

The question to decide

Whether a separate hand-instrumented preflight is the right shape at all. It exists because ADR-0039 wants exactly two guarantees (duplicate names, invalid UTF-8) without the other behavior changes that decoding through encoding/json/v2 would bring — case-sensitive field matching in particular, which the ADR explicitly does not claim.

Alternatives worth costing:

1. Keep it, accept the residual. Document that preflight peak is bounded by input size. Cheapest, and arguably already true.
2. Decode through json/v2 for real, and validate what the case-sensitivity change actually does to SPDX and CycloneDX fixtures. Removes the hand-instrumentation entirely; needs the behavior study ADR-0039 declined to do.
3. Ask upstream for a token-size option. A jsontext.MaxTokenSize-style option would close (6) properly and is plausibly useful beyond Bomly.

Context

• internal/sbom/strict.go — the function, its bounds, and the recorded reason for the open residual.
• Coverage: TestADocumentTooLargeToCheckIsRefused, TestNestedObjectsCannotAccumulatePastTheBound, TestSiblingObjectsDoNotAccumulate, TestLongMemberNamesAreBoundedByTheirBytes, TestClosedObjectsReleaseTheirNameBytes, TestOrdinaryNamesDoNotApproachTheByteBound, TestPrettyPrintingDoesNotCountAgainstTheByteBound, TestStrictPreflightReadsInPlace.
• Related: Strict SBOM ingest rejects escaped lone surrogates, a third class ADR-0039 does not claim #432 (lone-surrogate rejection is a third class ADR-0039 does not claim) — same function, same ADR, worth deciding together.

[bomly-guy]

Correction to the measurement this issue was filed on, and one of its two parts is now fixed.

I reported the residual as 1.00× input. That figure came from a probe that read tokens and discarded them — but the real code calls token.String() to size each member name, and I had not measured with it. CodeRabbit found the gap (#431, internal/sbom/strict.go).

Measured properly:

Case	Input	With String()	Without
Unescaped 100 MiB name	100 MiB	200 MiB	100 MiB
Escaped 100 MiB name	200 MiB	500 MiB	200 MiB

So the code was adding a full extra copy on top of the decoder's own retention — for escaped names, where the decoder cannot alias the input, 2.5× the input rather than 1×.

Fixed in #431. An oversized name is now refused on its source span before it is read: the span is free, never smaller than the name inside it, and a name whose span alone exceeds the whole budget cannot fit however it unescapes. Anything past the gate is small enough to measure exactly. Both cases above now stay at 1.00×.

CodeRabbit's suggested fix — a bounded unescape/count path — was declined for the same reason Codex's was: it reimplements the library's string tokenizer, which this project's delegation rule refuses. The span gate protects the same memory without parsing anything.

What remains open here is unchanged in substance and now correctly measured: the decoder retains a name internally before this code sees the token, so a single oversized name is held once at 1× input. That is the floor for any approach that reads the document, and closing it still needs either a jsontext token-size option or the tokenizer-mirroring this project refuses.

The design question this issue was really filed for stands, and this episode strengthens it: that is now seven memory findings on the same forty lines, and the count of my own comments about this function that were wrong until measured is now three.

[bomly-guy]

Two more findings against this function, both measured. One is refuted; one is real and left open deliberately.

Refuted — popped namespaces do not accumulate across depths. The claim was that jsontext keeps each popped objectNamespace in its stack's backing array until that depth is reused, so wide objects at successive depths would accumulate while the release-on-close accounting subtracts them. If that held, the bound would be unsound and three earlier fixes with it.

Measured, with a 50,000-name object at each successive depth, each closing before descending:

Levels	Input	Counter peak	Retained
10	6 MiB	50,012 names	11 MiB
40	25 MiB	50,042 names	21 MiB
160	107 MiB	50,162 names	13 MiB

Retention is flat in depth — noise, not growth. At 160 levels the accumulating behavior would have retained roughly eight million names. The accounting tracks real retention.

Real, and left open — the span gate counts leading whitespace. The gate's span runs from the end of the previous token, so it includes the delimiter and any whitespace before the name:

 8 MiB of spaces before a 3-byte name -> accepted
17 MiB of spaces before a 3-byte name -> ErrUnverifiableJSON

Not patched, and the reasoning is on the merits rather than the round count:

• No formatter emits sixteen megabytes of consecutive whitespace in one gap. This is unreachable from any produced document.
• The outcome is a refusal, not a wrong answer, and a hostile document is refused either way — so there is nothing to gain by making the gate tighter.
• Closing it means skipping insignificant whitespace by hand, which is a small piece of the lexer this file deliberately does not own.

It differs from the earlier whitespace defect, which was cumulative across many names and reachable by ordinary pretty-printing. That one mattered and was fixed; this one is a single 16 MiB gap.

Recorded in the code beside the gate. Running total: nine memory findings on this function. Seven were real, two refuted. That ratio is itself an argument for the design question above rather than for a tenth bound.