From aab138a10c6c3d5065482a2bf72cc7f3bba9d6dc Mon Sep 17 00:00:00 2001
From: linhongyu510 <204567536+linhongyu510@users.noreply.github.com>
Date: Sat, 19 Sep 2026 01:56:57 +0800
Subject: [PATCH] fix(jsonpath): report unsupported, not no-match, for negative
 index on-demand

A negative array index cannot be resolved by the on-demand scanner, which is
forward-only. That limitation is intended and already covered by
JsonPathDom.NegativeIndexSupportedButOnDemandUnsupported, but the way it was
surfaced lost the reason.

getJsonPathArrayIndex() advances with `cur_idx == idx` starting from 0, so a
negative idx never matches. The scan walked the array to the end, found
nothing, and returned false without setting an error, so
GetByJsonPathOnDemand() fell through to kErrorNoneNoMatch -- the same code a
missing key produces. Callers could not tell "sonic cannot evaluate this
path" from "this document has no such member", even though the DOM backend
resolves the very same path.

The SonicError overload of getJsonPath() already guards this case with
kUnsupportedJsonPath, but no public entry point reaches that overload;
ondemand.h calls the bool overload, which routes arrays through
getJsonPathArrayIndex(). This adds the same guard there.

Behaviour change is limited to negative indices:
  $[-1] on [0,1,2]   20 (no match) -> 17 (unsupported)
  $.a[-1]            20            -> 17
An absent key ($.zz) and an out-of-range positive index ($[9]) still report
kErrorNoneNoMatch, so the two outcomes remain distinguishable. DOM behaviour
is untouched.

Verified by replaying all 94 JSON path literals used by tests/jsonpath_test.cpp
against 11 documents through both backends (1034 result rows): only the 4
negative-index rows change, all from 20 to 17. The pre-existing
EXPECT_NE(err, kErrorNone) assertion still holds.
---
 include/sonic/internal/arch/simd_skip.h | 10 +++++++++
 tests/jsonpath_test.cpp                 | 27 +++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/include/sonic/internal/arch/simd_skip.h b/include/sonic/internal/arch/simd_skip.h
index 1f4ec258..31a6371d 100644
--- a/include/sonic/internal/arch/simd_skip.h
+++ b/include/sonic/internal/arch/simd_skip.h
@@ -677,6 +677,16 @@ class SkipScanner2 {
       JsonGeneratorInterface<serializeFlags> *jsonGenerator,
       const JsonGeneratorFactory<serializeFlags> &jsonGeneratorFactory,
       const int64_t idx) {
+    // On-demand scanning is forward-only, so a negative index cannot be
+    // resolved without knowing the array length up front. Report that
+    // explicitly: the loop below compares `cur_idx == idx` with cur_idx >= 0,
+    // which a negative idx never satisfies, so without this check the scan
+    // walks the whole array and the caller receives kErrorNoneNoMatch -- the
+    // same result as a key that genuinely is not there.
+    if (idx < 0) {
+      setError(SonicError::kUnsupportedJsonPath);
+      return false;
+    }
     RETURN_FALSE_IF_PARSE_ERROR(consume('['));
     int64_t cur_idx = 0;
     bool dirty = false;
diff --git a/tests/jsonpath_test.cpp b/tests/jsonpath_test.cpp
index acf91540..d5a2df11 100644
--- a/tests/jsonpath_test.cpp
+++ b/tests/jsonpath_test.cpp
@@ -779,6 +779,33 @@ TEST(JsonPathDom, NegativeIndexSupportedButOnDemandUnsupported) {
   EXPECT_EQ(std::get<0>(ondemand_got), "");
 }
 
+TEST(JsonPathOnDemand, NegativeIndexReportsUnsupportedNotNoMatch) {
+  // A negative index is a feature this backend does not implement, which is
+  // not the same thing as the path being absent from the document. Reporting
+  // both as kErrorNoneNoMatch leaves callers unable to tell "sonic cannot
+  // evaluate this path" from "the document has no such member", so a path the
+  // DOM backend resolves fine looks merely missing here.
+  for (const char* path : {"$[-1]", "$[-3]", "$[-9]"}) {
+    auto got = GetByJsonPathOnDemand<kSerializeJavaStyleFlag>("[0,1,2]", path);
+    EXPECT_EQ(std::get<1>(got), kUnsupportedJsonPath) << "path: " << path;
+    EXPECT_EQ(std::get<0>(got), "") << "path: " << path;
+  }
+
+  // Nested arrays reach the same code through a different call site.
+  auto nested = GetByJsonPathOnDemand<kSerializeJavaStyleFlag>(
+      R"({"a":[10,11,12]})", "$.a[-1]");
+  EXPECT_EQ(std::get<1>(nested), kUnsupportedJsonPath);
+
+  // A genuinely absent member must keep reporting no-match, so the two
+  // outcomes stay distinguishable.
+  auto absent_key =
+      GetByJsonPathOnDemand<kSerializeJavaStyleFlag>(R"({"a":1})", "$.zz");
+  EXPECT_EQ(std::get<1>(absent_key), kErrorNoneNoMatch);
+  auto absent_index =
+      GetByJsonPathOnDemand<kSerializeJavaStyleFlag>("[0,1,2]", "$[9]");
+  EXPECT_EQ(std::get<1>(absent_index), kErrorNoneNoMatch);
+}
+
 TEST(JsonPathDump, SerializeCoversEmptySingleAndMulti) {
   Document doc;
   doc.Parse(R"({"a":null,"s":"abc","arr":[null,"x",1]})");