From da540f35eac87f4320cb10831e11d34a67c56fec Mon Sep 17 00:00:00 2001 From: linhongyu510 <204567536+linhongyu510@users.noreply.github.com> Date: Sat, 19 Sep 2026 18:26:55 +0800 Subject: [PATCH 1/3] fix(jsonpath): return a null object member instead of skipping it on-demand GetByJsonPathOnDemand reported no match for a null object member, while GetByJsonPath returned "null" for the same document and path: {"a":null} $.a DOM "null" on-demand no match {"a":{"b":null}} $.a.b DOM "null" on-demand no match Arrays were unaffected -- $[0] over [null] already returned "null" -- so the two backends disagreed depending only on whether the null sat under a key or an index. In the object branch, once advanceKey() matched, a null value was skipped unconditionally. Skipping is only correct while the path still has steps left, because nothing can be selected out of null; when the key is the final step the null *is* the requested value. The array element path never had this problem because it recurses straight into getJsonPath(), where the path_is_nil branch emits the value. The skip is kept unconditional for FLATTEN/QUOTE, which is how wildcard collection runs: JsonPath.WildcardFiltersNull asserts $.root[*].a drops nulls and yields ["foo","bar"], and that behaviour is unchanged. Only RAW, the single-result style, now distinguishes "path ends here" from "path continues". Found with a differential fuzzer that generates random documents plus every path into them and compares the two backends. Before: 3928 mismatches out of 135242 (json, path) pairs, all of them a null object member. After: 0. Also replayed all 94 path literals from tests/jsonpath_test.cpp against 11 documents (1034 rows): byte-identical to upstream, since none of those documents pairs a key directly with null. --- include/sonic/internal/arch/simd_skip.h | 16 ++++++--- tests/jsonpath_test.cpp | 47 +++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/include/sonic/internal/arch/simd_skip.h b/include/sonic/internal/arch/simd_skip.h index 1f4ec258..d0a0bfa1 100644 --- a/include/sonic/internal/arch/simd_skip.h +++ b/include/sonic/internal/arch/simd_skip.h @@ -959,15 +959,21 @@ class SkipScanner2 { } if (found) { - // if not null - if (peek() != 'n') { - return getJsonPath( - path, index + 1, jsonGenerator, jsonGeneratorFactory); - } else { + // A null value only has to be skipped when the path continues past it, + // because nothing can be selected out of null. When the key is the last + // step, null is the result the caller asked for, so recurse and let the + // path_is_nil branch emit it -- exactly what the array element path + // already does for e.g. "$[0]" over "[null]". + // Under FLATTEN/QUOTE this call is collecting wildcard results, where + // nulls are deliberately dropped (see JsonPath.WildcardFiltersNull), + // so the skip must stay unconditional there. + if (peek() == 'n' && (style != RAW || index + 1 < path.size())) { // skip null RETURN_FALSE_IF_PARSE_ERROR(skipOne()); return false; } + return getJsonPath( + path, index + 1, jsonGenerator, jsonGeneratorFactory); } return false; } diff --git a/tests/jsonpath_test.cpp b/tests/jsonpath_test.cpp index acf91540..00bc84c2 100644 --- a/tests/jsonpath_test.cpp +++ b/tests/jsonpath_test.cpp @@ -767,6 +767,53 @@ TEST(JsonPathDom, NotFoundAndUnsupportedPath) { EXPECT_EQ(std::get<0>(unsupported), ""); } +TEST(JsonPathOnDemand, NullObjectMemberIsReturnedNotSkipped) { + // A null object member is a value, not a missing member. The on-demand + // backend skipped it whenever a key matched, so $.a over {"a":null} reported + // no match while the DOM backend returned "null" for the same input. + // The null only has to be skipped when the path continues past it, since + // nothing can be selected out of null. + struct Case { + const char* json; + const char* path; + }; + for (const auto& c : + {Case{R"({"a":null})", "$.a"}, Case{R"({"a":null,"b":1})", "$.a"}, + Case{R"({"b":1,"a":null})", "$.a"}, + Case{R"({"a":{"b":null}})", "$.a.b"}, + Case{R"({"a":[{"b":null}]})", "$.a[0].b"}}) { + auto od = GetByJsonPathOnDemand(c.json, c.path); + EXPECT_EQ(std::get<1>(od), kErrorNone) << c.json << " " << c.path; + EXPECT_EQ(std::get<0>(od), "null") << c.json << " " << c.path; + // Both backends must agree. + auto dom = GetByJsonPath(c.json, c.path); + EXPECT_EQ(std::get<1>(dom), std::get<1>(od)) << c.json << " " << c.path; + EXPECT_EQ(std::get<0>(dom), std::get<0>(od)) << c.json << " " << c.path; + } + + // Arrays already behaved correctly; keep them covered so the two stay + // aligned. + for (const auto& c : {Case{"[null]", "$[0]"}, Case{"[1,null]", "$[1]"}}) { + auto od = GetByJsonPathOnDemand(c.json, c.path); + EXPECT_EQ(std::get<1>(od), kErrorNone) << c.json << " " << c.path; + EXPECT_EQ(std::get<0>(od), "null") << c.json << " " << c.path; + } + + // The path continuing past a null must still not match. + for (const auto& c : + {Case{R"({"a":null})", "$.a.b"}, Case{R"({"a":null})", "$.a[0]"}, + Case{R"({"a":{"b":null}})", "$.a.b.c"}}) { + auto od = GetByJsonPathOnDemand(c.json, c.path); + EXPECT_NE(std::get<1>(od), kErrorNone) << c.json << " " << c.path; + } + + // Wildcard collection deliberately drops nulls; that must not change. + auto wc = GetByJsonPathOnDemand( + R"({"root": [{"a":null},{"a":"foo"},{"a":"bar"}]})", "$.root[*].a"); + EXPECT_EQ(std::get<1>(wc), kErrorNone); + EXPECT_EQ(std::get<0>(wc), R"(["foo","bar"])"); +} + TEST(JsonPathDom, NegativeIndexSupportedButOnDemandUnsupported) { auto dom_got = GetByJsonPath("[0,1,2]", "$[-1]"); EXPECT_EQ(std::get<1>(dom_got), kErrorNone); From 381c6a55bb28fece559ff91f7fb65700dfba75d1 Mon Sep 17 00:00:00 2001 From: linhongyu Date: Sat, 3 Oct 2026 17:09:50 +0800 Subject: [PATCH 2/3] test(jsonpath): update KeyNullElementPair to expect null from on-demand The on-demand backend now returns "null" for a null object member instead of skipping it (aligned with the DOM backend, see NullObjectMemberIsReturnedNotSkipped). The pre-existing KeyNullElementPair still asserted the old empty-string expectation, so it failed locally. --- tests/jsonpath_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/jsonpath_test.cpp b/tests/jsonpath_test.cpp index 00bc84c2..cc0df275 100644 --- a/tests/jsonpath_test.cpp +++ b/tests/jsonpath_test.cpp @@ -415,7 +415,7 @@ TEST(JsonPath, BadCases) { TEST(JsonPath, KeyNullElementPair) { auto json = R"( { "last_name": "Kim", "first_name":"Sam", "gender": false, "income": null, "age": 38} )"; - TestOk(json, "$.income", ""); + TestOk(json, "$.income", "null"); // null object member returned, not skipped (aligns with DOM) } TEST(JsonPath, KeyIntoStringValue) { From 4f0d38e389952aac59d13529811511cd8fff28e8 Mon Sep 17 00:00:00 2001 From: linhongyu Date: Mon, 12 Oct 2026 14:38:49 +0800 Subject: [PATCH 3/3] style(jsonpath_test): wrap TestOk call per clang-format Google style --- tests/jsonpath_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/jsonpath_test.cpp b/tests/jsonpath_test.cpp index cc0df275..5894d719 100644 --- a/tests/jsonpath_test.cpp +++ b/tests/jsonpath_test.cpp @@ -415,7 +415,8 @@ TEST(JsonPath, BadCases) { TEST(JsonPath, KeyNullElementPair) { auto json = R"( { "last_name": "Kim", "first_name":"Sam", "gender": false, "income": null, "age": 38} )"; - TestOk(json, "$.income", "null"); // null object member returned, not skipped (aligns with DOM) + TestOk(json, "$.income", + "null"); // null object member returned, not skipped (aligns with DOM) } TEST(JsonPath, KeyIntoStringValue) {