WeaselJson_OVERFLOW is documented as "json is too deeply nested" and stackSize as a nesting-depth budget, but the pushdown stack also held tokenizer progress tokens. When a true/false/null literal or a \uXXXX escape did not fit entirely in the current chunk, the slow path pushed the not-yet-matched symbols (T_R T_U N_TRUE, T_A T_L T_S N_FALSE, T_U T_L N_NULL, N_STRING_FOLLOWING_ESCAPE, four T_HEX, or six symbols for a surrogate pair) onto the same fixed-size stack, while the whole-buffer path consumed those characters inline and pushed nothing.
The minimum stackSize therefore depended on the chunking: true was accepted at stackSize 3 when fed whole and returned the sticky WeaselJson_OVERFLOW when fed one byte at a time. RootBuilder, testStreaming() and the corpus only ever used large stacks, so nothing caught it.
Fix (option 2 from the issue)
Keep the existing encoding, but reserve the transient symbols in addition to the caller's budget:
kTransientSymbols = 6 in src/parser3.h, with the upper bound derived and documented there (next to the constant, so it can be kept in sync with the pushes): the largest single push of transient symbols is t_hex2()'s six symbols for a UTF-16 surrogate pair, and none of those continuations pushes while another's symbols are pending, so at most six are on the stack at once. The bound is tight: with 5, "\ud801\udc37" streamed byte by byte still overflows at stackSize 3 while the whole-buffer parse succeeds.
Nesting pushes still go through push() and are bounded by stackEnd = stack() + stackSize, so WeaselJson_OVERFLOW keeps meaning exactly "the document's nesting exceeds stackSize"; the new pushTransient() writes in the reserve past stackEnd and so cannot fail.
include/weaseljson.h documents only the guarantee callers can rely on - that the accept/reject/overflow decision depends on the document's bytes and on stackSize, not on how the bytes are split across parse() calls. How that is achieved is left to the implementation (per review feedback).
Tests
New doctest acceptance is independent of the chunking: for literals, escapes, surrogate pairs, nested containers and a few invalid documents, sweeps stackSize 3..16, splits each document in two at every offset and also feeds it at strides 1..12, and requires the status to equal the whole-buffer status. Verified that it fails on the pre-fix parser (true, stackSize 3, cut 1: whole buffer accepts, two chunks return WeaselJson_OVERFLOW).
src/fuzz.cpp: testStreaming() now sweeps stack sizes {3,4,5,6,7,8,12,1024} instead of only 1024, and requires the status of every streaming pattern (strides 1..15 and every single split point) to match the whole-buffer status exactly - previously a REJECT-vs-OVERFLOW difference was tolerated. Verified that the fuzz_corpus replay aborts on the pre-fix code (the corpus contains a small null document) and passes with the fix.
All existing tests pass (ctest, Release/gcc including overflow state is sticky, create rejects too-small stack, python bindings and the schemagen tests), the chunking tests are clean under valgrind, and an additional out-of-tree randomized check (200k random documents, random stack sizes 3..12, random split points) found 49k whole-vs-split status mismatches on the pre-fix parser and none with the fix.
Closes #71.
## Problem
`WeaselJson_OVERFLOW` is documented as "json is too deeply nested" and `stackSize` as a nesting-depth budget, but the pushdown stack also held *tokenizer progress* tokens. When a `true`/`false`/`null` literal or a `\uXXXX` escape did not fit entirely in the current chunk, the slow path pushed the not-yet-matched symbols (`T_R T_U N_TRUE`, `T_A T_L T_S N_FALSE`, `T_U T_L N_NULL`, `N_STRING_FOLLOWING_ESCAPE`, four `T_HEX`, or six symbols for a surrogate pair) onto the same fixed-size stack, while the whole-buffer path consumed those characters inline and pushed nothing.
The minimum `stackSize` therefore depended on the chunking: `true` was accepted at `stackSize` 3 when fed whole and returned the sticky `WeaselJson_OVERFLOW` when fed one byte at a time. `RootBuilder`, `testStreaming()` and the corpus only ever used large stacks, so nothing caught it.
## Fix (option 2 from the issue)
Keep the existing encoding, but reserve the transient symbols in addition to the caller's budget:
* `kTransientSymbols = 6` in `src/parser3.h`, with the upper bound derived and documented there (next to the constant, so it can be kept in sync with the pushes): the largest single push of transient symbols is `t_hex2()`'s six symbols for a UTF-16 surrogate pair, and none of those continuations pushes while another's symbols are pending, so at most six are on the stack at once. The bound is tight: with 5, `"\ud801\udc37"` streamed byte by byte still overflows at `stackSize` 3 while the whole-buffer parse succeeds.
* `WeaselJsonParser_create()` allocates `stackSize + kTransientSymbols` slots (`Parser3::allocationSize()`).
* Nesting pushes still go through `push()` and are bounded by `stackEnd = stack() + stackSize`, so `WeaselJson_OVERFLOW` keeps meaning exactly "the document's nesting exceeds `stackSize`"; the new `pushTransient()` writes in the reserve past `stackEnd` and so cannot fail.
* `include/weaseljson.h` documents only the guarantee callers can rely on - that the accept/reject/overflow decision depends on the document's bytes and on `stackSize`, not on how the bytes are split across `parse()` calls. How that is achieved is left to the implementation (per review feedback).
## Tests
* New doctest `acceptance is independent of the chunking`: for literals, escapes, surrogate pairs, nested containers and a few invalid documents, sweeps `stackSize` 3..16, splits each document in two at *every* offset and also feeds it at strides 1..12, and requires the status to equal the whole-buffer status. Verified that it fails on the pre-fix parser (`true`, `stackSize` 3, cut 1: whole buffer accepts, two chunks return `WeaselJson_OVERFLOW`).
* `src/fuzz.cpp`: `testStreaming()` now sweeps stack sizes `{3,4,5,6,7,8,12,1024}` instead of only 1024, and requires the status of every streaming pattern (strides 1..15 and every single split point) to match the whole-buffer status *exactly* - previously a REJECT-vs-OVERFLOW difference was tolerated. Verified that the `fuzz_corpus` replay aborts on the pre-fix code (the corpus contains a small `null` document) and passes with the fix.
All existing tests pass (`ctest`, Release/gcc including `overflow state is sticky`, `create rejects too-small stack`, python bindings and the schemagen tests), the chunking tests are clean under valgrind, and an additional out-of-tree randomized check (200k random documents, random stack sizes 3..12, random split points) found 49k whole-vs-split status mismatches on the pre-fix parser and none with the fix.
WeaselJson_OVERFLOW is documented as "json is too deeply nested", and
stackSize as a nesting-depth budget. In practice the pushdown stack also
held tokenizer progress for a `true`/`false`/`null` literal or a \uXXXX
escape that straddled the end of a chunk: the whole-buffer path consumes
those characters inline and pushes nothing, while the slow path pushed up
to six symbols (a surrogate pair) onto the same stack. So the stackSize a
document needed depended on where the parse() boundaries fell: e.g. "true"
was accepted at stackSize 3 when fed whole but returned the sticky
WeaselJson_OVERFLOW when fed one byte at a time.
Reserve slots for those symbols in addition to the supplied stackSize,
as a fixed upper bound rather than by changing the encoding: the largest
single push of transient symbols is t_hex2()'s six symbols for a surrogate
pair, and no transient symbol is ever pushed while another one is on the
stack, so kTransientSymbols = 6 is enough. Nesting pushes are still bounded
by stackSize, so OVERFLOW now means exactly what the header says and the
accept/reject/overflow decision is a function of the input bytes and
stackSize only.
Document the reserve in weaseljson.h and cover the property with tests:
a doctest sweeping stackSize 3..16 over documents whose literals, escapes
and surrogate pairs land on every chunk boundary, and a fuzz harness that
now sweeps small stack sizes and requires the status of every chunking
(stride 1..15 plus every single split point) to match the whole-buffer
status exactly.
The create() documentation described how the reserve is implemented (six
extra slots for literals and \uXXXX escapes that straddle a chunk
boundary). That is an implementation detail; the header should only
promise the behaviour callers can rely on, i.e. that the accept/reject/
overflow decision depends on the document's bytes and on stackSize, not
on how they are split across parse() calls. The derivation of the upper
bound lives next to kTransientSymbols in src/parser3.h, where it can be
kept in sync with the pushes.
Also make the chunking test cover every split point, not just fixed
strides: each document is split in two at every possible offset, so every
literal, escape and surrogate pair in the fixture straddles a chunk
boundary. Verified that this fails on the pre-fix parser ("true",
stackSize 3, cut 1: whole buffer accepts, two chunks overflow).
The pre-commit job failed with
/root/.local/bin/pre-commit: No such file or directory
even though `pipx install pre-commit` succeeded: `pipx install` writes the
app to PIPX_BIN_DIR, and the runner image no longer uses ~/.local/bin for
it. Run the binary from PATH when pipx put it there, and fall back to
`pipx run pre-commit` otherwise, so the hook still runs regardless of
where pipx exposes its apps.
andrew
merged commit 426b2641ff into main2026-10-30 13:05:59 +00:00
andrew
deleted branch weaselbot/issue-712026-10-30 13:06:04 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
andrew
Closes #71.
Problem
WeaselJson_OVERFLOWis documented as "json is too deeply nested" andstackSizeas a nesting-depth budget, but the pushdown stack also held tokenizer progress tokens. When atrue/false/nullliteral or a\uXXXXescape did not fit entirely in the current chunk, the slow path pushed the not-yet-matched symbols (T_R T_U N_TRUE,T_A T_L T_S N_FALSE,T_U T_L N_NULL,N_STRING_FOLLOWING_ESCAPE, fourT_HEX, or six symbols for a surrogate pair) onto the same fixed-size stack, while the whole-buffer path consumed those characters inline and pushed nothing.The minimum
stackSizetherefore depended on the chunking:truewas accepted atstackSize3 when fed whole and returned the stickyWeaselJson_OVERFLOWwhen fed one byte at a time.RootBuilder,testStreaming()and the corpus only ever used large stacks, so nothing caught it.Fix (option 2 from the issue)
Keep the existing encoding, but reserve the transient symbols in addition to the caller's budget:
kTransientSymbols = 6insrc/parser3.h, with the upper bound derived and documented there (next to the constant, so it can be kept in sync with the pushes): the largest single push of transient symbols ist_hex2()'s six symbols for a UTF-16 surrogate pair, and none of those continuations pushes while another's symbols are pending, so at most six are on the stack at once. The bound is tight: with 5,"\ud801\udc37"streamed byte by byte still overflows atstackSize3 while the whole-buffer parse succeeds.WeaselJsonParser_create()allocatesstackSize + kTransientSymbolsslots (Parser3::allocationSize()).push()and are bounded bystackEnd = stack() + stackSize, soWeaselJson_OVERFLOWkeeps meaning exactly "the document's nesting exceedsstackSize"; the newpushTransient()writes in the reserve paststackEndand so cannot fail.include/weaseljson.hdocuments only the guarantee callers can rely on - that the accept/reject/overflow decision depends on the document's bytes and onstackSize, not on how the bytes are split acrossparse()calls. How that is achieved is left to the implementation (per review feedback).Tests
acceptance is independent of the chunking: for literals, escapes, surrogate pairs, nested containers and a few invalid documents, sweepsstackSize3..16, splits each document in two at every offset and also feeds it at strides 1..12, and requires the status to equal the whole-buffer status. Verified that it fails on the pre-fix parser (true,stackSize3, cut 1: whole buffer accepts, two chunks returnWeaselJson_OVERFLOW).src/fuzz.cpp:testStreaming()now sweeps stack sizes{3,4,5,6,7,8,12,1024}instead of only 1024, and requires the status of every streaming pattern (strides 1..15 and every single split point) to match the whole-buffer status exactly - previously a REJECT-vs-OVERFLOW difference was tolerated. Verified that thefuzz_corpusreplay aborts on the pre-fix code (the corpus contains a smallnulldocument) and passes with the fix.All existing tests pass (
ctest, Release/gcc includingoverflow state is sticky,create rejects too-small stack, python bindings and the schemagen tests), the chunking tests are clean under valgrind, and an additional out-of-tree randomized check (200k random documents, random stack sizes 3..12, random split points) found 49k whole-vs-split status mismatches on the pre-fix parser and none with the fix.@@ -57,0 +53,4 @@* increases the depth of nested json accepted. `stackSize` is an exact budget:* whether a document is accepted, rejected or reported as too deeply nested* depends only on its bytes and on `stackSize`, not on how the bytes are split* across WeaselJsonParser_parse calls. To get that guarantee, the parserHow we achieve this is an implementation detail and doesn't belong in the public header
The create() documentation described how the reserve is implemented (six extra slots for literals and \uXXXX escapes that straddle a chunk boundary). That is an implementation detail; the header should only promise the behaviour callers can rely on, i.e. that the accept/reject/ overflow decision depends on the document's bytes and on stackSize, not on how they are split across parse() calls. The derivation of the upper bound lives next to kTransientSymbols in src/parser3.h, where it can be kept in sync with the pushes. Also make the chunking test cover every split point, not just fixed strides: each document is split in two at every possible offset, so every literal, escape and surrogate pair in the fixture straddles a chunk boundary. Verified that this fails on the pre-fix parser ("true", stackSize 3, cut 1: whole buffer accepts, two chunks overflow).5785912b6cto507be6f0a2The pre-commit job failed with /root/.local/bin/pre-commit: No such file or directory even though `pipx install pre-commit` succeeded: `pipx install` writes the app to PIPX_BIN_DIR, and the runner image no longer uses ~/.local/bin for it. Run the binary from PATH when pipx put it there, and fall back to `pipx run pre-commit` otherwise, so the hook still runs regardless of where pipx exposes its apps.