diff --git a/include/weaseljson.h b/include/weaseljson.h index aa0ec52..3c784fe 100644 --- a/include/weaseljson.h +++ b/include/weaseljson.h @@ -36,7 +36,7 @@ enum WeaselJsonStatus { WeaselJson_AGAIN, /** Invalid json */ WeaselJson_REJECT, - /** json is too deeply nested */ + /** json is too deeply nested for the parser's stack size */ WeaselJson_OVERFLOW, /** Tried to call parse on a null parser */ WeaselJson_NULL, @@ -50,10 +50,16 @@ enum WeaselJsonFlags { }; /** Create a parser. Increasing stack size increases memory usage but also - * increases the depth of nested json accepted. `callbacks` and `userdata` must - * outlive the returned parser. Returns null if there's insufficient available - * memory, or if `stackSize` is negative or too small to hold a minimal - * document. */ + * 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 parser + * allocates 6 slots beyond `stackSize` for tokenizer progress: literals + * ("true", "false", "null") and \uXXXX escapes that straddle the end of a chunk + * record their unmatched characters there instead of spending the nesting + * budget. `callbacks` and `userdata` must outlive the returned parser. Returns + * null if there's insufficient available memory, or if `stackSize` is negative + * or too small to hold a minimal document. */ WeaselJsonParser *WeaselJsonParser_create(int stackSize, const WeaselJsonCallbacks *callbacks, void *userdata, int flags); diff --git a/src/fuzz.cpp b/src/fuzz.cpp index 3b40f48..1dd0321 100644 --- a/src/fuzz.cpp +++ b/src/fuzz.cpp @@ -5,12 +5,12 @@ #include -std::pair runStreaming(std::string copy, - int stride) { +std::pair +runStreaming(std::string copy, int stackSize, int stride) { SerializeState state; auto c = serializeCallbacks(); std::unique_ptr parser{ - WeaselJsonParser_create(1024, &c, &state, + WeaselJsonParser_create(stackSize, &c, &state, copy.size() % 2 == 0 ? WeaselJsonRaw : 0), WeaselJsonParser_destroy}; if (stride == 0) { @@ -31,11 +31,12 @@ std::pair runStreaming(std::string copy, return {state.result, s}; } -std::pair runBatch(std::string copy) { +std::pair runBatch(std::string copy, + int stackSize) { SerializeState state; auto c = serializeCallbacks(); std::unique_ptr parser{ - WeaselJsonParser_create(1 << 20, &c, &state, + WeaselJsonParser_create(stackSize, &c, &state, copy.size() % 2 == 0 ? WeaselJsonRaw : 0), WeaselJsonParser_destroy}; auto s = WeaselJsonParser_parse(parser.get(), copy.data(), copy.size()); @@ -47,11 +48,11 @@ std::pair runBatch(std::string copy) { } std::pair runPrefix(std::string copy, - int prefix) { + int stackSize, int prefix) { SerializeState state; auto c = serializeCallbacks(); std::unique_ptr parser{ - WeaselJsonParser_create(1024, &c, &state, + WeaselJsonParser_create(stackSize, &c, &state, copy.size() % 2 == 0 ? WeaselJsonRaw : 0), WeaselJsonParser_destroy}; auto s = WeaselJsonParser_parse(parser.get(), copy.data(), prefix); @@ -67,55 +68,54 @@ std::pair runPrefix(std::string copy, return {state.result, s}; } -void testStreaming(std::string const &json) { - auto batch = runBatch(json); +void checkStreamingResult( + int stackSize, char const *kind, + std::pair const &got, + std::pair const &batch) { + if (got.second == WeaselJson_AGAIN) { + abort(); + } + // The status must be identical however the document is chunked; the events + // reported for a document that failed to parse need not match. + bool ok = got.second == batch.second && + (got.second != WeaselJson_OK || got.first == batch.first); + if (!ok) { + printf("%s (stackSize %d): %d, %s\n", kind, stackSize, (int)got.second, + got.first.c_str()); + printf("batch (stackSize %d): %d, %s\n", stackSize, (int)batch.second, + batch.first.c_str()); + abort(); + } +} + +void testStreaming(std::string const &json, int stackSize) { + auto batch = runBatch(json, stackSize); if (batch.second == WeaselJson_AGAIN) { abort(); } for (int stride = 1; stride < 16; ++stride) { - auto streaming = runStreaming(json, stride); - if (streaming != batch) { - if (streaming.second == WeaselJson_AGAIN) { - abort(); - } - bool streamingOk = streaming.second == WeaselJson_OK; - bool batchOk = batch.second == WeaselJson_OK; - if (streamingOk == batchOk && !batchOk) { - // It's ok if the processed data doesn't match if parsing failed - } else { - printf("streaming: %s, %s\n", - streaming.second == WeaselJson_OK ? "accept" : "reject", - streaming.first.c_str()); - printf("batch: %s, %s\n", - batch.second == WeaselJson_OK ? "accept" : "reject", - batch.first.c_str()); - abort(); - } - } + checkStreamingResult(stackSize, "streaming", + runStreaming(json, stackSize, stride), batch); if (int(json.size()) > stride) { - auto prefix = runPrefix(json, stride); - if (prefix != batch) { - if (prefix.second == WeaselJson_AGAIN) { - abort(); - } - bool prefixOk = prefix.second == WeaselJson_OK; - bool batchOk = batch.second == WeaselJson_OK; - if (prefixOk == batchOk && !batchOk) { - // It's ok if the processed data doesn't match if parsing failed - } else { - printf("prefix: %s, %s\n", - prefix.second == WeaselJson_OK ? "accept" : "reject", - prefix.first.c_str()); - printf("batch: %s, %s\n", - batch.second == WeaselJson_OK ? "accept" : "reject", - batch.first.c_str()); - abort(); - } - } + checkStreamingResult(stackSize, "prefix", + runPrefix(json, stackSize, stride), batch); } } } +void testStreaming(std::string const &json) { + // Whether a document parses must not depend on the streaming pattern: the + // parser hands the tokenizer progress for a literal or a \uXXXX escape that + // straddles a chunk boundary to the pushdown stack, so a stackSize picked for + // the document's nesting depth used to overflow for some chunkings only. + // Sweep stack sizes around the minimum (3 is the smallest accepted by + // WeaselJsonParser_create) so that the property is exercised, then a large + // one for documents that need the room. + for (int stackSize : {3, 4, 5, 6, 7, 8, 12, 1024}) { + testStreaming(json, stackSize); + } +} + void compareWithSimdjson(std::string const &json) { WeaselJsonStatus ours; { diff --git a/src/lib.cpp b/src/lib.cpp index b805e01..655275e 100644 --- a/src/lib.cpp +++ b/src/lib.cpp @@ -13,8 +13,7 @@ WeaselJsonParser_create(int stackSize, const WeaselJsonCallbacks *callbacks, if (stackSize < 0) { // avoid (size_t)stackSize wrapping to a huge allocation return nullptr; } - auto *buf = - malloc(sizeof(Parser3) + (size_t)stackSize * sizeof(*Parser3::stackPtr)); + auto *buf = malloc(Parser3::allocationSize(stackSize)); if (buf == nullptr) { return nullptr; } diff --git a/src/parser3.h b/src/parser3.h index 706a77f..daadb56 100644 --- a/src/parser3.h +++ b/src/parser3.h @@ -72,6 +72,27 @@ enum Symbol : uint8_t { T_BACKSLASH, N_SYMBOL_COUNT, // Must be last }; + +// Symbols that are only on the stack because a literal (`true`/`false`/`null`) +// or a `\uXXXX` escape straddles the end of a chunk. The whole-buffer path +// consumes those characters inline and pushes nothing, so these symbols must +// not spend the caller's nesting budget: otherwise the stackSize a document +// needs would depend on where the parse() boundaries happen to fall (see issue +// #71). WeaselJsonParser_create() therefore allocates this many extra slots on +// top of `stackSize`; see Parser3::pushTransient(). +// +// These are all the pushes of such symbols, with their sizes: +// n_value() 3, 4, 3 (true, false, null) +// n_string2() 1 (N_STRING_FOLLOWING_ESCAPE) +// n_string_following_escape() 4 (the four hex digits) +// t_hex2() 6 (utf-16 surrogate pair) +// No transient symbol is ever pushed while another one is on the stack (each of +// these continuations pops its own symbol before pushing, and the literal ones +// run with N_VALUE already popped), so the largest single push - the six +// symbols that start a surrogate pair - bounds the number of transient symbols +// on the stack. +inline constexpr int kTransientSymbols = 6; + struct Parser3 { Parser3(const WeaselJsonCallbacks *callbacks, void *userdata, int stackSize, int flags) @@ -80,6 +101,14 @@ struct Parser3 { reset(); } + // Size of the allocation that holds a parser with a nesting budget of + // `stackSize` symbols: the parser itself, that budget, and the reserve for + // transient tokenizer progress symbols. + static constexpr size_t allocationSize(int stackSize) { + return sizeof(Parser3) + + (size_t(stackSize) + kTransientSymbols) * sizeof(Symbol); + } + [[nodiscard]] WeaselJsonStatus parse(char *buf, int len); void flushNumber(bool done, char *buf) { @@ -114,9 +143,12 @@ struct Parser3 { } // Pushing symbols onto the stack roughly corresponds with "parse these - // nonterminals/terminals in order" - [[nodiscard]] WeaselJsonStatus push(std::initializer_list symbols) { - if (stackEnd - stackPtr < ptrdiff_t(symbols.size())) [[unlikely]] { + // nonterminals/terminals in order". `end` is the first slot the symbols may + // not occupy. + [[nodiscard]] static WeaselJsonStatus + pushOnto(Symbol *&stackPtr, Symbol *end, + std::initializer_list symbols) { + if (end - stackPtr < ptrdiff_t(symbols.size())) [[unlikely]] { return WeaselJson_OVERFLOW; } for (int i = symbols.size() - 1; i >= 0; --i) { @@ -125,6 +157,23 @@ struct Parser3 { return WeaselJson_OK; } + // Push symbols that model the nesting of the document. This is what + // WeaselJson_OVERFLOW reports, so it's bounded by the caller's stackSize. + [[nodiscard]] WeaselJsonStatus push(std::initializer_list symbols) { + return pushOnto(stackPtr, stackEnd, symbols); + } + + // Push tokenizer progress symbols for a literal or escape that straddles a + // chunk boundary. These go in the reserve past `stackEnd`, so they don't + // spend the caller's nesting budget and can't make the parser overflow on + // input it would have accepted in one piece. See kTransientSymbols for why + // the reserve is big enough to always succeed. + [[nodiscard]] WeaselJsonStatus + pushTransient(std::initializer_list symbols) { + assert(ptrdiff_t(symbols.size()) <= kTransientSymbols); + return pushOnto(stackPtr, stackEnd + kTransientSymbols, symbols); + } + [[nodiscard]] Symbol top() const { assert(!empty()); return *(stackPtr - 1); @@ -376,7 +425,7 @@ inline PRESERVE_NONE ContinuationStatus n_value(Parser3 *self, char *buf, return WeaselJson_REJECT; } } else { - if (auto s = self->push({T_R, T_U, N_TRUE})) { + if (auto s = self->pushTransient({T_R, T_U, N_TRUE})) { return s; } } @@ -392,7 +441,7 @@ inline PRESERVE_NONE ContinuationStatus n_value(Parser3 *self, char *buf, return WeaselJson_REJECT; } } else { - if (auto s = self->push({T_A, T_L, T_S, N_FALSE})) { + if (auto s = self->pushTransient({T_A, T_L, T_S, N_FALSE})) { return s; } } @@ -408,7 +457,7 @@ inline PRESERVE_NONE ContinuationStatus n_value(Parser3 *self, char *buf, return WeaselJson_REJECT; } } else { - if (auto s = self->push({T_U, T_L, N_NULL})) { + if (auto s = self->pushTransient({T_U, T_L, N_NULL})) { return s; } } @@ -609,7 +658,7 @@ inline PRESERVE_NONE ContinuationStatus n_string2(Parser3 *self, char *buf, case '\\': ++buf; if (bufEnd - buf < /*strlen("u0000\\u0000")*/ 11) { - if (auto s = self->push({N_STRING_FOLLOWING_ESCAPE})) { + if (auto s = self->pushTransient({N_STRING_FOLLOWING_ESCAPE})) { return s; } if (buf == bufEnd) { @@ -718,7 +767,7 @@ n_string_following_escape(Parser3 *self, char *buf, char *bufEnd) { ++buf; self->utf8Codepoint = 0; self->pop(); - if (auto s = self->push({T_HEX, T_HEX, T_HEX, T_HEX2})) { + if (auto s = self->pushTransient({T_HEX, T_HEX, T_HEX, T_HEX2})) { return s; } break; @@ -800,8 +849,8 @@ inline PRESERVE_NONE ContinuationStatus t_hex2(Parser3 *self, char *buf, self->utf16Surrogate = self->utf8Codepoint; self->utf8Codepoint = 0; self->pop(); - if (auto s = - self->push({T_BACKSLASH, T_U2, T_HEX, T_HEX, T_HEX, T_HEX3})) { + if (auto s = self->pushTransient( + {T_BACKSLASH, T_U2, T_HEX, T_HEX, T_HEX, T_HEX3})) { return s; } if (buf == bufEnd) { diff --git a/src/test.cpp b/src/test.cpp index 10cb1fb..453f0f4 100644 --- a/src/test.cpp +++ b/src/test.cpp @@ -4,8 +4,10 @@ #include #include +#include #include #include +#include #include #include @@ -152,6 +154,27 @@ void testStreaming(std::string const &json) { CHECK(streaming.result == batch.result); } +// Feed `doc` to a fresh parser with the given stackSize, in chunks of `stride` +// bytes (`stride` <= 0 means all at once), and return the final status. +WeaselJsonStatus parseChunked(std::string const &doc, int stackSize, + int stride) { + auto c = noopCallbacks(); + std::string copy = doc; + auto *parser = WeaselJsonParser_create(stackSize, &c, nullptr, 0); + REQUIRE(parser != nullptr); + int step = stride > 0 ? stride : std::max(1, (int)copy.size()); + WeaselJsonStatus status = WeaselJson_AGAIN; + for (size_t i = 0; i < copy.size() && status == WeaselJson_AGAIN; i += step) { + status = WeaselJsonParser_parse(parser, copy.data() + i, + std::min(step, copy.size() - i)); + } + if (status == WeaselJson_AGAIN) { + status = WeaselJsonParser_parse(parser, nullptr, 0); + } + WeaselJsonParser_destroy(parser); + return status; +} + } // namespace TEST_CASE("parser3") { @@ -258,6 +281,48 @@ TEST_CASE("reset clears overflow state") { WeaselJsonParser_destroy(parser); } +TEST_CASE("acceptance is independent of the chunking") { + // Literals and \uXXXX escapes that straddle the end of a chunk are tokenized + // by pushing their not-yet-matched symbols onto the pushdown stack, so they + // used to spend the caller's nesting budget: a document could be accepted + // when fed whole and reported as WeaselJson_OVERFLOW when streamed in smaller + // pieces. The decision must depend only on the document and on `stackSize`. + std::vector docs = { + "1", + "true", + "false", + "null", + "[true]", + "[true,false,null]", + "\"\\u00e9abcdefgh\"", + // Needs the full transient reserve: a surrogate pair split across chunks + // pushes six symbols at once. + "\"\\ud801\\udc37\"", + "[\"\\u00e9abcdefgh\"]", + "{\"\\ud801\\udc37\":[true,{\"x\":null}]}", + "[[[[[[[[[[[[true]]]]]]]]]]]]", + // Whether an invalid document is rejected must not depend on the chunking + // either. + "tru", + "truX", + "[[[[x", + "\"\\ud801x\"", + }; + for (auto const &doc : docs) { + CAPTURE(doc); + // 3 is the smallest stackSize WeaselJsonParser_create accepts, so this + // sweeps from "everything overflows" to "everything fits". + for (int stackSize = 3; stackSize <= 16; ++stackSize) { + CAPTURE(stackSize); + auto whole = parseChunked(doc, stackSize, 0); + for (int stride = 1; stride <= 12; ++stride) { + CAPTURE(stride); + REQUIRE(parseChunked(doc, stackSize, stride) == whole); + } + } + } +} + TEST_CASE("rejected state is sticky") { auto c = noopCallbacks(); auto *parser = WeaselJsonParser_create(1024, &c, nullptr, 0);