Merge 49e747a929 into 75cdc51f86

src/acl.c

@@ -844,6 +844,18 @@ sds ACLDescribeSelector(aclSelector *selector) {
     return res;
 }
 
+int selectorContainsClosingParanthesis(const sds s) {
+    size_t len = sdslen(s);
+    const char *p = s;
+
+    while (len--) {
+        if (*p == ')') return 1;
+        p++;
+    }
+
+    return 0;
+}
+
 /* This is similar to ACLDescribeSelectorCommandRules(), however instead of
  * describing just the user command rules, everything is described: user
  * flags, keys, passwords and finally the command rules obtained via
@@ -885,7 +897,11 @@ robj *ACLDescribeUser(user *u) {
         if (selector->flags & SELECTOR_FLAG_ROOT) {
             res = sdscatfmt(res, "%s", default_perm);
         } else {
-            res = sdscatfmt(res, " (%s)", default_perm);
+            if (selectorContainsClosingParanthesis(default_perm)) {
+                res = sdscatfmt(res, " \"(%s)\"", default_perm);
+            } else {
+                res = sdscatfmt(res, " (%s)", default_perm);
+            }
         }
         sdsfree(default_perm);
     }
@@ -2326,7 +2342,7 @@ sds ACLLoadFromFile(const char *filename) {
         if (lines[i][0] == '\0') continue;
 
         /* Split into arguments */
-        argv = sdssplitlen(lines[i],sdslen(lines[i])," ",1,&argc);
+        argv = sdssplitargs(lines[i],&argc);
         if (argv == NULL) {
             errors = sdscatprintf(errors,
                      "%s:%d: unbalanced quotes in acl line. ",

tests/unit/acl-v2.tcl

@@ -1,4 +1,6 @@
-start_server {tags {"acl external:skip"}} {
+set server_path [tmpdir "acl-v2.acl"]
+exec touch $server_path/user.acl
+start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags [list "acl" "external:skip"]] {
     set r2 [redis_client]
     test {Test basic multiple selectors} {
         r ACL SETUSER selector-1 on -@all resetkeys nopass
@@ -523,6 +525,32 @@ start_server {tags {"acl external:skip"}} {
         assert_equal "ERR wrong number of arguments for 'set' command" $e
     }
 
+    test {Test selectors with closing parenthesis} {
+        r ACL SETUSER selector-store ON NOPASS +@all "(+@all ~bar))"
+        r ACL SETUSER selector-wo-parenthesis ON NOPASS +@all "(+@all ~bar)"
+
+        # Verify selector is wrapped in quote if parenthesis exists.
+        set response [lindex [r ACL LIST] [lsearch [r ACL LIST] "user selector-store*"]]
+        assert_equal "user selector-store on nopass sanitize-payload resetchannels +@all \"(~bar) resetchannels +@all)\"" $response
+        
+        # Verify selector is not wrapped in quote if parenthesis doesn't exists.
+        set response [lindex [r ACL LIST] [lsearch [r ACL LIST] "user selector-wo-parenthesis*"]]
+        assert_equal "user selector-wo-parenthesis on nopass sanitize-payload resetchannels +@all (~bar resetchannels +@all)" $response
+
+        # Verify the key permissions
+        assert_equal "OK" [r ACL DRYRUN selector-store SET bar) world]
+        assert_equal "OK" [r ACL DRYRUN selector-store GET bar)]
+        assert_match {*has no permissions to access the 'bar))' key*} [r ACL DRYRUN selector-store SET bar)) world]
+    }   
+
+    test {Test ACL SAVE/LOAD with selectors containing closing parenthesis} {
+        set users_before_load [r ACL LIST]
+        r ACL SAVE
+        r ACL LOAD
+        set users_after_load [r ACL LIST]
+        assert_equal $users_before_load $users_after_load
+    }
+
     $r2 close
 }