From 7f8151fb64e5e440f71542b0d878629509c09c30 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Thu, 9 Jul 2026 01:36:29 -0500 Subject: [PATCH] change: json_decode rejects malformed JSON instead of silent partial (#495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recursive-descent JSON parser was lenient: arrays/objects `break` on an unexpected token and returned the *partial* container, numbers stopped at the first non-numeric char, and json_decode never checked for trailing input. So truncated documents ("[1,2,", "{\"a\":"), partial/garbage containers ("{]", "[1,2,3,]"), unterminated strings, over-deep nesting, and trailing garbage ("[1,2,3] trailing", "42abc") all succeeded with rc=0 — and a genuine JSON `null` was indistinguishable from a parse failure (both returned null). Add a thread-local strict-parse error flag that the parse functions SET on any malformed condition; builtin_json_decode resets it on entry, and after parsing one value requires that only whitespace remains, raising `value` otherwise. Valid JSON is unchanged. The flag is CHECKED only by json_decode — json_path and the ext_http header/body parsers call the same parser but ignore the flag, keeping their historical lenient behavior. Load-bearing check: nothing tested `json_decode == null` for validity, and every stdlib/test/example feeds valid JSON — zero suite tests broke except test_json_depth (a >200-deep document now raises catchably rather than returning a truncated value; the security property "no SIGSEGV" is preserved and re-asserted). Regression: suite section [116] extended (test_raise_on_bad_input.eigs, now 43 checks — truncated/partial/garbage raise; valid arrays/objects and a bare `null` still parse). Closes #495 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 11 ++++++ src/builtins.c | 63 ++++++++++++++++++++++++------ tests/run_all_tests.sh | 7 +++- tests/test_json_depth.eigs | 13 ++++-- tests/test_raise_on_bad_input.eigs | 46 ++++++++++++++++++++++ 5 files changed, 124 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afec96a..b8bcab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,17 @@ All notable changes to EigenScript are documented here. raises `value` instead of being silently skipped by `strtok`, which masked a malformed path; a genuine lookup miss still returns `""` (#507). + Batch 2c — `json_decode` strictness (#495): + - `json_decode` rejects malformed input with a `value` error instead of + silently succeeding. A truncated document (`[1, 2,`, `{"a":`), an + unterminated string, a partial/garbage container (`{]`, `[1,2,3,]`), + over-deep nesting, and trailing garbage after a complete value + (`[1,2,3] trailing`, `42abc`) all used to return a partial value with + `rc=0` — and a genuine JSON `null` was indistinguishable from a parse + failure (both returned `null`). Valid JSON is unchanged. `json_path` and + the HTTP header/body parsers keep their lenient behavior (they ignore the + strict-parse flag) (#495). + ### Fixed - **Circular `import` / `load_file` no longer crashes (#496).** A mutual or self-referential `import` (a→b→a) or `load_file` used to recurse through diff --git a/src/builtins.c b/src/builtins.c index 523ed3b..b1ce256 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -827,13 +827,23 @@ char* eigs_json_encode(Value *v) { Value* eigs_json_parse_value(const char *s, int *pos); +/* #495: strict-parse error flag. The recursive-descent parser has no error + * channel (it returns a Value* and, historically, a partial container on + * 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. */ +static __thread int g_json_parse_err = 0; + 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)++; } static Value* eigs_json_parse_string(const char *s, int *pos) { - if (s[*pos] != '"') return NULL; + if (s[*pos] != '"') { g_json_parse_err = 1; return NULL; } /* #495 */ (*pos)++; strbuf buf; strbuf_init(&buf); @@ -875,6 +885,7 @@ static Value* eigs_json_parse_string(const char *s, int *pos) { (*pos)++; } if (s[*pos] == '"') (*pos)++; + else g_json_parse_err = 1; /* #495: unterminated string (hit EOF) */ Value *v = make_str(buf.data); strbuf_free(&buf); return v; @@ -906,11 +917,14 @@ static Value* eigs_json_parse_array(const char *s, int *pos) { eigs_json_skip_ws(s, pos); Value *val = eigs_json_parse_value(s, pos); if (val) list_append_owned(list, val); + if (g_json_parse_err) return list; /* #495: propagate */ eigs_json_skip_ws(s, pos); if (s[*pos] == ',') { (*pos)++; continue; } - if (s[*pos] == ']') { (*pos)++; break; } - break; + if (s[*pos] == ']') { (*pos)++; return list; } + g_json_parse_err = 1; /* #495: expected ',' or ']' */ + return list; } + g_json_parse_err = 1; /* #495: hit EOF before ']' (truncated) */ return list; } @@ -922,37 +936,49 @@ static Value* eigs_json_parse_object(const char *s, int *pos) { while (s[*pos]) { eigs_json_skip_ws(s, pos); Value *key = eigs_json_parse_string(s, pos); - if (!key) break; + if (!key || g_json_parse_err) { /* #495: bad/absent key */ + if (key) val_decref(key); + g_json_parse_err = 1; + return dict; + } eigs_json_skip_ws(s, pos); - if (s[*pos] == ':') (*pos)++; + if (s[*pos] != ':') { /* #495: missing colon */ + val_decref(key); + g_json_parse_err = 1; + return dict; + } + (*pos)++; eigs_json_skip_ws(s, pos); Value *val = eigs_json_parse_value(s, pos); dict_set_owned(dict, key->data.str, val ? val : make_null()); val_decref(key); /* dict interns its own copy of the key */ + if (g_json_parse_err) return dict; /* #495: propagate */ eigs_json_skip_ws(s, pos); if (s[*pos] == ',') { (*pos)++; continue; } - if (s[*pos] == '}') { (*pos)++; break; } - break; + if (s[*pos] == '}') { (*pos)++; return dict; } + g_json_parse_err = 1; /* #495: expected ',' or '}' */ + return dict; } + g_json_parse_err = 1; /* #495: hit EOF before '}' (truncated) */ return dict; } Value* eigs_json_parse_value(const char *s, int *pos) { eigs_json_skip_ws(s, pos); - if (!s[*pos]) return make_null(); + if (!s[*pos]) { g_json_parse_err = 1; return make_null(); } /* #495: value expected, hit EOF */ if (s[*pos] == '"') return eigs_json_parse_string(s, pos); /* Refuse to descend past the nesting limit (stack-exhaustion guard). * The enclosing array/object loop breaks on the unconsumed bracket, so * parsing terminates cleanly instead of crashing. */ if (s[*pos] == '[') { - if (g_json_depth >= JSON_MAX_DEPTH) return make_null(); + if (g_json_depth >= JSON_MAX_DEPTH) { g_json_parse_err = 1; return make_null(); } g_json_depth++; Value *v = eigs_json_parse_array(s, pos); g_json_depth--; return v; } if (s[*pos] == '{') { - if (g_json_depth >= JSON_MAX_DEPTH) return make_null(); + if (g_json_depth >= JSON_MAX_DEPTH) { g_json_parse_err = 1; return make_null(); } g_json_depth++; Value *v = eigs_json_parse_object(s, pos); g_json_depth--; @@ -962,6 +988,7 @@ Value* eigs_json_parse_value(const char *s, int *pos) { if (strncmp(s + *pos, "null", 4) == 0) { *pos += 4; return make_null(); } if (strncmp(s + *pos, "true", 4) == 0) { *pos += 4; return make_num(1); } if (strncmp(s + *pos, "false", 5) == 0) { *pos += 5; return make_num(0); } + g_json_parse_err = 1; /* #495: unrecognized token */ return make_null(); } @@ -971,8 +998,22 @@ 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 + * 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; int pos = 0; - return eigs_json_parse_value(arg->data.str, &pos); + Value *v = eigs_json_parse_value(arg->data.str, &pos); + eigs_json_skip_ws(arg->data.str, &pos); + if (g_json_parse_err || arg->data.str[pos] != '\0') { + if (v) val_decref(v); + rt_error(EK_VALUE, 0, "json_decode: invalid JSON at position %d", pos); + return make_null(); + } + return v; } Value* builtin_coalesce(Value *arg) { diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index fdf2229..cf6e833 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1032,9 +1032,12 @@ rm -f "$CPG_FILE" # pattern → value, non-string → type_mismatch), substr (negative start counts # from the end), list_truncate (negative len → value), json_path (empty path # segment → value). -echo "[116] Silent-Tolerance Batch-2: bad input raises (13 issues)" +# Batch-2c adds #495: json_decode rejects truncated / partial / trailing- +# garbage JSON (was a silent partial value; also made a genuine `null` +# indistinguishable from a parse failure). +echo "[116] Silent-Tolerance Batch-2: bad input raises (14 issues)" check_eigs_suite "invalid input raises instead of silent null/0/empty" \ - test_raise_on_bad_input.eigs "ALL_RAISE_TESTS_DONE" 35 + test_raise_on_bad_input.eigs "ALL_RAISE_TESTS_DONE" 43 # [117] for-in snapshots the iteration length at loop entry (#491). Mutating # the sequence in the body is well-defined: appending no longer loops forever diff --git a/tests/test_json_depth.eigs b/tests/test_json_depth.eigs index dec9feb..ec88445 100644 --- a/tests/test_json_depth.eigs +++ b/tests/test_json_depth.eigs @@ -10,9 +10,16 @@ loop while i < 5000: deep is "[" + deep i is i + 1 -# This must return (truncated/null) rather than crash. Reaching the next -# line at all is the assertion. -result is json_decode of deep +# This must not crash (SIGSEGV). #495 made the parser strict, so over-deep +# nesting now RAISES (catchable) instead of returning a truncated value — +# either way the security property holds: the process survives. Reaching the +# line after the catch at all is the assertion. +deep_raised is 0 +try: + result is json_decode of deep +catch e: + deep_raised is 1 +assert of [deep_raised == 1, "over-deep JSON raises rather than crashing"] print of "survived deeply nested json" # Normal nested JSON within the limit still parses correctly. diff --git a/tests/test_raise_on_bad_input.eigs b/tests/test_raise_on_bad_input.eigs index acc5b38..bfe725b 100644 --- a/tests/test_raise_on_bad_input.eigs +++ b/tests/test_raise_on_bad_input.eigs @@ -233,5 +233,51 @@ assert_true of [jp_empty == 1 and jp_empty_k == "value", "json_path empty segmen assert_eq of [json_path of ["{\"a\": {\"b\": 5}}", "a.b"], "5", "json_path with a valid path still works"] +# ---- #495 json_decode: malformed JSON raises (was a silent partial value) ---- +jd_trunc is 0 +jd_trunc_k is "" +try: + ig is json_decode of "[1, 2," +catch e: + jd_trunc is 1 + jd_trunc_k is e.kind +assert_true of [jd_trunc == 1 and jd_trunc_k == "value", "json_decode truncated array raises value"] + +jd_garbage is 0 +try: + ig is json_decode of "[1, 2, 3] trailing" +catch e: + jd_garbage is 1 +assert_true of [jd_garbage == 1, "json_decode trailing garbage raises"] + +jd_partial_obj is 0 +try: + ig is json_decode of "{\"a\":" +catch e: + jd_partial_obj is 1 +assert_true of [jd_partial_obj == 1, "json_decode truncated object raises"] + +jd_num_garbage is 0 +try: + ig is json_decode of "42abc" +catch e: + jd_num_garbage is 1 +assert_true of [jd_num_garbage == 1, "json_decode number with trailing garbage raises"] + +jd_empty is 0 +try: + ig is json_decode of "" +catch e: + jd_empty is 1 +assert_true of [jd_empty == 1, "json_decode of an empty string raises"] + +# valid JSON — including a bare null, which must stay distinguishable from a +# parse failure (the whole point of #495) +assert_eq of [len of (json_decode of "[1, 2, 3]"), 3, "json_decode of a valid array still works"] +jd_ok_null is json_decode of "null" +assert_true of [(type of jd_ok_null) == "none", "json_decode of a bare null still returns null"] +jd_ok_obj is json_decode of "{\"a\": 1, \"b\": [2, 3]}" +assert_true of [jd_ok_obj.a == 1 and (len of jd_ok_obj.b) == 2, "json_decode of a valid nested object still works"] + print of "ALL_RAISE_TESTS_DONE" test_summary of null