Skip to content

Commit

Permalink
fix(json_family): Fix json get crash due to an invalid json path (#3580)
Browse files Browse the repository at this point in the history
fixes dragonflydb##3558

Signed-off-by: Stepan Bagritsevich <[email protected]>
  • Loading branch information
BagritsevichStepan authored Aug 27, 2024
1 parent 80e1dbb commit f4fd0f1
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/facade/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ extern const char kClusterNotConfigured[];
extern const char kLoadingErr[];
extern const char kUndeclaredKeyErr[];
extern const char kInvalidDumpValueErr[];
extern const char kInvalidJsonPathErr[];

extern const char kSyntaxErrType[];
extern const char kScriptErrType[];
Expand Down
1 change: 1 addition & 0 deletions src/facade/facade.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const char kClusterNotConfigured[] = "Cluster is not yet configured";
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 kSyntaxErrType[] = "syntax_error";
const char kScriptErrType[] = "script_error";
Expand Down
2 changes: 2 additions & 0 deletions src/facade/op_status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ std::string_view StatusToMsg(OpStatus status) {
return "at least 1 input key is needed for this command";
case OpStatus::MEMBER_NOTFOUND:
return kKeyNotFoundErr;
case OpStatus::INVALID_JSON_PATH:
return kInvalidJsonPathErr;
default:
LOG(ERROR) << "Unsupported status " << status;
return "Internal error";
Expand Down
3 changes: 2 additions & 1 deletion src/facade/op_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ enum class OpStatus : uint16_t {
INVALID_NUMERIC_RESULT,
CANCELLED,
AT_LEAST_ONE_KEY,
MEMBER_NOTFOUND
MEMBER_NOTFOUND,
INVALID_JSON_PATH
};

class OpResultBase {
Expand Down
4 changes: 4 additions & 0 deletions src/server/json_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,14 @@ OpResult<std::string> OpJsonGet(const OpArgs& op_args, string_view key,
jsoncons::json_object_arg}; // see https://github.com/danielaparker/jsoncons/issues/482
if (paths.size() == 1) {
auto eval_result = eval_wrapped(paths[0].second);
if (!eval_result) {
return OpStatus::INVALID_JSON_PATH;
}
out = std::move(eval_result).value(); // TODO(Print not existing path to the user)
} else {
for (const auto& [path_str, path] : paths) {
auto eval_result = eval_wrapped(path);
DCHECK(eval_result);
out[path_str] = std::move(eval_result).value(); // TODO(Print not existing path to the user)
}
}
Expand Down
19 changes: 19 additions & 0 deletions src/server/json_family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,25 @@ TEST_F(JsonFamilyTest, GetLegacy) {

resp = Run({"JSON.GET", "json", "$.name", "$.lastSeen"}); // V2 Response
ASSERT_THAT(resp, "{\"$.lastSeen\":[1478476800],\"$.name\":[\"Leonard Cohen\"]}");

json = R"(
{"a":"first","b":{"field":"second"},"c":{"field":"third"}}
)";

resp = Run({"JSON.SET", "json", "$", json});
ASSERT_THAT(resp, "OK");

resp = Run({"JSON.GET", "json", "bar"}); // V1 Response
ASSERT_THAT(resp, ErrArg("ERR invalid JSON path"));

resp = Run({"JSON.GET", "json", "$.bar"}); // V2 Response
ASSERT_THAT(resp, "[]");

resp = Run({"JSON.GET", "json", "bar", "$.a"}); // V2 Response
ASSERT_THAT(resp, R"({"$.a":["first"],"bar":[]})");

resp = Run({"JSON.GET", "json", "$.bar"}); // V2 Response
ASSERT_THAT(resp, "[]");
}

static const string PhonebookJson = R"(
Expand Down

0 comments on commit f4fd0f1

Please sign in to comment.