forked from weaselab/weaseljson
Address review: keep the transient-token reserve out of 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).
This commit is contained in:
1 parent
ac619af24d
commit
5785912b6c
3 files changed
+54
-28
No files matched your search
@@ -50,14 +50,10 @@ enum WeaselJsonFlags {
|
||||
};
|
||||
|
||||
/** Create a parser. Increasing stack size increases memory usage but also
|
||||
* 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
|
||||
* increases the depth of nested json accepted. 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. `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,
|
||||
|
||||
+13
-10
@@ -73,24 +73,27 @@ enum Symbol : uint8_t {
|
||||
N_SYMBOL_COUNT, // Must be last
|
||||
};
|
||||
|
||||
// Symbols that are only on the stack because a literal (`true`/`false`/`null`)
|
||||
// Some symbols are on the stack only 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().
|
||||
// needs would depend on where the parse() boundaries happen to fall (issue
|
||||
// #71). WeaselJsonParser_create() allocates this many slots beyond `stackSize`
|
||||
// to hold them, and Parser3::pushTransient() pushes them there.
|
||||
//
|
||||
// These are all the pushes of such symbols, with their sizes:
|
||||
// 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.
|
||||
//
|
||||
// Upper bound on how many of them are on the stack at once: 6. These are
|
||||
// pushdown symbols, so the ones that are on the stack are consumed before
|
||||
// anything else is pushed; and none of those continuations pushes while
|
||||
// another's symbols are still pending, because each pops the symbol it took
|
||||
// before pushing (the literal ones pop N_VALUE before n_value() pushes, and the
|
||||
// rest pop themselves). The number of pending symbols is therefore bounded by
|
||||
// the largest single push, the six symbols that start a surrogate pair.
|
||||
inline constexpr int kTransientSymbols = 6;
|
||||
|
||||
struct Parser3 {
|
||||
|
||||
+37
-10
@@ -154,19 +154,30 @@ 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.
|
||||
// Feed `doc` to a fresh parser with the given stackSize, cutting it into chunks
|
||||
// at the offsets in `cuts` (ascending, each > 0 and < doc.size(); an empty
|
||||
// `cuts` feeds the whole document at once), and return the final status.
|
||||
WeaselJsonStatus parseChunked(std::string const &doc, int stackSize,
|
||||
int stride) {
|
||||
std::vector<size_t> const &cuts) {
|
||||
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<int>(step, copy.size() - i));
|
||||
size_t pos = 0;
|
||||
for (size_t cut : cuts) {
|
||||
if (status != WeaselJson_AGAIN) {
|
||||
break;
|
||||
}
|
||||
if (cut > pos && cut < copy.size()) {
|
||||
status =
|
||||
WeaselJsonParser_parse(parser, copy.data() + pos, int(cut - pos));
|
||||
pos = cut;
|
||||
}
|
||||
}
|
||||
if (status == WeaselJson_AGAIN && pos < copy.size()) {
|
||||
status = WeaselJsonParser_parse(parser, copy.data() + pos,
|
||||
int(copy.size() - pos));
|
||||
}
|
||||
if (status == WeaselJson_AGAIN) {
|
||||
status = WeaselJsonParser_parse(parser, nullptr, 0);
|
||||
@@ -175,6 +186,15 @@ WeaselJsonStatus parseChunked(std::string const &doc, int stackSize,
|
||||
return status;
|
||||
}
|
||||
|
||||
// The offsets to cut `doc` into chunks of `stride` bytes.
|
||||
std::vector<size_t> stridedCuts(std::string const &doc, size_t stride) {
|
||||
std::vector<size_t> cuts;
|
||||
for (size_t cut = stride; cut < doc.size(); cut += stride) {
|
||||
cuts.push_back(cut);
|
||||
}
|
||||
return cuts;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
TEST_CASE("parser3") {
|
||||
@@ -314,10 +334,17 @@ TEST_CASE("acceptance is independent of the chunking") {
|
||||
// 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) {
|
||||
auto whole = parseChunked(doc, stackSize, {});
|
||||
// Every way to split the document in two, so that every literal and
|
||||
// escape in `docs` straddles a chunk boundary at some point.
|
||||
for (size_t cut = 1; cut < doc.size(); ++cut) {
|
||||
CAPTURE(cut);
|
||||
REQUIRE(parseChunked(doc, stackSize, {cut}) == whole);
|
||||
}
|
||||
for (size_t stride = 1; stride <= 12; ++stride) {
|
||||
CAPTURE(stride);
|
||||
REQUIRE(parseChunked(doc, stackSize, stride) == whole);
|
||||
REQUIRE(parseChunked(doc, stackSize, stridedCuts(doc, stride)) ==
|
||||
whole);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user