fix: issue #2154 (#2178)

simdjson · May 10, 2024 · e2f8797 · e2f8797
1 parent bf78341
commit e2f8797
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 2 deletions.
diff --git a/include/simdjson/dom/element-inl.h b/include/simdjson/dom/element-inl.h
@@ -375,6 +375,23 @@ inline simdjson_result<element> element::operator[](const char *key) const noexc
   return at_key(key);
 }
 
+inline bool is_pointer_well_formed(std::string_view json_pointer) noexcept {
+  if (simdjson_unlikely(json_pointer[0] != '/')) {
+    return false;
+  }
+  size_t escape = json_pointer.find('~');
+  if (escape == std::string_view::npos) {
+    return true;
+  }
+  if (escape == json_pointer.size() - 1) {
+    return false;
+  }
+  if (json_pointer[escape + 1] != '0' && json_pointer[escape + 1] != '1') {
+    return false;
+  }
+  return true;
+}
+
 inline simdjson_result<element> element::at_pointer(std::string_view json_pointer) const noexcept {
   SIMDJSON_DEVELOPMENT_ASSERT(tape.usable()); // https://github.com/simdjson/simdjson/issues/1914
   switch (tape.tape_ref_type()) {
@@ -383,7 +400,10 @@ inline simdjson_result<element> element::at_pointer(std::string_view json_pointe
     case internal::tape_type::START_ARRAY:
       return array(tape).at_pointer(json_pointer);
     default: {
-      if(!json_pointer.empty()) { // a non-empty string is invalid on an atom
+      if (!json_pointer.empty()) { // a non-empty string can be invalid, or accessing a primitive (issue 2154)
+        if (is_pointer_well_formed(json_pointer)) {
+          return NO_SUCH_FIELD;
+        }
         return INVALID_JSON_POINTER;
       }
       // an empty string means that we return the current node

diff --git a/include/simdjson/error.h b/include/simdjson/error.h
@@ -39,7 +39,7 @@ enum error_code {
   INDEX_OUT_OF_BOUNDS,        ///< JSON array index too large
   NO_SUCH_FIELD,              ///< JSON field not found in object
   IO_ERROR,                   ///< Error reading a file
-  INVALID_JSON_POINTER,       ///< Invalid JSON pointer reference
+  INVALID_JSON_POINTER,       ///< Invalid JSON pointer syntax
   INVALID_URI_FRAGMENT,       ///< Invalid URI fragment
   UNEXPECTED_ERROR,           ///< indicative of a bug in simdjson
   PARSER_IN_USE,              ///< parser is already in use.

diff --git a/include/simdjson/generic/ondemand/value-inl.h b/include/simdjson/generic/ondemand/value-inl.h
@@ -239,6 +239,26 @@ simdjson_inline int32_t value::current_depth() const noexcept{
   return iter.json_iter().depth();
 }
 
+inline bool is_pointer_well_formed(std::string_view json_pointer) noexcept {
+  if (simdjson_unlikely(json_pointer.empty())) { // can't be
+    return false;
+  }
+  if (simdjson_unlikely(json_pointer[0] != '/')) {
+    return false;
+  }
+  size_t escape = json_pointer.find('~');
+  if (escape == std::string_view::npos) {
+    return true;
+  }
+  if (escape == json_pointer.size() - 1) {
+    return false;
+  }
+  if (json_pointer[escape + 1] != '0' && json_pointer[escape + 1] != '1') {
+    return false;
+  }
+  return true;
+}
+
 simdjson_inline simdjson_result<value> value::at_pointer(std::string_view json_pointer) noexcept {
   json_type t;
   SIMDJSON_TRY(type().get(t));
@@ -249,6 +269,10 @@ simdjson_inline simdjson_result<value> value::at_pointer(std::string_view json_p
     case json_type::object:
       return (*this).get_object().at_pointer(json_pointer);
     default:
+      // a non-empty string can be invalid, or accessing a primitive (issue 2154)
+      if (is_pointer_well_formed(json_pointer)) {
+        return NO_SUCH_FIELD;
+      }
       return INVALID_JSON_POINTER;
   }
 }

diff --git a/tests/dom/pointercheck.cpp b/tests/dom/pointercheck.cpp
@@ -191,10 +191,40 @@ bool issue1142() {
   return true;
 }
 
+bool issue2154() { // mistakenly taking value as path should not raise INVALID_JSON_POINTER
+#if SIMDJSON_EXCEPTIONS
+  std::cout << "issue 2154" << std::endl;
+  auto example_json = R"__(
+      {
+        "obj": {
+          "s": "42",
+          "n": 42,
+          "f": 4.2
+        }
+      }
+    )__"_padded;
+  dom::parser parser;
+  dom::element example = parser.parse(example_json);
+  std::string_view sfield = example.at_pointer("/obj/s");
+  ASSERT_EQUAL(sfield, "42");
+  int64_t nfield = example.at_pointer("/obj/n");
+  ASSERT_EQUAL(nfield, 42);
+  ASSERT_ERROR(example.at_pointer("/obj/X/42").error(), NO_SUCH_FIELD);
+  ASSERT_ERROR(example.at_pointer("/obj/s/42").error(), NO_SUCH_FIELD);
+  ASSERT_ERROR(example.at_pointer("/obj/n/42").error(), NO_SUCH_FIELD);
+  ASSERT_ERROR(example.at_pointer("/obj/f/4.2").error(), NO_SUCH_FIELD);
+  ASSERT_ERROR(example.at_pointer("/obj/f/4~").error(), INVALID_JSON_POINTER);
+  ASSERT_ERROR(example.at_pointer("/obj/f/~").error(), INVALID_JSON_POINTER);
+  ASSERT_ERROR(example.at_pointer("/obj/f/~1").error(), NO_SUCH_FIELD);
+#endif
+  return true;
+}
+
 int main() {
   if (true
     && demo()
     && issue1142()
+    && issue2154()
 #ifdef SIMDJSON_ENABLE_DEPRECATED_API
     && legacy_support()
 #endif

diff --git a/tests/ondemand/ondemand_json_pointer_tests.cpp b/tests/ondemand/ondemand_json_pointer_tests.cpp
@@ -385,8 +385,38 @@ namespace json_pointer_tests {
         TEST_SUCCEED();
     }
 #endif
+
+    bool issue2154() { // mistakenly taking value as path should not raise INVALID_JSON_POINTER
+#if SIMDJSON_EXCEPTIONS
+      std::cout << "issue 2154" << std::endl;
+      auto example_json = R"__({
+            "obj": {
+                "s": "42",
+                "n": 42,
+                "f": 4.2
+            }
+        })__"_padded;
+        ondemand::parser parser;
+        ondemand::document doc;
+        ASSERT_SUCCESS(parser.iterate(example_json).get(doc));
+        std::string_view sfield = doc.at_pointer("/obj/s");
+        ASSERT_EQUAL(sfield, "42");
+        int64_t nfield = doc.at_pointer("/obj/n");
+        ASSERT_EQUAL(nfield, 42);
+        ASSERT_ERROR(doc.at_pointer("/obj/X/42").error(), NO_SUCH_FIELD);
+        ASSERT_ERROR(doc.at_pointer("/obj/s/42").error(), NO_SUCH_FIELD);
+        ASSERT_ERROR(doc.at_pointer("/obj/n/42").error(), NO_SUCH_FIELD);
+        ASSERT_ERROR(doc.at_pointer("/obj/f/4.2").error(), NO_SUCH_FIELD);
+        ASSERT_ERROR(doc.at_pointer("/obj/f/4~").error(), INVALID_JSON_POINTER);
+        ASSERT_ERROR(doc.at_pointer("/obj/f/~").error(), INVALID_JSON_POINTER);
+        ASSERT_ERROR(doc.at_pointer("/obj/f/~1").error(), NO_SUCH_FIELD);
+#endif
+      return true;
+    }
+
     bool run() {
         return
+                issue2154() &&
 #if SIMDJSON_EXCEPTIONS
                 json_pointer_invalidation_exceptions() &&
 #endif