From 45d3310694406fb0f338f7c639cda9754edb88f8 Mon Sep 17 00:00:00 2001
From: Wen Hui <wen.hui.ware@gmail.com>
Date: Mon, 21 Aug 2023 17:53:46 +0800
Subject: [PATCH] BITCOUNT: check for argument, before checking for key
 (#12394)

Generally, In any command we first check for  the argument and then check if key exist.

Some of the examples are

```
127.0.0.1:6379> getrange no-key invalid1 invalid2
(error) ERR value is not an integer or out of range
127.0.0.1:6379> setbit no-key 1 invalid
(error) ERR bit is not an integer or out of range
127.0.0.1:6379> xrange no-key invalid1 invalid2
(error) ERR Invalid stream ID specified as stream command argument
```

**Before change**
```
bitcount no-key invalid1 invalid2
0
```

**After change**
```
bitcount no-key invalid1 invalid2
(error) ERR value is not an integer or out of range
```
---
 src/bitops.c          | 31 ++++++++++++++++++-------------
 tests/unit/bitops.tcl |  5 +++++
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/bitops.c b/src/bitops.c
index 23d80554e..e2384d814 100644
--- a/src/bitops.c
+++ b/src/bitops.c
@@ -802,25 +802,12 @@ void bitcountCommand(client *c) {
     int isbit = 0;
     unsigned char first_byte_neg_mask = 0, last_byte_neg_mask = 0;
 
-    /* Lookup, check for type, and return 0 for non existing keys. */
-    if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL ||
-        checkType(c,o,OBJ_STRING)) return;
-    p = getObjectReadOnlyString(o,&strlen,llbuf);
-
     /* Parse start/end range if any. */
     if (c->argc == 4 || c->argc == 5) {
-        long long totlen = strlen;
-        /* Make sure we will not overflow */
-        serverAssert(totlen <= LLONG_MAX >> 3);
         if (getLongLongFromObjectOrReply(c,c->argv[2],&start,NULL) != C_OK)
             return;
         if (getLongLongFromObjectOrReply(c,c->argv[3],&end,NULL) != C_OK)
             return;
-        /* Convert negative indexes */
-        if (start < 0 && end < 0 && start > end) {
-            addReply(c,shared.czero);
-            return;
-        }
         if (c->argc == 5) {
             if (!strcasecmp(c->argv[4]->ptr,"bit")) isbit = 1;
             else if (!strcasecmp(c->argv[4]->ptr,"byte")) isbit = 0;
@@ -829,6 +816,20 @@ void bitcountCommand(client *c) {
                 return;
             }
         }
+	/* Lookup, check for type, and return 0 for non existing keys. */
+        if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL ||
+            checkType(c,o,OBJ_STRING)) return;
+        p = getObjectReadOnlyString(o,&strlen,llbuf);
+        long long totlen = strlen;
+
+        /* Make sure we will not overflow */
+        serverAssert(totlen <= LLONG_MAX >> 3);
+
+        /* Convert negative indexes */
+        if (start < 0 && end < 0 && start > end) {
+            addReply(c,shared.czero);
+            return;
+        }
         if (isbit) totlen <<= 3;
         if (start < 0) start = totlen+start;
         if (end < 0) end = totlen+end;
@@ -844,6 +845,10 @@ void bitcountCommand(client *c) {
             end >>= 3;
         }
     } else if (c->argc == 2) {
+        /* Lookup, check for type, and return 0 for non existing keys. */
+        if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL ||
+            checkType(c,o,OBJ_STRING)) return;
+        p = getObjectReadOnlyString(o,&strlen,llbuf);
         /* The whole string. */
         start = 0;
         end = strlen-1;
diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl
index 1b7db407a..d17fe62da 100644
--- a/tests/unit/bitops.tcl
+++ b/tests/unit/bitops.tcl
@@ -140,6 +140,11 @@ start_server {tags {"bitops"}} {
         set e
     } {ERR *syntax*}
 
+    test {BITCOUNT against non-integer value} {
+        catch {r bitcount no-key a b} e
+        set e
+    } {ERR *not an integer*}
+
     test {BITCOUNT regression test for github issue #582} {
         r del foo
         r setbit foo 0 1