diff --git a/src/core/json/json_object.cc b/src/core/json/json_object.cc index c9ffdcc1b85b..2f14545b5953 100644 --- a/src/core/json/json_object.cc +++ b/src/core/json/json_object.cc @@ -18,8 +18,16 @@ optional JsonFromString(string_view input, PMR_NS::memory_resource* mr return false; }; + // The maximum allowed JSON nesting depth is 256. + const uint32_t json_nesting_depth_limit = 256; + + /* The maximum possible JSON nesting depth is either the specified json_nesting_depth_limit or + half of the input size. Since nesting a JSON object requires at least 2 characters. */ + auto parser_options = jsoncons::json_options{}.max_nesting_depth( + std::min(json_nesting_depth_limit, uint32_t(input.size() / 2))); + json_decoder decoder(std::pmr::polymorphic_allocator{mr}); - json_parser parser(basic_json_decode_options{}, JsonErrorHandler); + json_parser parser(parser_options, JsonErrorHandler); parser.update(input); parser.finish_parse(decoder, ec); diff --git a/src/facade/error.h b/src/facade/error.h index c480adbb642a..abb575372e32 100644 --- a/src/facade/error.h +++ b/src/facade/error.h @@ -36,6 +36,7 @@ extern const char kLoadingErr[]; extern const char kUndeclaredKeyErr[]; extern const char kInvalidDumpValueErr[]; extern const char kInvalidJsonPathErr[]; +extern const char kJsonParseError[]; extern const char kSyntaxErrType[]; extern const char kScriptErrType[]; diff --git a/src/facade/facade.cc b/src/facade/facade.cc index 763cfd3bd378..72e7d1eae90d 100644 --- a/src/facade/facade.cc +++ b/src/facade/facade.cc @@ -97,6 +97,7 @@ const char kLoadingErr[] = "-LOADING Dragonfly is loading the dataset in memory" const char kUndeclaredKeyErr[] = "script tried accessing undeclared key"; const char kInvalidDumpValueErr[] = "DUMP payload version or checksum are wrong"; const char kInvalidJsonPathErr[] = "invalid JSON path"; +const char kJsonParseError[] = "failed to parse JSON"; const char kSyntaxErrType[] = "syntax_error"; const char kScriptErrType[] = "script_error"; diff --git a/src/facade/op_status.cc b/src/facade/op_status.cc index c4c21bfd1732..24f282d6d9f3 100644 --- a/src/facade/op_status.cc +++ b/src/facade/op_status.cc @@ -36,6 +36,8 @@ std::string_view StatusToMsg(OpStatus status) { return kKeyNotFoundErr; case OpStatus::INVALID_JSON_PATH: return kInvalidJsonPathErr; + case OpStatus::INVALID_JSON: + return kJsonParseError; default: LOG(ERROR) << "Unsupported status " << status; return "Internal error"; diff --git a/src/facade/op_status.h b/src/facade/op_status.h index 892ee255a123..953939c3f928 100644 --- a/src/facade/op_status.h +++ b/src/facade/op_status.h @@ -30,7 +30,8 @@ enum class OpStatus : uint16_t { CANCELLED, AT_LEAST_ONE_KEY, MEMBER_NOTFOUND, - INVALID_JSON_PATH + INVALID_JSON_PATH, + INVALID_JSON, }; class OpResultBase { diff --git a/src/server/json_family.cc b/src/server/json_family.cc index c11c549ae68b..8d9a8536f9b2 100644 --- a/src/server/json_family.cc +++ b/src/server/json_family.cc @@ -343,7 +343,7 @@ OpResult SetJson(const OpArgs& op_args, string_view ke std::optional parsed_json = ShardJsonFromString(json_str); if (!parsed_json) { VLOG(1) << "got invalid JSON string '" << json_str << "' cannot be saved"; - return OpStatus::INVALID_VALUE; + return OpStatus::INVALID_JSON; } if (JsonEnconding() == kEncodingJsonFlat) { @@ -955,8 +955,8 @@ OpResult OpDel(const OpArgs& op_args, string_view key, string_view path, if (json_path.HoldsJsonPath()) { const json::Path& path = json_path.AsJsonPath(); - long deletions = - json::MutatePath(path, [](optional, JsonType* val) { return true; }, json_val); + long deletions = json::MutatePath( + path, [](optional, JsonType* val) { return true; }, json_val); return deletions; } @@ -1345,7 +1345,7 @@ OpResult OpSet(const OpArgs& op_args, string_view key, string_view path, optional parsed_json = ShardJsonFromString(json_str); if (!parsed_json) { VLOG(1) << "got invalid JSON string '" << json_str << "' cannot be saved"; - return OpStatus::INVALID_VALUE; + return OpStatus::INVALID_JSON; } const JsonType& new_json = parsed_json.value(); @@ -1444,7 +1444,7 @@ OpStatus OpMerge(const OpArgs& op_args, string_view key, string_view path, std::optional parsed_json = ShardJsonFromString(json_str); if (!parsed_json) { VLOG(1) << "got invalid JSON string '" << json_str << "' cannot be saved"; - return OpStatus::SYNTAX_ERR; + return OpStatus::INVALID_JSON; } auto cb = [&](std::optional cur_path, JsonType* val) -> MutateCallbackResult<> { diff --git a/src/server/json_family_test.cc b/src/server/json_family_test.cc index 729fb6f91649..d04fe7fd994e 100644 --- a/src/server/json_family_test.cc +++ b/src/server/json_family_test.cc @@ -3017,4 +3017,33 @@ TEST_F(JsonFamilyTest, GetString) { EXPECT_THAT(resp, ErrArg("WRONGTYPE")); } +TEST_F(JsonFamilyTest, MaxNestingJsonDepth) { + auto generate_nested_json = [](int depth) -> std::string { + std::string json = "{"; + for (int i = 0; i < depth - 1; ++i) { + json += R"("key": {)"; + } + json += R"("key": "value")"; // Innermost value + for (int i = 0; i < depth - 1; ++i) { + json += "}"; + } + json += "}"; + return json; + }; + + // Generate JSON with maximum allowed depth (256) + /* std::string valid_json = generate_nested_json(255); + + // Test with valid JSON at depth 256 + auto resp = Run({"JSON.SET", "valid_json", ".", valid_json}); + EXPECT_THAT(resp, "OK"); */ + + // Generate JSON exceeding maximum depth (257) + std::string invalid_json = generate_nested_json(257); + + // Test with invalid JSON at depth 257 + auto resp = Run({"JSON.SET", "invalid_json", ".", invalid_json}); + EXPECT_THAT(resp, ErrArg("failed to parse JSON")); +} + } // namespace dfly