diff --git a/src/acl.c b/src/acl.c index caf099c8d..101f225ad 100644 --- a/src/acl.c +++ b/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. ", diff --git a/tests/unit/acl-v2.tcl b/tests/unit/acl-v2.tcl index b233118dd..44da69314 100644 --- a/tests/unit/acl-v2.tcl +++ b/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 }