Add check to find duplicate option

Signed-off-by: Shivshankar-Reddy <[email protected]>
Shivshankar-Reddy · Jun 18, 2024 · a30d4ce · a30d4ce
1 parent be2c321
commit a30d4ce
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 8 deletions.
diff --git a/src/server.c b/src/server.c
@@ -3710,6 +3710,57 @@ int commandCheckArity(client *c, sds *err) {
     return 1;
 }
 
+int findDuplicatesOptions(client *c, const char *token) {
+    int matched = 0;
+    for (int j = 2, k = c->argc - 1; j <= k; j++, k--) {
+        if (!strcasecmp(token, c->argv[j]->ptr)) {
+            matched++;
+        }
+        if (j != k && !strcasecmp(token, c->argv[k]->ptr)) {
+            matched++;
+        }
+        if (matched > 1) {
+            return 0;
+        }
+    }
+    return 1;
+}
+
+/* Check if c->cmd has duplicate options, fills `err` with details in case it has.
+ * Return 1 if no duplicate options are found. */
+int commandCheckArgTokenDuplicacy(client *c, sds *err) {
+    if (c->argc <= 4) return 1;
+    for (int iter = 0; iter < c->cmd->num_args; iter++) {
+        if (c->cmd->args[iter].token && !(c->cmd->args[iter].flags & CMD_ARG_MULTIPLE_TOKEN)) {
+            if(!findDuplicatesOptions(c, c->cmd->args[iter].token)) {
+                if (err) {
+                    *err = sdsnew(NULL);
+                    *err = sdscatprintf(*err, "duplicate '%s' option for '%s' command", c->cmd->args[iter].token, c->cmd->fullname);
+                }
+                return 0;
+            }
+        }
+        if (c->cmd->args[iter].subargs) {
+            for (int subiter = 0; subiter < c->cmd->args[iter].num_args; subiter++) {
+                if (c->cmd->args[iter].subargs[subiter].token && !(c->cmd->args[iter].subargs[subiter].flags & CMD_ARG_MULTIPLE_TOKEN)) {
+                    if(!findDuplicatesOptions(c, c->cmd->args[iter].subargs[subiter].token)) {
+                        if (err) {
+                            *err = sdsnew(NULL);
+                            *err = sdscatprintf(*err, "duplicate '%s' option for '%s' command", c->cmd->args[iter].subargs[subiter].token, c->cmd->fullname);
+                        }
+                        return 0;
+                    }
+                } else {
+                    continue;
+                }
+            }
+        } else {
+            continue;
+        }
+    }
+    return 1;
+}
+
 /* If we're executing a script, try to extract a set of command flags from
  * it, in case it declared them. Note this is just an attempt, we don't yet
  * know the script command is well formed.*/
@@ -3783,6 +3834,10 @@ int processCommand(client *c) {
             rejectCommandSds(c, err);
             return C_OK;
         }
+        if (c->cmd->arity != c->argc && c->cmd->args && !commandCheckArgTokenDuplicacy(c, &err)) {
+            rejectCommandSds(c, err);
+            return C_OK;
+        }
 
 
         /* Check if the command is marked as protected and the relevant configuration allows it */

diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl
@@ -846,17 +846,17 @@ start_server {tags {"acl external:skip"}} {
     test {When an authentication chain is used in the HELLO cmd, the last auth cmd has precedence} {
         r ACL setuser secure-user1 >supass on +@all
         r ACL setuser secure-user2 >supass on +@all
-        r HELLO 2 AUTH secure-user pass AUTH secure-user2 supass AUTH secure-user1 supass
+        r HELLO 2 AUTH secure-user1 supass
         assert {[r ACL whoami] eq {secure-user1}}
-        catch {r HELLO 2 AUTH secure-user supass AUTH secure-user2 supass AUTH secure-user pass} e
+        catch {r HELLO 2 AUTH secure-user pass} e
         assert_match "WRONGPASS invalid username-password pair or user is disabled." $e
         assert {[r ACL whoami] eq {secure-user1}}
     }
 
     test {When a setname chain is used in the HELLO cmd, the last setname cmd has precedence} {
-        r HELLO 2 setname client1 setname client2 setname client3 setname client4
+        r HELLO 2 setname client4
         assert {[r client getname] eq {client4}}
-        catch {r HELLO 2 setname client5 setname client6 setname "client name"} e
+        catch {r HELLO 2 setname "client name"} e
         assert_match "ERR Client names cannot contain spaces, newlines or special characters." $e
         assert {[r client getname] eq {client4}}
     }

diff --git a/tests/unit/functions.tcl b/tests/unit/functions.tcl
@@ -956,7 +956,7 @@ start_server {tags {"scripting"}} {
     test {FUNCTION - test function list libraryname multiple times} {
         catch {r function list withcode libraryname foo libraryname foo} e
         set _ $e
-    } {*Unknown argument libraryname*}
+    } {*ERR duplicate*}
 
     test {FUNCTION - verify OOM on function load and function restore} {
         r function flush

diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl
@@ -1782,7 +1782,7 @@ foreach {type large} [array get largevalue] {
         assert_error "ERR syntax error*" {r lmpop 1 mylist{t} LEFT bar_arg}
         assert_error "ERR syntax error*" {r lmpop 1 mylist{t} RIGHT LEFT}
         assert_error "ERR syntax error*" {r lmpop 1 mylist{t} COUNT}
-        assert_error "ERR syntax error*" {r lmpop 1 mylist{t} LEFT COUNT 1 COUNT 2}
+        assert_error "ERR duplicate*" {r lmpop 1 mylist{t} LEFT COUNT 1 COUNT 2}
         assert_error "ERR syntax error*" {r lmpop 2 mylist{t} mylist2{t} bad_arg}
 
         assert_error "ERR count*" {r lmpop 1 mylist{t} LEFT COUNT 0}

diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl
@@ -1280,7 +1280,7 @@ start_server {tags {"zset"}} {
         assert_error "ERR syntax error*" {r zmpop 1 myzset{t} MIN bar_arg}
         assert_error "ERR syntax error*" {r zmpop 1 myzset{t} MAX MIN}
         assert_error "ERR syntax error*" {r zmpop 1 myzset{t} COUNT}
-        assert_error "ERR syntax error*" {r zmpop 1 myzset{t} MAX COUNT 1 COUNT 2}
+        assert_error "ERR duplicate*" {r zmpop 1 myzset{t} MAX COUNT 1 COUNT 2}
         assert_error "ERR syntax error*" {r zmpop 2 myzset{t} myzset2{t} bad_arg}
 
         assert_error "ERR count*" {r zmpop 1 myzset{t} MIN COUNT 0}
@@ -2122,7 +2122,7 @@ start_server {tags {"zset"}} {
         assert_error "ERR syntax error*" {r bzmpop 1 1 myzset{t} MIN bar_arg}
         assert_error "ERR syntax error*" {r bzmpop 1 1 myzset{t} MAX MIN}
         assert_error "ERR syntax error*" {r bzmpop 1 1 myzset{t} COUNT}
-        assert_error "ERR syntax error*" {r bzmpop 1 1 myzset{t} MIN COUNT 1 COUNT 2}
+        assert_error "ERR duplicate*" {r bzmpop 1 1 myzset{t} MIN COUNT 1 COUNT 2}
         assert_error "ERR syntax error*" {r bzmpop 1 2 myzset{t} myzset2{t} bad_arg}
 
         assert_error "ERR count*" {r bzmpop 1 1 myzset{t} MIN COUNT 0}