Skip to content

Commit e6ef677

Browse files
vyavdoshenkovyavdoshenko
authored
fix: JSON RESP3 compatibility - remove extra nested arrays for most commands (#5747)
Fixed: #5741
1 parent 0fe60ad commit e6ef677

File tree

2 files changed

+166
-1
lines changed

2 files changed

+166
-1
lines changed

‎src/server/json_family.cc‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include <absl/strings/str_join.h>
1010
#include <absl/strings/str_split.h>
1111

12+
#include <type_traits>
13+
1214
#include "absl/cleanup/cleanup.h"
1315
#include "base/flags.h"
1416
#include "base/logging.h"
@@ -312,7 +314,10 @@ template <typename T> void Send(const JsonCallbackResult<T>& result, RedisReplyB
312314
if (rb->IsResp3()) {
313315
rb->StartArray(arr.size());
314316
for (const auto& item : arr) {
315-
rb->StartArray(1);
317+
// For JSON.TYPE (std::string), preserve nested array behavior for compatibility
318+
if constexpr (std::is_same_v<T, std::string>) {
319+
rb->StartArray(1);
320+
}
316321
Send(item, rb);
317322
}
318323
} else {

‎src/server/json_family_test.cc‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3261,4 +3261,164 @@ TEST_F(JsonFamilyTest, JsonIntPathTest) {
32613261
EXPECT_THAT(resp, "[\"large.jpg\"]");
32623262
}
32633263

3264+
TEST_F(JsonFamilyTest, ARRLEN_RESP3NestedArrayBug) {
3265+
Run({"HELLO", "3"});
3266+
3267+
string json = R"({"a":[1], "b":{"a":[1,2,3]}, "c":{"x":"not_a"}})";
3268+
auto resp = Run({"JSON.SET", "doc", ".", json});
3269+
ASSERT_THAT(resp, "OK");
3270+
3271+
// In RESP3 mode, this should return [1, 3] (direct integers)
3272+
// NOT [[1], [3]] (integers wrapped in arrays)
3273+
resp = Run({"JSON.ARRLEN", "doc", "$..a"});
3274+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3275+
EXPECT_EQ(resp.GetVec().size(), 2);
3276+
3277+
// The bug: each element is wrapped in array when it shouldn't be
3278+
// Check that elements are NOT arrays themselves
3279+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY))); // Should be integer, not array
3280+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY))); // Should be integer, not array
3281+
3282+
// Verify the actual values
3283+
EXPECT_THAT(resp.GetVec()[0], IntArg(1));
3284+
EXPECT_THAT(resp.GetVec()[1], IntArg(3));
3285+
}
3286+
3287+
TEST_F(JsonFamilyTest, ARRAPPEND_RESP3NestedArrayBug) {
3288+
Run({"HELLO", "3"});
3289+
3290+
auto resp = Run({"JSON.SET", "doc", ".", R"({"a":[1], "b":{"a":[1,2,3]}})"});
3291+
ASSERT_THAT(resp, "OK");
3292+
3293+
resp = Run({"JSON.ARRAPPEND", "doc", "$..a", "2"});
3294+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3295+
ASSERT_EQ(resp.GetVec().size(), 2);
3296+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3297+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3298+
EXPECT_THAT(resp.GetVec()[0], IntArg(2));
3299+
EXPECT_THAT(resp.GetVec()[1], IntArg(4));
3300+
}
3301+
3302+
TEST_F(JsonFamilyTest, ARRINDEX_RESP3NestedArrayBug) {
3303+
Run({"HELLO", "3"});
3304+
3305+
auto resp = Run({"JSON.SET", "doc", ".", R"({"a":["x","y"], "b":{"a":["y","z"]}})"});
3306+
ASSERT_THAT(resp, "OK");
3307+
3308+
resp = Run({"JSON.ARRINDEX", "doc", "$..a", R"("y")"});
3309+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3310+
ASSERT_EQ(resp.GetVec().size(), 2);
3311+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3312+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3313+
EXPECT_THAT(resp.GetVec()[0], IntArg(1));
3314+
EXPECT_THAT(resp.GetVec()[1], IntArg(0));
3315+
}
3316+
3317+
TEST_F(JsonFamilyTest, ARRPOP_RESP3NestedArrayBug) {
3318+
Run({"HELLO", "3"});
3319+
3320+
auto resp = Run({"JSON.SET", "doc", ".", R"({"a":[7], "b":{"a":[8]}})"});
3321+
ASSERT_THAT(resp, "OK");
3322+
3323+
resp = Run({"JSON.ARRPOP", "doc", "$..a"});
3324+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3325+
ASSERT_EQ(resp.GetVec().size(), 2);
3326+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3327+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3328+
}
3329+
3330+
TEST_F(JsonFamilyTest, ARRTRIM_RESP3NestedArrayBug) {
3331+
Run({"HELLO", "3"});
3332+
3333+
auto resp = Run({"JSON.SET", "doc", ".", R"({"a":[1,2], "b":{"a":[3,4,5]}})"});
3334+
ASSERT_THAT(resp, "OK");
3335+
3336+
resp = Run({"JSON.ARRTRIM", "doc", "$..a", "0", "0"});
3337+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3338+
ASSERT_EQ(resp.GetVec().size(), 2);
3339+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3340+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3341+
EXPECT_THAT(resp.GetVec()[0], IntArg(1));
3342+
EXPECT_THAT(resp.GetVec()[1], IntArg(1));
3343+
}
3344+
3345+
TEST_F(JsonFamilyTest, STRLEN_RESP3NestedArrayBug) {
3346+
Run({"HELLO", "3"});
3347+
3348+
auto resp = Run({"JSON.SET", "doc", ".", R"({"s":"hi", "b":{"s":"abc"}})"});
3349+
ASSERT_THAT(resp, "OK");
3350+
3351+
resp = Run({"JSON.STRLEN", "doc", "$..s"});
3352+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3353+
ASSERT_EQ(resp.GetVec().size(), 2);
3354+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3355+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3356+
EXPECT_THAT(resp.GetVec()[0], IntArg(2));
3357+
EXPECT_THAT(resp.GetVec()[1], IntArg(3));
3358+
}
3359+
3360+
TEST_F(JsonFamilyTest, OBJLEN_RESP3NestedArrayBug) {
3361+
Run({"HELLO", "3"});
3362+
3363+
auto resp = Run({"JSON.SET", "doc", ".", R"({"o":{"k":1}, "b":{"o":{"k":1,"m":2}}})"});
3364+
ASSERT_THAT(resp, "OK");
3365+
3366+
resp = Run({"JSON.OBJLEN", "doc", "$..o"});
3367+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3368+
ASSERT_EQ(resp.GetVec().size(), 2);
3369+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3370+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3371+
EXPECT_THAT(resp.GetVec()[0], IntArg(1));
3372+
EXPECT_THAT(resp.GetVec()[1], IntArg(2));
3373+
}
3374+
3375+
TEST_F(JsonFamilyTest, OBJKEYS_RESP3NestedArrayBug) {
3376+
Run({"HELLO", "3"});
3377+
3378+
auto resp = Run({"JSON.SET", "doc", ".", R"({"o":{"k":1}, "b":{"o":{"k":1,"m":2}}})"});
3379+
ASSERT_THAT(resp, "OK");
3380+
3381+
resp = Run({"JSON.OBJKEYS", "doc", "$..o"});
3382+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3383+
ASSERT_EQ(resp.GetVec().size(), 2);
3384+
// Each element should be array of keys, not array wrapped again
3385+
auto& el0 = resp.GetVec()[0];
3386+
auto& el1 = resp.GetVec()[1];
3387+
ASSERT_THAT(el0, ArgType(RespExpr::ARRAY));
3388+
ASSERT_THAT(el1, ArgType(RespExpr::ARRAY));
3389+
EXPECT_THAT(el0.GetVec(), ElementsAre("k"));
3390+
// Order of keys in objects is not guaranteed, so check size only for the second
3391+
EXPECT_EQ(el1.GetVec().size(), 2);
3392+
}
3393+
3394+
TEST_F(JsonFamilyTest, STRAPPEND_RESP3NestedArrayBug) {
3395+
Run({"HELLO", "3"});
3396+
3397+
auto resp = Run({"JSON.SET", "doc", ".", R"({"s":"a", "b":{"s":"zz"}})"});
3398+
ASSERT_THAT(resp, "OK");
3399+
3400+
resp = Run({"JSON.STRAPPEND", "doc", "$..s", R"("b")"});
3401+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3402+
ASSERT_EQ(resp.GetVec().size(), 2);
3403+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3404+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3405+
EXPECT_THAT(resp.GetVec()[0], IntArg(2));
3406+
EXPECT_THAT(resp.GetVec()[1], IntArg(3));
3407+
}
3408+
3409+
TEST_F(JsonFamilyTest, TOGGLE_RESP3NestedArrayBug) {
3410+
Run({"HELLO", "3"});
3411+
3412+
auto resp = Run({"JSON.SET", "doc", ".", R"({"b":true, "x":{"b":false}})"});
3413+
ASSERT_THAT(resp, "OK");
3414+
3415+
resp = Run({"JSON.TOGGLE", "doc", "$..b"});
3416+
ASSERT_THAT(resp, ArgType(RespExpr::ARRAY));
3417+
ASSERT_EQ(resp.GetVec().size(), 2);
3418+
EXPECT_THAT(resp.GetVec()[0], Not(ArgType(RespExpr::ARRAY)));
3419+
EXPECT_THAT(resp.GetVec()[1], Not(ArgType(RespExpr::ARRAY)));
3420+
EXPECT_THAT(resp.GetVec()[0], IntArg(0));
3421+
EXPECT_THAT(resp.GetVec()[1], IntArg(1));
3422+
}
3423+
32643424
} // namespace dfly

0 commit comments

Comments
 (0)