From 504810a5d1599c4e5f8c749934de272a625a687f Mon Sep 17 00:00:00 2001 From: Pavlo Yatsukhnenko Date: Tue, 1 Nov 2022 12:53:23 +0200 Subject: Issue #2068, Refactor ACL command --- library.c | 21 +++++++++++ library.h | 1 + redis.c | 44 +--------------------- redis_commands.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++--------- redis_commands.h | 5 ++- 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 @@ -2168,6 +2168,27 @@ redis_xinfo_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_t return FAILURE; } +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; 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); -- cgit v1.2.3 From 376d4d27b01a05c1a0634dfad7b3d2e6429a0a7e Mon Sep 17 00:00:00 2001 From: Pavlo Yatsukhnenko Date: Tue, 1 Nov 2022 13:11:54 +0200 Subject: Use fast_zpp API --- redis_commands.c | 84 +++++++++++++++++++++++--------------------------------- 1 file changed, 34 insertions(+), 50 deletions(-) diff --git a/redis_commands.c b/redis_commands.c index 5c3dcb90..47faaebd 100644 --- a/redis_commands.c +++ b/redis_commands.c @@ -2105,93 +2105,77 @@ 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; + zend_string *op, *zstr; + zval *z_args = NULL; + int argc = 0, 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; - } + ZEND_PARSE_PARAMETERS_START(1, -1) + Z_PARAM_STR(op) + Z_PARAM_OPTIONAL + Z_PARAM_VARIADIC('*', z_args, argc) + ZEND_PARSE_PARAMETERS_END_EX(return 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") + if (zend_string_equals_literal_ci(op, "CAT") || + zend_string_equals_literal_ci(op, "LIST") || + zend_string_equals_literal_ci(op, "USERS") ) { *ctx = NULL; - } else if (zend_string_equals_literal_ci(zstr, "LOAD") || - zend_string_equals_literal_ci(zstr, "SAVE") + } else if (zend_string_equals_literal_ci(op, "LOAD") || + zend_string_equals_literal_ci(op, "SAVE") ) { *ctx = PHPREDIS_CTX_PTR; - } else if (zend_string_equals_literal_ci(zstr, "GENPASS") || - zend_string_equals_literal_ci(zstr, "WHOAMI") + } else if (zend_string_equals_literal_ci(op, "GENPASS") || + zend_string_equals_literal_ci(op, "WHOAMI") ) { *ctx = PHPREDIS_CTX_PTR + 1; - } else if (zend_string_equals_literal_ci(zstr, "SETUSER")) { - if (argc < 2) { + } else if (zend_string_equals_literal_ci(op, "SETUSER")) { + if (argc < 1) { php_error_docref(NULL, E_WARNING, "ACL SETUSER requires at least one argument"); - zend_string_release(zstr); - goto failure; + return FAILURE; } *ctx = PHPREDIS_CTX_PTR; - } else if (zend_string_equals_literal_ci(zstr, "DELUSER")) { - if (argc < 2) { + } else if (zend_string_equals_literal_ci(op, "DELUSER")) { + if (argc < 1) { php_error_docref(NULL, E_WARNING, "ACL DELUSER requires at least one argument"); - zend_string_release(zstr); - goto failure; + return FAILURE; } *ctx = PHPREDIS_CTX_PTR + 2; - } else if (zend_string_equals_literal_ci(zstr, "GETUSER")) { - if (argc < 2) { + } else if (zend_string_equals_literal_ci(op, "GETUSER")) { + if (argc < 1) { php_error_docref(NULL, E_WARNING, "ACL GETUSER requires at least one argument"); - zend_string_release(zstr); - goto failure; + return FAILURE; } *ctx = PHPREDIS_CTX_PTR + 3; - } else if (zend_string_equals_literal_ci(zstr, "DRYRUN")) { - if (argc < 3) { + } else if (zend_string_equals_literal_ci(op, "DRYRUN")) { + if (argc < 2) { php_error_docref(NULL, E_WARNING, "ACL DRYRUN requires at least two arguments"); - zend_string_release(zstr); - goto failure; + return 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")) { + } else if (zend_string_equals_literal_ci(op, "LOG")) { + if (argc > 0 && Z_TYPE(z_args[0]) == IS_STRING && ZVAL_STRICMP_STATIC(&z_args[0], "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; + php_error_docref(NULL, E_WARNING, "Unknown ACL operation '%s'", ZSTR_VAL(op)); + return FAILURE; } - REDIS_CMD_INIT_SSTR_STATIC(&cmdstr, argc, "ACL"); - redis_cmd_append_sstr_zstr(&cmdstr, zstr); - zend_string_release(zstr); + REDIS_CMD_INIT_SSTR_STATIC(&cmdstr, 1 + argc, "ACL"); + redis_cmd_append_sstr_zstr(&cmdstr, op); - for (i = 1; i < argc; ++i) { + for (i = 0; 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 -- cgit v1.2.3