Issue #2068, Refactor ACL command

5 files changed, 120 insertions, 62 deletions

diff --git a/library.c b/library.c
index 3772b73d..1560e6b0 100644
--- a/library.c
+++ b/library.c
@@ -2169,6 +2169,27 @@ redis_xinfo_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_t
 }
 
 PHP_REDIS_API int
+redis_acl_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx)
+{
+    if (ctx == NULL) {
+        return redis_read_variant_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR) {
+        return redis_boolean_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR + 1) {
+        return redis_string_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR + 2) {
+        return redis_long_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR + 3) {
+        return redis_acl_getuser_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else if (ctx == PHPREDIS_CTX_PTR + 4) {
+        return redis_acl_log_reply(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, z_tab, NULL);
+    } else {
+        ZEND_ASSERT(!"memory corruption?");
+        return FAILURE;
+    }
+}
+
+PHP_REDIS_API int
 redis_read_acl_log_reply(RedisSock *redis_sock, zval *zret, long count) {
     zval zsub;
     int i, nsub;
diff --git a/library.h b/library.h
index 05b34ed4..c52b0f2f 100644
--- a/library.h
+++ b/library.h
@@ -161,6 +161,7 @@ PHP_REDIS_API int
 redis_read_mpop_response(RedisSock *redis_sock, zval *zdst, int elements, void *ctx);
 
 /* Specialized ACL reply handlers */
+PHP_REDIS_API int redis_acl_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 PHP_REDIS_API int redis_read_acl_getuser_reply(RedisSock *redis_sock, zval *zret, long len);
 PHP_REDIS_API int redis_acl_getuser_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 PHP_REDIS_API int redis_read_acl_log_reply(RedisSock *redis_sock, zval *zret, long count);
diff --git a/redis.c b/redis.c
index 0afb4b48..2aef973d 100644
--- a/redis.c
+++ b/redis.c
@@ -1129,49 +1129,7 @@ PHP_METHOD(Redis, type)
 
 /* {{{ proto mixed Redis::acl(string $op, ...) }}} */
 PHP_METHOD(Redis, acl) {
-    RedisSock *redis_sock;
-    FailableResultCallback cb;
-    zval *zargs;
-    zend_string *op;
-    char *cmd;
-    int cmdlen, argc = ZEND_NUM_ARGS();
-
-    if (argc < 1 || (redis_sock = redis_sock_get(getThis(), 0)) == NULL) {
-        if (argc < 1) {
-            php_error_docref(NULL, E_WARNING, "ACL command requires at least one argument");
-        }
-        RETURN_FALSE;
-    }
-
-    zargs = emalloc(argc * sizeof(*zargs));
-    if (zend_get_parameters_array(ht, argc, zargs) == FAILURE) {
-        efree(zargs);
-        RETURN_FALSE;
-    }
-
-    /* Read the subcommand and set response callback */
-    op = zval_get_string(&zargs[0]);
-    if (zend_string_equals_literal_ci(op, "GETUSER")) {
-        cb = redis_acl_getuser_reply;
-    } else if (zend_string_equals_literal_ci(op, "LOG")) {
-        cb = redis_acl_log_reply;
-    } else {
-        cb = redis_read_variant_reply;
-    }
-
-    /* Make our command and free args */
-    cmd = redis_variadic_str_cmd("ACL", zargs, argc, &cmdlen);
-
-    zend_string_release(op);
-    efree(zargs);
-
-    REDIS_PROCESS_REQUEST(redis_sock, cmd, cmdlen);
-    if (IS_ATOMIC(redis_sock)) {
-        if (cb(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL) < 0) {
-            RETURN_FALSE;
-        }
-    }
-    REDIS_PROCESS_RESPONSE(cb);
+    REDIS_PROCESS_CMD(acl, redis_acl_response);
 }
 
 /* {{{ proto long Redis::append(string key, string val) */
diff --git a/redis_commands.c b/redis_commands.c
index 187a2cdf..5c3dcb90 100644
--- a/redis_commands.c
+++ b/redis_commands.c
@@ -2100,6 +2100,100 @@ redis_pop_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
     return SUCCESS;
 }
 
+int
+redis_acl_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
+              char **cmd, int *cmd_len, short *slot, void **ctx)
+{
+    smart_string cmdstr = {0};
+    zend_string *zstr;
+    zval *z_args;
+    int argc, i;
+
+    if ((argc = ZEND_NUM_ARGS()) < 1) {
+        php_error_docref(NULL, E_WARNING, "ACL command requires at least one argument");
+        return FAILURE;
+    }
+
+    z_args = ecalloc(argc, sizeof(*z_args));
+    if (zend_get_parameters_array(ht, argc, z_args) == FAILURE) {
+        goto failure;
+    }
+
+    zstr = zval_get_string(&z_args[0]);
+    if (zend_string_equals_literal_ci(zstr, "CAT") ||
+        zend_string_equals_literal_ci(zstr, "LIST") ||
+        zend_string_equals_literal_ci(zstr, "USERS")
+    ) {
+        *ctx = NULL;
+    } else if (zend_string_equals_literal_ci(zstr, "LOAD") ||
+        zend_string_equals_literal_ci(zstr, "SAVE")
+    ) {
+        *ctx = PHPREDIS_CTX_PTR;
+    } else if (zend_string_equals_literal_ci(zstr, "GENPASS") ||
+        zend_string_equals_literal_ci(zstr, "WHOAMI")
+    ) {
+        *ctx = PHPREDIS_CTX_PTR + 1;
+    } else if (zend_string_equals_literal_ci(zstr, "SETUSER")) {
+        if (argc < 2) {
+            php_error_docref(NULL, E_WARNING, "ACL SETUSER requires at least one argument");
+            zend_string_release(zstr);
+            goto failure;
+        }
+        *ctx = PHPREDIS_CTX_PTR;
+    } else if (zend_string_equals_literal_ci(zstr, "DELUSER")) {
+        if (argc < 2) {
+            php_error_docref(NULL, E_WARNING, "ACL DELUSER requires at least one argument");
+            zend_string_release(zstr);
+            goto failure;
+        }
+        *ctx = PHPREDIS_CTX_PTR + 2;
+    } else if (zend_string_equals_literal_ci(zstr, "GETUSER")) {
+        if (argc < 2) {
+            php_error_docref(NULL, E_WARNING, "ACL GETUSER requires at least one argument");
+            zend_string_release(zstr);
+            goto failure;
+        }
+        *ctx = PHPREDIS_CTX_PTR + 3;
+    } else if (zend_string_equals_literal_ci(zstr, "DRYRUN")) {
+        if (argc < 3) {
+            php_error_docref(NULL, E_WARNING, "ACL DRYRUN requires at least two arguments");
+            zend_string_release(zstr);
+            goto failure;
+        }
+        *ctx = PHPREDIS_CTX_PTR;
+    } else if (zend_string_equals_literal_ci(zstr, "LOG")) {
+        if (argc > 1 && Z_TYPE(z_args[1]) == IS_STRING && ZVAL_STRICMP_STATIC(&z_args[1], "RESET")) {
+            *ctx = PHPREDIS_CTX_PTR;
+        } else {
+            *ctx = PHPREDIS_CTX_PTR + 4;
+        }
+    } else {
+        php_error_docref(NULL, E_WARNING, "Unknown ACL operation '%s'", ZSTR_VAL(zstr));
+        zend_string_release(zstr);
+        goto failure;
+    }
+
+    REDIS_CMD_INIT_SSTR_STATIC(&cmdstr, argc, "ACL");
+    redis_cmd_append_sstr_zstr(&cmdstr, zstr);
+    zend_string_release(zstr);
+
+    for (i = 1; i < argc; ++i) {
+        zstr = zval_get_string(&z_args[i]);
+        redis_cmd_append_sstr_zstr(&cmdstr, zstr);
+        zend_string_release(zstr);
+    }
+    efree(z_args);
+
+    *cmd = cmdstr.c;
+    *cmd_len = cmdstr.len;
+
+    return SUCCESS;
+
+failure:
+    efree(z_args);
+    return FAILURE;
+}
+
 /* Attempt to pull a long expiry from a zval.  We're more restrictave than zval_get_long
  * because that function will return integers from things like open file descriptors
  * which should simply fail as a TTL */
@@ -3168,23 +3262,6 @@ int redis_pfcount_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
     return SUCCESS;
 }
 
-char *redis_variadic_str_cmd(char *kw, zval *argv, int argc, int *cmd_len) {
-    smart_string cmdstr = {0};
-    zend_string *zstr;
-    int i;
-
-    redis_cmd_init_sstr(&cmdstr, argc, kw, strlen(kw));
-
-    for (i = 0; i < argc; i++) {
-        zstr = zval_get_string(&argv[i]);
-        redis_cmd_append_sstr_zstr(&cmdstr, zstr);
-        zend_string_release(zstr);
-    }
-
-    *cmd_len = cmdstr.len;
-    return cmdstr.c;
-}
-
 int redis_auth_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
                    char **cmd, int *cmd_len, short *slot, void **ctx)
 {
diff --git a/redis_commands.h b/redis_commands.h
index f56662e7..7faef7d5 100644
--- a/redis_commands.h
+++ b/redis_commands.h
@@ -31,8 +31,6 @@ int redis_build_raw_cmd(zval *z_args, int argc, char **cmd, int *cmd_len);
 /* Construct a script command */
 smart_string *redis_build_script_cmd(smart_string *cmd, int argc, zval *z_args);
 
-char *redis_variadic_str_cmd(char *kw, zval *argv, int argc, int *cmd_len);
-
 /* Redis command generics.  Many commands share common prototypes meaning that
  * we can write one function to handle all of them.  For example, there are
  * many COMMAND key value commands, or COMMAND key commands. */
@@ -187,6 +185,9 @@ int redis_geosearchstore_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock
  * specific processing we do (e.g. verifying subarguments) that make them
  * unique */
 
+int redis_acl_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
+    char **cmd, int *cmd_len, short *slot, void **ctx);
+
 int redis_set_cmd(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
     char **cmd, int *cmd_len, short *slot, void **ctx);