fix: accept '-' character when parsing json fields (#2271)

Fixes #2265
Also switch to our own fork of jsoncons instead of using patches.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

patches/jsoncons-v0.171.0.patch

@@ -1,110 +0,0 @@
-diff --git a/include/jsoncons/json_encoder.hpp b/include/jsoncons/json_encoder.hpp
-index 6a1daba63..d20673171 100644
---- a/include/jsoncons/json_encoder.hpp
-+++ b/include/jsoncons/json_encoder.hpp
-@@ -355,6 +355,7 @@ namespace detail {
-                     colon_str_.push_back(':');
-                     break;
-             }
-+            colon_str_.append(options.after_key_chars());
-             switch (options.spaces_around_comma())
-             {
-                 case spaces_option::space_after:
-@@ -1021,9 +1022,9 @@ namespace detail {
-             sink_.append(options_.new_line_chars().data(),options_.new_line_chars().length());
-             for (int i = 0; i < indent_amount_; ++i)
-             {
--                sink_.push_back(' ');
-+                sink_.append(options_.indent_chars().data(), options_.indent_chars().length());
-             }
--            column_ = indent_amount_;
-+            column_ = indent_amount_ * options_.new_line_chars().length();
-         }
- 
-         void new_line(std::size_t len)
-@@ -1031,7 +1032,7 @@ namespace detail {
-             sink_.append(options_.new_line_chars().data(),options_.new_line_chars().length());
-             for (std::size_t i = 0; i < len; ++i)
-             {
--                sink_.push_back(' ');
-+                sink_.append(options_.indent_chars().data(), options_.indent_chars().length());
-             }
-             column_ = len;
-         }
-diff --git a/include/jsoncons/json_options.hpp b/include/jsoncons/json_options.hpp
-index 58dcf3ba3..74d5ab217 100644
---- a/include/jsoncons/json_options.hpp
-+++ b/include/jsoncons/json_options.hpp
-@@ -425,6 +425,8 @@ private:
-     uint8_t indent_size_;
-     std::size_t line_length_limit_;
-     string_type new_line_chars_;
-+    string_type after_key_chars_;
-+    string_type indent_chars_;
- public:
-     basic_json_encode_options()
-         : escape_all_non_ascii_(false),
-@@ -445,6 +447,7 @@ public:
-           line_length_limit_(line_length_limit_default)
-     {
-         new_line_chars_.push_back('\n');
-+        indent_chars_.push_back('\t');
-     }
- 
-     basic_json_encode_options(const basic_json_encode_options&) = default;
-@@ -467,7 +470,9 @@ public:
-           precision_(other.precision_),
-           indent_size_(other.indent_size_),
-           line_length_limit_(other.line_length_limit_),
--          new_line_chars_(std::move(other.new_line_chars_))
-+          new_line_chars_(std::move(other.new_line_chars_)),
-+          after_key_chars_(std::move(other.after_key_chars_)),
-+          indent_chars_(std::move(other.indent_chars))
-     {
-     }
- 
-@@ -515,6 +520,16 @@ public:
-         return new_line_chars_;
-     }
- 
-+    string_type after_key_chars() const
-+    {
-+        return after_key_chars_;
-+    }
-+
-+    string_type indent_chars() const
-+    {
-+        return indent_chars_;
-+    }
-+
-     std::size_t line_length_limit() const 
-     {
-         return line_length_limit_;
-@@ -599,6 +614,8 @@ public:
-     using basic_json_encode_options<CharT>::pad_inside_object_braces;
-     using basic_json_encode_options<CharT>::pad_inside_array_brackets;
-     using basic_json_encode_options<CharT>::new_line_chars;
-+    using basic_json_encode_options<CharT>::after_key_chars;
-+    using basic_json_encode_options<CharT>::indent_chars;
-     using basic_json_encode_options<CharT>::line_length_limit;
-     using basic_json_encode_options<CharT>::float_format;
-     using basic_json_encode_options<CharT>::precision;
-@@ -761,6 +778,18 @@ public:
-         return *this;
-     }
- 
-+    basic_json_options& after_key_chars(const string_type& value)
-+    {
-+        this->after_key_chars_ = value;
-+        return *this;
-+    }
-+
-+    basic_json_options& indent_chars(const string_type& value)
-+    {
-+        this->indent_chars_ = value;
-+        return *this;
-+    }
-+
-     basic_json_options& lossless_number(bool value) 
-     {
-         this->lossless_number_ = value;

src/CMakeLists.txt

@@ -72,8 +72,11 @@ set(REFLEX "${THIRD_PARTY_LIB_DIR}/reflex/bin/reflex")
 
 add_third_party(
   jsoncons
-  URL https://github.com/danielaparker/jsoncons/archive/refs/tags/v0.171.1.tar.gz
-  PATCH_COMMAND patch -p1 -i "${CMAKE_SOURCE_DIR}/patches/jsoncons-v0.171.0.patch"
+  GIT_REPOSITORY https://github.com/dragonflydb/jsoncons
+  # URL https://github.com/danielaparker/jsoncons/archive/refs/tags/v0.171.1.tar.gz
+  GIT_TAG Dragonfly
+  GIT_SHALLOW 1
+  # PATCH_COMMAND patch -p1 -i "${CMAKE_SOURCE_DIR}/patches/jsoncons-v0.171.0.patch"
   CMAKE_PASS_FLAGS "-DJSONCONS_BUILD_TESTS=OFF -DJSONCONS_HAS_POLYMORPHIC_ALLOCATOR=ON"
   LIB "none"
 )

src/core/json_test.cc

@@ -85,18 +85,15 @@ TEST_F(JsonTest, Path) {
   EXPECT_FALSE(ec);
 
   expr.evaluate(j1, [](const std::string& path, const json& val) {
-    ASSERT_EQ("$", path);
+    ASSERT_EQ("$['field']", path);
     ASSERT_EQ(1, val.as<int>());
   });
 
   expr = jsonpath::make_expression<json>("$.field-dash", ec);
-  EXPECT_TRUE(ec);  // can not parse '-'
+  ASSERT_FALSE(ec);  // parses '-'
 
-  ec = {};
-  expr = jsonpath::make_expression<json>("$.'field-dash'", ec);
-  ASSERT_FALSE(ec);
   expr.evaluate(j1, [](const std::string& path, const json& val) {
-    ASSERT_EQ("$", path);
+    ASSERT_EQ("$['field-dash']", path);
     ASSERT_EQ(2, val.as<int>());
   });
 }

src/server/json_family.cc

@@ -38,7 +38,7 @@ using OptBool = optional<bool>;
 using OptLong = optional<long>;
 using OptSizeT = optional<size_t>;
 using OptString = optional<string>;
-using JsonReplaceCb = function<void(const string&, JsonType&)>;
+using JsonReplaceCb = function<void(const JsonExpression::path_node_type&, JsonType&)>;
 using JsonReplaceVerify = std::function<OpStatus(JsonType&)>;
 using CI = CommandId;
 
@@ -119,28 +119,28 @@ void PrintOptVec(ConnectionContext* cntx, const OpResult<vector<optional<T>>>& r
 }
 
 error_code JsonReplace(JsonType& instance, string_view path, JsonReplaceCb callback) {
-  using evaluator_t = jsoncons::jsonpath::detail::jsonpath_evaluator<JsonType, JsonType&>;
+  using evaluator_t = jsonpath::detail::jsonpath_evaluator<JsonType, JsonType&>;
   using value_type = evaluator_t::value_type;
   using reference = evaluator_t::reference;
   using json_selector_t = evaluator_t::path_expression_type;
-  using json_location_type = evaluator_t::json_location_type;
+
   jsonpath::custom_functions<JsonType> funcs = jsonpath::custom_functions<JsonType>();
 
   error_code ec;
-  jsoncons::jsonpath::detail::static_resources<value_type, reference> static_resources(funcs);
+  jsonpath::detail::static_resources<value_type, reference> static_resources(funcs);
   evaluator_t e;
   json_selector_t expr = e.compile(static_resources, path, ec);
   if (ec) {
     return ec;
   }
 
-  jsoncons::jsonpath::detail::dynamic_resources<value_type, reference> resources;
-  auto f = [&callback](const json_location_type& path, reference val) {
-    callback(path.to_string(), val);
+  jsonpath::detail::dynamic_resources<value_type, reference> resources;
+  auto f = [&callback](const json_selector_t::path_node_type& path, reference val) {
+    callback(path, val);
   };
 
-  expr.evaluate(resources, instance, resources.root_path_node(), instance, f,
-                jsonpath::result_options::nodups);
+  expr.evaluate(resources, instance, json_selector_t::path_node_type{}, instance, f,
+                jsonpath::result_options::nodups | jsonpath::result_options::path);
   return ec;
 }
 
@@ -532,7 +532,7 @@ OpResult<vector<OptSizeT>> OpArrLen(const OpArgs& op_args, string_view key,
 
 OpResult<vector<OptBool>> OpToggle(const OpArgs& op_args, string_view key, string_view path) {
   vector<OptBool> vec;
-  auto cb = [&vec](const string& path, JsonType& val) {
+  auto cb = [&vec](const auto&, JsonType& val) {
     if (val.is_bool()) {
       bool current_val = val.as_bool() ^ true;
       val = current_val;
@@ -558,7 +558,7 @@ OpResult<string> OpDoubleArithmetic(const OpArgs& op_args, string_view key, stri
   bool has_fractional_part = (modf(num, &int_part) != 0);
   json output(json_array_arg);
 
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     if (val.is_number()) {
       double result = arithmetic_op(val.as<double>(), num);
       if (isinf(result)) {
@@ -607,9 +607,10 @@ OpResult<long> OpDel(const OpArgs& op_args, string_view key, string_view path) {
   }
 
   vector<string> deletion_items;
-  auto cb = [&](const string& path, JsonType& val) { deletion_items.emplace_back(path); };
+  auto cb = [&](const JsonExpression::path_node_type& path, JsonType& val) {
+    deletion_items.emplace_back(jsonpath::to_string(path));
+  };
 
-  // json j = move(result.value());
   JsonType& json_entry = *(result.value());
   error_code ec = JsonReplace(json_entry, path, cb);
   if (ec) {
@@ -672,7 +673,7 @@ OpResult<vector<StringVec>> OpObjKeys(const OpArgs& op_args, string_view key,
 OpResult<vector<OptSizeT>> OpStrAppend(const OpArgs& op_args, string_view key, string_view path,
                                        const vector<string_view>& strs) {
   vector<OptSizeT> vec;
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     if (val.is_string()) {
       string new_val = val.as_string();
       for (auto& str : strs) {
@@ -698,7 +699,7 @@ OpResult<vector<OptSizeT>> OpStrAppend(const OpArgs& op_args, string_view key, s
 // Clears containers(arrays or objects) and zeroing numbers.
 OpResult<long> OpClear(const OpArgs& op_args, string_view key, string_view path) {
   long clear_items = 0;
-  auto cb = [&clear_items](const string& path, JsonType& val) {
+  auto cb = [&clear_items](const auto& path, JsonType& val) {
     if (!(val.is_object() || val.is_array() || val.is_number())) {
       return;
     }
@@ -725,7 +726,7 @@ OpResult<long> OpClear(const OpArgs& op_args, string_view key, string_view path)
 OpResult<vector<OptString>> OpArrPop(const OpArgs& op_args, string_view key, string_view path,
                                      int index) {
   vector<OptString> vec;
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto& path, JsonType& val) {
     if (!val.is_array() || val.empty()) {
       vec.emplace_back(nullopt);
       return;
@@ -768,7 +769,7 @@ OpResult<vector<OptString>> OpArrPop(const OpArgs& op_args, string_view key, str
 OpResult<vector<OptSizeT>> OpArrTrim(const OpArgs& op_args, string_view key, string_view path,
                                      int start_index, int stop_index) {
   vector<OptSizeT> vec;
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     if (!val.is_array()) {
       vec.emplace_back(nullopt);
       return;
@@ -824,7 +825,7 @@ OpResult<vector<OptSizeT>> OpArrInsert(const OpArgs& op_args, string_view key, s
   // Insert user-supplied value into the supplied index that should be valid.
   // If at least one index isn't valid within an array in the json doc, the operation is discarded.
   // Negative indexes start from the end of the array.
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     if (out_of_boundaries_encountered) {
       return;
     }
@@ -889,7 +890,7 @@ OpResult<vector<OptSizeT>> OpArrAppend(const OpArgs& op_args, string_view key, s
     return result.status();
   }
 
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     if (!val.is_array()) {
       vec.emplace_back(nullopt);
       return;
@@ -1091,7 +1092,7 @@ OpResult<bool> OpSet(const OpArgs& op_args, string_view key, string_view path,
   bool path_exists = false;
   bool operation_result = false;
   const JsonType& new_json = parsed_json.value();
-  auto cb = [&](const string& path, JsonType& val) {
+  auto cb = [&](const auto&, JsonType& val) {
     path_exists = true;
     if (!is_nx_condition) {
       operation_result = true;