From 67117ff081b60f28a49c4c5db383a62953f71f3e Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Tue, 3 Sep 2024 15:00:31 +0300 Subject: [PATCH] fix: crash during SORT DESC call (#3637) fixes #3636 The problem was that instead of implementing GT operator, we implemented !LT, which is GE operator. As a result the iterators inside the sort algorithm reached out of bounds. --------- Signed-off-by: Roman Gershman --- src/server/generic_family.cc | 7 ++++--- src/server/generic_family_test.cc | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/server/generic_family.cc b/src/server/generic_family.cc index de42b278a..2e53c4922 100644 --- a/src/server/generic_family.cc +++ b/src/server/generic_family.cc @@ -1184,9 +1184,10 @@ void GenericFamily::Sort(CmdArgList args, ConnectionContext* cntx) { }); } else { std::sort(entries.begin(), entries.end(), - [reversed, &entries](const auto& lhs, const auto& rhs) { - DCHECK(&rhs < &(*entries.end()) && &rhs >= &(*entries.begin())); - return bool(lhs.Cmp() < rhs.Cmp()) ^ reversed; + [reversed, &entries](const auto& lhs, const auto& rhs) -> bool { + DCHECK((&rhs - entries.data()) >= 0); + DCHECK((&rhs - entries.data()) < int64_t(entries.size())); + return reversed ? rhs.Cmp() < lhs.Cmp() : lhs.Cmp() < rhs.Cmp(); }); } diff --git a/src/server/generic_family_test.cc b/src/server/generic_family_test.cc index 4c2406db7..3aed8bae0 100644 --- a/src/server/generic_family_test.cc +++ b/src/server/generic_family_test.cc @@ -481,6 +481,15 @@ TEST_F(GenericFamilyTest, Sort) { ; } +TEST_F(GenericFamilyTest, SortBug3636) { + Run({"RPUSH", "foo", "1.100000023841858", "1.100000023841858", "1.100000023841858", "-15710", + "1.100000023841858", "1.100000023841858", "1.100000023841858", "-15710", "-15710", + "1.100000023841858", "-15710", "-15710", "-15710", "-15710", "1.100000023841858", "-15710", + "-15710"}); + auto resp = Run({"SORT", "foo", "desc", "alpha"}); + ASSERT_THAT(resp, ArrLen(17)); +} + TEST_F(GenericFamilyTest, TimeNoKeys) { auto resp = Run({"time"}); EXPECT_THAT(resp, ArrLen(2));