From 986ef7c0e2f6baa06148b769744c880aae2396f9 Mon Sep 17 00:00:00 2001 From: adiholden Date: Mon, 20 Jan 2025 21:07:49 +0200 Subject: [PATCH] fix (stream): XRANGE incorrectly interprets the end parameter (#4443) Signed-off-by: adi_holden --- src/server/stream_family.cc | 15 +++++++++------ src/server/stream_family_test.cc | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/server/stream_family.cc b/src/server/stream_family.cc index e284f3f48..802c4265f 100644 --- a/src/server/stream_family.cc +++ b/src/server/stream_family.cc @@ -257,15 +257,16 @@ bool ParseID(string_view strid, bool strict, uint64_t missing_seq, ParsedStreamI return true; } -bool ParseRangeId(string_view id, RangeId* dest) { +enum class RangeBoundary { kStart, kEnd }; +bool ParseRangeId(string_view id, RangeBoundary type, RangeId* dest) { if (id.empty()) return false; if (id[0] == '(') { dest->exclude = true; id.remove_prefix(1); } - - return ParseID(id, dest->exclude, 0, &dest->parsed_id); + uint64 missing_seq = type == RangeBoundary::kStart ? 0 : -1; + return ParseID(id, dest->exclude, missing_seq, &dest->parsed_id); } /* This is a wrapper function for lpGet() to directly get an integer value @@ -2206,7 +2207,8 @@ void XRangeGeneric(std::string_view key, std::string_view start, std::string_vie CmdArgList args, bool is_rev, Transaction* tx, SinkReplyBuilder* builder) { RangeOpts range_opts; RangeId rs, re; - if (!ParseRangeId(start, &rs) || !ParseRangeId(end, &re)) { + if (!ParseRangeId(start, RangeBoundary::kStart, &rs) || + !ParseRangeId(end, RangeBoundary::kEnd, &re)) { return builder->SendError(kInvalidStreamId, kSyntaxErrType); } @@ -2504,7 +2506,8 @@ bool ParseXpendingOptions(CmdArgList& args, PendingOpts& opts, SinkReplyBuilder* string_view start = ArgS(args, id_indx); id_indx++; string_view end = ArgS(args, id_indx); - if (!ParseRangeId(start, &rs) || !ParseRangeId(end, &re)) { + if (!ParseRangeId(start, RangeBoundary::kStart, &rs) || + !ParseRangeId(end, RangeBoundary::kEnd, &re)) { builder->SendError(kInvalidStreamId, kSyntaxErrType); return false; } @@ -3227,7 +3230,7 @@ void StreamFamily::XAutoClaim(CmdArgList args, const CommandContext& cmd_cntx) { string_view start = ArgS(args, 4); RangeId rs; - if (!ParseRangeId(start, &rs)) { + if (!ParseRangeId(start, RangeBoundary::kStart, &rs)) { return rb->SendError(kSyntaxErr); } diff --git a/src/server/stream_family_test.cc b/src/server/stream_family_test.cc index 0b5ae10cb..12edae158 100644 --- a/src/server/stream_family_test.cc +++ b/src/server/stream_family_test.cc @@ -93,6 +93,21 @@ TEST_F(StreamFamilyTest, AddExtended) { EXPECT_THAT(Run({"xlen", "key4"}), IntArg(601)); } +TEST_F(StreamFamilyTest, XrangeRangeAutocomplete) { + Run({"xadd", "mystream", "1609459200000-0", "0", "0"}); + Run({"xadd", "mystream", "1609459200001-0", "1", "1"}); + Run({"xadd", "mystream", "1609459200001-1", "2", "2"}); + Run({"xadd", "mystream", "1609459200002-0", "3", "3"}); + auto resp = Run({"xrange", "mystream", "1609459200000", "1609459200001"}); + EXPECT_THAT(resp, RespElementsAre(RespElementsAre("1609459200000-0", RespElementsAre("0", "0")), + RespElementsAre("1609459200001-0", RespElementsAre("1", "1")), + RespElementsAre("1609459200001-1", RespElementsAre("2", "2")))); + resp = Run({"xrange", "mystream", "1609459200000", "(1609459200001"}); + EXPECT_THAT(resp, RespElementsAre(RespElementsAre("1609459200000-0", RespElementsAre("0", "0")), + RespElementsAre("1609459200001-0", RespElementsAre("1", "1")), + RespElementsAre("1609459200001-1", RespElementsAre("2", "2")))); +} + TEST_F(StreamFamilyTest, Range) { Run({"xadd", "key", "1-*", "f1", "v1"}); Run({"xadd", "key", "1-*", "f2", "v2"});