From 987e6feaa51df8aa2d60b79d20bbce795d7fc6cb Mon Sep 17 00:00:00 2001
From: Borys <borys@dragonflydb.io>
Date: Tue, 24 Sep 2024 16:54:35 +0300
Subject: [PATCH] fix: GETRANGE params validation (#3781)

fix: getrange params validation
---
 src/server/string_family.cc      | 24 ++++++++++++++++--------
 src/server/string_family_test.cc |  4 ++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/server/string_family.cc b/src/server/string_family.cc
index 96d4367f7004..d6a084667f21 100644
--- a/src/server/string_family.cc
+++ b/src/server/string_family.cc
@@ -141,19 +141,27 @@ OpResult<StringValue> OpGetRange(const OpArgs& op_args, string_view key, int32_t
                                  int32_t end) {
   auto read = [start, end](std::string_view slice) mutable -> string_view {
     int32_t strlen = slice.size();
+    if (strlen == 0)
+      return "";
 
-    if (start < 0)
+    if (start < 0) {
+      if (end < start) {
+        return "";
+      }
       start = strlen + start;
-    if (end < 0)
-      end = strlen + end;
+      start = max(start, 0);
+    }
 
-    end = min(end, strlen - 1);
+    if (end < 0) {
+      end = strlen + end;
+      end = max(end, 0);
+    } else {
+      end = min(end, strlen - 1);
+    }
 
-    if (strlen == 0 || start > end)
+    if (start > end) {
       return "";
-
-    start = max(start, 0);
-    end = max(end, 0);
+    }
 
     return slice.substr(start, end - start + 1);
   };
diff --git a/src/server/string_family_test.cc b/src/server/string_family_test.cc
index ad75080b67c3..2e4c5066cdcf 100644
--- a/src/server/string_family_test.cc
+++ b/src/server/string_family_test.cc
@@ -499,12 +499,16 @@ TEST_F(StringFamilyTest, Range) {
 
   Run({"SET", "key4", "1"});
   EXPECT_EQ(Run({"getrange", "key4", "-1", "-2"}), "");
+  EXPECT_EQ(Run({"getrange", "key4", "0", "-2"}), "1");
 
   EXPECT_EQ(CheckedInt({"SETRANGE", "key5", "1", ""}), 0);
   EXPECT_EQ(Run({"GET", "key5"}).type, facade::RespExpr::NIL);
 
   EXPECT_EQ(CheckedInt({"SETRANGE", "num", "6", ""}), 4);
   EXPECT_EQ(Run({"GET", "num"}), "1234");
+
+  // we support only 256MB string so this test is failed now
+  // EXPECT_THAT(CheckedInt({"SETRANGE", "", "268435456", "0"}), 268435457);
 }
 
 TEST_F(StringFamilyTest, IncrByFloat) {