From 6ceadfb5805369f896f0f137c30a4f7210198ff3 Mon Sep 17 00:00:00 2001
From: Zihao Lin <104664078@163.com>
Date: Tue, 20 Aug 2024 12:34:43 +0800
Subject: [PATCH] Improve GETRANGE command behavior (#12272)

Fixed the issue about GETRANGE and SUBSTR command
return unexpected result caused by the `start` and `end` out of
definition range of string.

---
## break change
Before this PR, when negative `end` was out of range (i.e., end <
-strlen), we would fix it to 0 to get the substring, which also resulted
in the first character still being returned for this kind of out of
range.
After this PR, we ensure that `GETRANGE` returns an empty bulk when the
negative end index is out of range.

Closes #11738

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
---
 src/t_string.c             | 19 +++++--------------
 tests/unit/type/string.tcl | 12 ++++++++++++
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/src/t_string.c b/src/t_string.c
index ce095ca65b5..067617a92d0 100644
--- a/src/t_string.c
+++ b/src/t_string.c
@@ -499,24 +499,15 @@ void getrangeCommand(client *c) {
         strlen = sdslen(str);
     }
 
-    /* Convert negative indexes */
-    if (start < 0 && end < 0 && start > end) {
+    if (start < 0) start += strlen;
+    if (end < 0) end += strlen;
+    if (strlen == 0 || start >= (long long)strlen || end < 0 || start > end) {
         addReply(c,shared.emptybulk);
         return;
     }
-    if (start < 0) start = strlen+start;
-    if (end < 0) end = strlen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
-    if ((unsigned long long)end >= strlen) end = strlen-1;
-
-    /* Precondition: end >= 0 && end < strlen, so the only condition where
-     * nothing can be returned is: start > end. */
-    if (start > end || strlen == 0) {
-        addReply(c,shared.emptybulk);
-    } else {
-        addReplyBulkCBuffer(c,(char*)str+start,end-start+1);
-    }
+    if (end >= (long long)strlen) end = strlen-1;
+    addReplyBulkCBuffer(c,(char*)str+start,end-start+1);
 }
 
 void mgetCommand(client *c) {
diff --git a/tests/unit/type/string.tcl b/tests/unit/type/string.tcl
index 94702ec3dc3..2b69692c478 100644
--- a/tests/unit/type/string.tcl
+++ b/tests/unit/type/string.tcl
@@ -464,6 +464,12 @@ start_server {tags {"string"}} {
         assert_equal "" [r getrange mykey 5 3]
         assert_equal " World" [r getrange mykey 5 5000]
         assert_equal "Hello World" [r getrange mykey -5000 10000]
+        assert_equal "" [r getrange mykey 0 -100]
+        assert_equal "" [r getrange mykey 1 -100]
+        assert_equal "" [r getrange mykey -1 -100]
+        assert_equal "" [r getrange mykey -100 -99]
+        assert_equal "" [r getrange mykey -100 -100]
+        assert_equal "" [r getrange mykey -100 -101]
     }
 
     test "GETRANGE against integer-encoded value" {
@@ -474,6 +480,12 @@ start_server {tags {"string"}} {
         assert_equal "" [r getrange mykey 5 3]
         assert_equal "4" [r getrange mykey 3 5000]
         assert_equal "1234" [r getrange mykey -5000 10000]
+        assert_equal "" [r getrange mykey 0 -100]
+        assert_equal "" [r getrange mykey 1 -100]
+        assert_equal "" [r getrange mykey -1 -100]
+        assert_equal "" [r getrange mykey -100 -99]
+        assert_equal "" [r getrange mykey -100 -100]
+        assert_equal "" [r getrange mykey -100 -101]
     }
 
     test "GETRANGE fuzzing" {