From ff562897eba75921c0c26eccaa1769014d86c030 Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Sun, 10 Dec 2023 09:32:52 +0200 Subject: [PATCH] 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 --- patches/jsoncons-v0.171.0.patch | 110 -------------------------------- src/CMakeLists.txt | 7 +- src/core/json_test.cc | 9 +-- src/server/json_family.cc | 41 ++++++------ 4 files changed, 29 insertions(+), 138 deletions(-) delete mode 100644 patches/jsoncons-v0.171.0.patch diff --git a/patches/jsoncons-v0.171.0.patch b/patches/jsoncons-v0.171.0.patch deleted file mode 100644 index ac7c38243..000000000 --- a/patches/jsoncons-v0.171.0.patch +++ /dev/null @@ -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::pad_inside_object_braces; - using basic_json_encode_options::pad_inside_array_brackets; - using basic_json_encode_options::new_line_chars; -+ using basic_json_encode_options::after_key_chars; -+ using basic_json_encode_options::indent_chars; - using basic_json_encode_options::line_length_limit; - using basic_json_encode_options::float_format; - using basic_json_encode_options::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; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 7c9f9a311..28a9b6dd7 100644 --- a/src/CMakeLists.txt +++ b/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" ) diff --git a/src/core/json_test.cc b/src/core/json_test.cc index df1463c65..510bd9da0 100644 --- a/src/core/json_test.cc +++ b/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()); }); expr = jsonpath::make_expression("$.field-dash", ec); - EXPECT_TRUE(ec); // can not parse '-' + ASSERT_FALSE(ec); // parses '-' - ec = {}; - expr = jsonpath::make_expression("$.'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()); }); } diff --git a/src/server/json_family.cc b/src/server/json_family.cc index 343ee514f..996b7773c 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -38,7 +38,7 @@ using OptBool = optional; using OptLong = optional; using OptSizeT = optional; using OptString = optional; -using JsonReplaceCb = function; +using JsonReplaceCb = function; using JsonReplaceVerify = std::function; using CI = CommandId; @@ -119,28 +119,28 @@ void PrintOptVec(ConnectionContext* cntx, const OpResult>>& r } error_code JsonReplace(JsonType& instance, string_view path, JsonReplaceCb callback) { - using evaluator_t = jsoncons::jsonpath::detail::jsonpath_evaluator; + using evaluator_t = jsonpath::detail::jsonpath_evaluator; 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 funcs = jsonpath::custom_functions(); error_code ec; - jsoncons::jsonpath::detail::static_resources static_resources(funcs); + jsonpath::detail::static_resources 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 resources; - auto f = [&callback](const json_location_type& path, reference val) { - callback(path.to_string(), val); + jsonpath::detail::dynamic_resources 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> OpArrLen(const OpArgs& op_args, string_view key, OpResult> OpToggle(const OpArgs& op_args, string_view key, string_view path) { vector 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 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(), num); if (isinf(result)) { @@ -607,9 +607,10 @@ OpResult OpDel(const OpArgs& op_args, string_view key, string_view path) { } vector 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> OpObjKeys(const OpArgs& op_args, string_view key, OpResult> OpStrAppend(const OpArgs& op_args, string_view key, string_view path, const vector& strs) { vector 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> OpStrAppend(const OpArgs& op_args, string_view key, s // Clears containers(arrays or objects) and zeroing numbers. OpResult 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 OpClear(const OpArgs& op_args, string_view key, string_view path) OpResult> OpArrPop(const OpArgs& op_args, string_view key, string_view path, int index) { vector 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> OpArrPop(const OpArgs& op_args, string_view key, str OpResult> OpArrTrim(const OpArgs& op_args, string_view key, string_view path, int start_index, int stop_index) { vector 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> 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> 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 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;