diff --git a/src/builtins.c b/src/builtins.c index 7612b342..31534074 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -981,9 +981,12 @@ Value* eigs_json_parse_value(const char *s, int *pos); * malformed input). This thread-local flag is SET by the parse functions on * any malformed condition (unexpected token, truncation, unterminated * string, over-deep nesting) and CHECKED only by builtin_json_decode, which - * resets it on entry and raises. Other callers (json_path, ext_http) ignore - * it and keep the historical lenient behavior. Thread-local like g_json_depth - * because JSON parsing is per-thread and non-reentrant across threads. */ + * raises on it. It is also checked mid-parse by the array/object propagation + * guards, so it must be CLEAR at the start of every fresh parse — that reset + * is owned by eigs_json_parse_root (#777), never by individual callers. + * Callers other than json_decode don't read it after parsing and keep the + * historical lenient behavior. Thread-local like g_json_depth because JSON + * parsing is per-thread and non-reentrant across threads. */ static __thread int g_json_parse_err = 0; /* #724: recoverable-parse flag — a SECOND channel, parallel to @@ -998,12 +1001,31 @@ static __thread int g_json_parse_err = 0; * trading the old CESU-8-in-one-field bug for a lost-fields bug. Recoverable * conditions therefore never touch g_json_parse_err: lenient callers receive * the complete document with U+FFFD in the one bad string, and only - * builtin_json_decode reads this flag (reset on entry, alongside - * g_json_parse_err) to raise under strict decode. A side benefit: a strict - * decode that raises on a recoverable condition leaves g_json_parse_err - * CLEAR, so it does not poison the next lenient parse in the same thread. */ + * builtin_json_decode reads this flag to raise under strict decode. A side + * benefit: a strict decode that raises on a recoverable condition leaves + * g_json_parse_err CLEAR, so it does not poison the next lenient parse in + * the same thread. Like g_json_parse_err it is reset by + * eigs_json_parse_root (#777), never by individual callers. */ static __thread int g_json_parse_recoverable = 0; +/* #777: the one non-recursive entry point for a top-level JSON parse, and + * the single owner of the parse flags' lifetime. eigs_json_parse_value is + * recursive, so it cannot clear the flags itself — but the parser CHECKS + * g_json_parse_err mid-parse (the array element and object first-key + * propagation guards), so a flag left set by one malformed parse silently + * truncated arrays and emptied objects for the NEXT caller in the same + * thread (json_path, ext_http). Per-caller resets are how that bug happened + * — json_decode remembered, every other caller forgot — so both channels are + * cleared here instead: this wrapper is the one place that defines "fresh + * parse". Callers must not touch the flags directly; builtin_json_decode + * still reads them after the call to decide whether to raise, exactly as + * before. */ +Value* eigs_json_parse_root(const char *s, int *pos) { + g_json_parse_err = 0; + g_json_parse_recoverable = 0; + return eigs_json_parse_value(s, pos); +} + static void eigs_json_skip_ws(const char *s, int *pos) { while (s[*pos] && (s[*pos] == ' ' || s[*pos] == '\t' || s[*pos] == '\n' || s[*pos] == '\r')) (*pos)++; @@ -1286,16 +1308,15 @@ Value* builtin_json_decode(Value *arg) { arg ? val_type_name(arg->type) : "null"); return make_null(); } - /* #495: strict decode. Reset the parse-error flag, parse one value, then - * require that only whitespace remains. A partial container, a truncated + /* #495: strict decode. Parse one value via eigs_json_parse_root (#777: + * the wrapper resets both parse flags), then require that only + * whitespace remains. A partial container, a truncated * document, an unterminated string, over-deep nesting, or trailing * garbage after a complete value now raises instead of silently * succeeding (which also made a genuine JSON `null` indistinguishable * from a parse failure). */ - g_json_parse_err = 0; - g_json_parse_recoverable = 0; /* #724: also raise on repaired scalars */ int pos = 0; - Value *v = eigs_json_parse_value(arg->data.str, &pos); + Value *v = eigs_json_parse_root(arg->data.str, &pos); eigs_json_skip_ws(arg->data.str, &pos); if (g_json_parse_err || g_json_parse_recoverable || arg->data.str[pos] != '\0') { @@ -2902,7 +2923,7 @@ Value* builtin_json_path(Value *arg) { } int pos = 0; - Value *root = eigs_json_parse_value(json_str, &pos); + Value *root = eigs_json_parse_root(json_str, &pos); /* #777: fresh parse */ if (!root) return make_str(""); Value *current = root; /* walks borrowed children of root */ diff --git a/src/eigenscript.h b/src/eigenscript.h index a8412ec6..c14e2af8 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -1208,6 +1208,11 @@ int resolve_eigenscript_file(const char *path, char *resolved, size_t resolved_c int resolve_eigenscript_file_from(const char *base, const char *path, char *resolved, size_t resolved_cap); Value* eigs_json_parse_value(const char *s, int *pos); +/* #777: the ONLY entry point for a top-level (non-recursive) JSON parse. + * Clears both thread-local parse flags (g_json_parse_err, + * g_json_parse_recoverable) before delegating to eigs_json_parse_value, so + * one malformed parse cannot poison the next parse in the same thread. */ +Value* eigs_json_parse_root(const char *s, int *pos); /* Encode any Value as JSON. Returns heap-owned string (caller frees). * Functions/builtins emit "null" (matches the json_encode builtin). */ char* eigs_json_encode(Value *v); diff --git a/src/ext_http.c b/src/ext_http.c index b368b0ea..6cf7175d 100644 --- a/src/ext_http.c +++ b/src/ext_http.c @@ -408,7 +408,7 @@ Value* builtin_http_post(Value *arg) { char header_bufs[32][256]; /* up to 32 headers */ int hdr_count = 0; int jpos = 0; - Value *hdr_obj = eigs_json_parse_value(headers_json, &jpos); + Value *hdr_obj = eigs_json_parse_root(headers_json, &jpos); /* #777 */ /* #755: a JSON OBJECT is the shape a caller reaches for first, and it used * to send NO headers at all — this tested only VAL_LIST, while * eigs_json_parse_object returns a VAL_DICT, so the loop never ran and the @@ -620,7 +620,7 @@ Value* builtin_shared_incr(Value *arg) { double cur = 0; if (idx >= 0) { int pos = 0; - Value *parsed = eigs_json_parse_value(s->shared[idx].json, &pos); + Value *parsed = eigs_json_parse_root(s->shared[idx].json, &pos); /* #777 */ /* Read the number out BEFORE dropping the ref — decref may free it — * and drop it on the mismatch path too, which is the one an early * return makes easy to miss. */ @@ -679,7 +679,7 @@ Value* builtin_shared_get(Value *arg) { pthread_mutex_unlock(&s->shared_mu); if (!json_copy) return make_null(); int pos = 0; - Value *v = eigs_json_parse_value(json_copy, &pos); + Value *v = eigs_json_parse_root(json_copy, &pos); /* #777 */ free(json_copy); return v ? v : make_null(); } @@ -1295,7 +1295,7 @@ static void handle_request(int fd) { int idx = shared_find(srv, "require_auth"); if (idx >= 0) { int jpos = 0; - Value *parsed = eigs_json_parse_value(srv->shared[idx].json, &jpos); + Value *parsed = eigs_json_parse_root(srv->shared[idx].json, &jpos); /* #777 */ if (parsed && parsed->type == VAL_STR) { auth_src = xstrdup(parsed->data.str); } diff --git a/tests/test_json_hard.eigs b/tests/test_json_hard.eigs index e6f02612..6cf65c72 100644 --- a/tests/test_json_hard.eigs +++ b/tests/test_json_hard.eigs @@ -280,6 +280,84 @@ assert of [json_path of [len_hex, "k2"] == "ok", "JH92 sibling after malformed e len_before is "{\"k0\": \"v0\", \"k\": \"x\\ude00y\"}" assert of [json_path of [len_before, "k0"] == "v0", "JH93 sibling before bad string survives"] + +# --- #777: one malformed parse must not poison the NEXT parse in the same --- +# thread. g_json_parse_err is thread-global and the parser CHECKS it mid-parse +# (object first-key guard, array element guard); before the fix only +# json_decode reset it, so a stale flag EMPTIED objects and TRUNCATED arrays +# for every lenient caller that followed. Each case below poisons the flag +# first, then parses a fresh, well-formed document through json_path. + +# Shape: lenient-after-lenient (the issue's own repro). +pois1 is json_path of ["{\"a\": \"unterminated", "a"] +assert of [json_path of ["{\"user\": {\"name\": \"bob\"}}", "user.name"] == "bob", "JH118 lenient json_path after malformed json_path"] + +pois2 is json_path of ["{\"a\": \"unterminated", "a"] +assert of [json_path of ["[10, 20, 30]", "2"] == "30", "JH119 array element after malformed json_path"] + +# Shape: lenient-after-strict-raise — a raising json_decode leaves the flag +# set; the next lenient parse must still start fresh. (The left half of each +# assert confirms the raise really happened, so the case cannot pass +# vacuously.) +pois3_err is 0 +try: + pois3 is json_decode of "\"abc" +catch pois3_e: + pois3_err is 1 +assert of [(pois3_err == 1) == (json_path of ["{\"user\": {\"name\": \"alice\"}}", "user.name"] == "alice"), "JH120 lenient json_path after strict raise"] + +pois4_err is 0 +try: + pois4 is json_decode of "[1, 2" +catch pois4_e: + pois4_err is 1 +assert of [(pois4_err == 1) == (json_path of ["[7, 8, 9]", "1"] == "8"), "JH121 array element after strict raise"] + +# Shape: object-empty — the stale flag fired the object first-key guard, so +# a well-formed object decoded to an EMPTY dict and every lookup missed. +pois5 is json_path of ["{\"k\": ", "k"] +assert of [json_path of ["{\"k1\": \"v1\", \"k2\": \"v2\"}", "k1"] == "v1", "JH122 first key of object survives"] + +# Shape: array-truncate — the stale flag fired the array element guard after +# the first element, so a well-formed array decoded to a ONE-ELEMENT list. +pois6 is json_path of ["{\"k\": ", "k"] +assert of [json_path of ["[\"x\", \"y\"]", "1"] == "y", "JH123 second element of array survives"] + +# The poison source can be any malformed document shape — a truncated array +# sets the flag just like an unterminated string does. +pois7 is json_path of ["[1, 2", "0"] +assert of [json_path of ["{\"ok\": \"yes\"}", "ok"] == "yes", "JH124 object survives poison from truncated array"] + +# Two malformed parses in a row must not compound: the next good parse is +# still a fresh one. +pois8a is json_path of ["{\"a\": ", "a"] +pois8b is json_path of ["[", "0"] +assert of [json_path of ["{\"deep\": {\"leaf\": 42}}", "deep.leaf"] == "42", "JH125 nested path after double poison"] + +# A bare non-JSON token poisons too; a three-level path must still resolve. +pois9 is json_path of ["not json at all", "x"] +assert of [json_path of ["{\"m\": {\"n\": {\"o\": \"end\"}}}", "m.n.o"] == "end", "JH126 three-level path after poison"] + +# The RECOVERABLE channel is load-bearing in the wrapper too: a lenient +# parse over a recoverable document (unpaired surrogate, U+FFFD substituted) +# leaves g_json_parse_recoverable set, and a STRICT decode of a perfectly +# clean document that follows must still start fresh and NOT raise. This +# fails if the "g_json_parse_recoverable = 0;" line is removed from +# eigs_json_parse_root. (On pre-#777 code json_decode cleared both flags on +# its own entry, so this exact scenario was safe then — this test guards the +# wrapper, not the old reset site.) +rec_doc is "{\"k\": \"x\\ude00y\"}" +rec_lenient is json_path of [rec_doc, "k"] +rec_strict_err is 0 +try: + rec_strict is json_decode of "{\"clean\": 1}" +catch rec_strict_e: + rec_strict_err is 1 +assert of [rec_strict_err == 0, "JH127 clean strict decode after recoverable lenient parse"] + +# Leave the flags clean for the tests that follow even on an unfixed build: +# json_decode resets them on entry (that reset is the pre-#777 behavior). +pois_cleanup is json_decode of "{}" # --- #725: json_build/json_path must keep magnitude (%.15g, like json_encode) --- # The old %.6f destroyed tiny values ({"v": 0.000000}) and printed huge ones # as 22-char fixed-point blobs. json_encode already used %.15g.