Update RedisCluster scan logic for large SCAN cursors.

We also need to update the `RedisCluster` logic to handle very large
curosr values, in addition to handling them for the `Redis` and
`RedisArray` classes.

See #2454, #2458

cluster_library.c

@@ -2279,8 +2279,9 @@ PHP_REDIS_API void cluster_gen_mbulk_resp(INTERNAL_FUNCTION_PARAMETERS,
 }
 
 /* HSCAN, SSCAN, ZSCAN */
-PHP_REDIS_API int cluster_scan_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
-                              REDIS_SCAN_TYPE type, long *it)
+PHP_REDIS_API int
+cluster_scan_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c,
+                  REDIS_SCAN_TYPE type, uint64_t *cursor)
 {
     char *pit;
 
@@ -2304,12 +2305,11 @@ PHP_REDIS_API int cluster_scan_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *
     }
 
     // Push the new iterator value to our caller
-    *it = atol(pit);
+    *cursor = strtoull(pit, NULL, 10);
     efree(pit);
 
     // We'll need another MULTIBULK response for the payload
-    if (cluster_check_response(c, &c->reply_type) < 0)
-    {
+    if (cluster_check_response(c, &c->reply_type) < 0) {
         return FAILURE;
     }
 

cluster_library.h

@@ -488,7 +488,7 @@ PHP_REDIS_API void cluster_msetnx_resp(INTERNAL_FUNCTION_PARAMETERS,
 
 /* Response handler for ZSCAN, SSCAN, and HSCAN */
 PHP_REDIS_API int cluster_scan_resp(INTERNAL_FUNCTION_PARAMETERS,
-    redisCluster *c, REDIS_SCAN_TYPE type, long *it);
+    redisCluster *c, REDIS_SCAN_TYPE type, uint64_t *cursor);
 
 /* INFO response handler */
 PHP_REDIS_API void cluster_info_resp(INTERNAL_FUNCTION_PARAMETERS,

library.c

@@ -4512,4 +4512,45 @@ void redis_conf_auth(HashTable *ht, const char *key, size_t keylen,
     redis_extract_auth_info(zv, user, pass);
 }
 
+/* Update a zval with the current 64 bit scan cursor.  This presents a problem
+ * because we can only represent up to 63 bits in a PHP integer.  So depending
+ * on the cursor value, we may need to represent it as a string. */
+void redisSetScanCursor(zval *zv, uint64_t cursor) {
+    char tmp[21];
+    size_t len;
+
+    ZEND_ASSERT(zv != NULL && (Z_TYPE_P(zv) == IS_LONG ||
+                               Z_TYPE_P(zv) == IS_STRING));
+
+    if (Z_TYPE_P(zv) == IS_STRING)
+        zend_string_release(Z_STR_P(zv));
+
+    if (cursor > ZEND_LONG_MAX) {
+        len = snprintf(tmp, sizeof(tmp), "%llu", (unsigned long long)cursor);
+        ZVAL_STRINGL(zv, tmp, len);
+    } else {
+        ZVAL_LONG(zv, cursor);
+    }
+}
+
+/* Get a Redis SCAN cursor value out of a zval.  These are always taken as a
+ * reference argument that that must be `null`, `int`, or `string`.  */
+uint64_t redisGetScanCursor(zval *zv, zend_bool *was_zero) {
+    ZEND_ASSERT(zv != NULL && (Z_TYPE_P(zv) == IS_LONG ||
+                               Z_TYPE_P(zv) == IS_STRING ||
+                               Z_TYPE_P(zv) == IS_NULL));
+
+    if (Z_TYPE_P(zv) == IS_NULL) {
+        convert_to_long(zv);
+        *was_zero = 0;
+        return 0;
+    } else if (Z_TYPE_P(zv) == IS_STRING) {
+        *was_zero = Z_STRLEN_P(zv) == 1 && Z_STRVAL_P(zv)[0] == '0';
+        return strtoull(Z_STRVAL_P(zv), NULL, 10);
+    } else {
+        *was_zero = Z_LVAL_P(zv) == 0;
+        return Z_LVAL_P(zv);
+    }
+}
+
 /* vim: set tabstop=4 softtabstop=4 expandtab shiftwidth=4: */

library.h

@@ -100,8 +100,10 @@ PHP_REDIS_API int redis_mbulk_reply_zipped_keys_dbl(INTERNAL_FUNCTION_PARAMETERS
 PHP_REDIS_API int redis_mbulk_reply_assoc(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 PHP_REDIS_API int redis_mbulk_reply_double(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
 
-PHP_REDIS_API int redis_sock_read_scan_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, REDIS_SCAN_TYPE type, uint64_t *cursor);
+void redisSetScanCursor(zval *zv, uint64_t cursor);
+uint64_t redisGetScanCursor(zval *zv, zend_bool *was_zero);
 
+PHP_REDIS_API int redis_sock_read_scan_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, REDIS_SCAN_TYPE type, uint64_t *cursor);
 
 PHP_REDIS_API int redis_xrange_reply(INTERNAL_FUNCTION_PARAMETERS,
     RedisSock *redis_sock, zval *z_tab, void *ctx);

redis.c

@@ -2744,40 +2744,6 @@ redis_build_scan_cmd(char **cmd, REDIS_SCAN_TYPE type, char *key, int key_len,
     return cmdstr.len;
 }
 
-/* Update a zval with the current 64 bit scan cursor.  This presents a problem
- * because we can only represent up to 63 bits in a PHP integer.  So depending
- * on the cursor value, we may need to represent it as a string. */
-static void updateScanCursorZVal(zval *zv, uint64_t cursor) {
-    char tmp[21];
-    size_t len;
-
-    ZEND_ASSERT(zv != NULL && (Z_TYPE_P(zv) == IS_LONG ||
-                               Z_TYPE_P(zv) == IS_STRING));
-
-    if (Z_TYPE_P(zv) == IS_STRING)
-        zend_string_release(Z_STR_P(zv));
-
-    if (cursor > ZEND_LONG_MAX) {
-        len = snprintf(tmp, sizeof(tmp), "%llu", (unsigned long long)cursor);
-        ZVAL_STRINGL(zv, tmp, len);
-    } else {
-        ZVAL_LONG(zv, cursor);
-    }
-}
-
-static uint64_t getScanCursorZVal(zval *zv, zend_bool *was_zero) {
-    ZEND_ASSERT(zv != NULL && (Z_TYPE_P(zv) == IS_LONG ||
-                               Z_TYPE_P(zv) == IS_STRING));;
-
-    if (Z_TYPE_P(zv) == IS_STRING) {
-        *was_zero = Z_STRLEN_P(zv) == 1 && Z_STRVAL_P(zv)[0] == '0';
-        return strtoull(Z_STRVAL_P(zv), NULL, 10);
-    } else {
-        *was_zero = Z_LVAL_P(zv) == 0;
-        return Z_LVAL_P(zv);
-    }
-}
-
 /* {{{ proto redis::scan(&$iterator, [pattern, [count, [type]]]) */
 PHP_REDIS_API void
 generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) {
@@ -2825,18 +2791,10 @@ generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) {
         RETURN_FALSE;
     }
 
-    /* If our cursor is NULL (it can only be null|int|string), convert it to a
-     * long and initialize it to zero for oure initial SCAN.  Otherwise et the
-     * uint64_t value from the zval which can either be in the form of a long or
-     * a string (if the cursor is too large to fit in a zend_long). */
-    if (Z_TYPE_P(z_cursor) == IS_NULL) {
-        convert_to_long(z_cursor);
-        cursor = 0;
-    } else {
-        cursor = getScanCursorZVal(z_cursor, &completed);
-        if (completed)
-            RETURN_FALSE;
-    }
+    /* Get our SCAN cursor short circuiting if we're done */
+    cursor = redisGetScanCursor(z_cursor, &completed);
+    if (completed)
+        RETURN_FALSE;
 
     /* Prefix our key if we've got one and we have a prefix set */
     if(key_len) {
@@ -2889,7 +2847,7 @@ generic_scan_cmd(INTERNAL_FUNCTION_PARAMETERS, REDIS_SCAN_TYPE type) {
     if(key_free) efree(key);
 
     /* Update our iterator reference */
-    updateScanCursorZVal(z_cursor, cursor);
+    redisSetScanCursor(z_cursor, cursor);
 }
 
 PHP_METHOD(Redis, scan) {

redis_cluster.c

@@ -2266,8 +2266,10 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
     short slot;
     zval *z_it;
     HashTable *hash;
-    long it, num_ele;
+    long num_ele;
     zend_long count = 0;
+    zend_bool complted;
+    uint64_t cursor;
 
     // Can't be in MULTI mode
     if (!CLUSTER_IS_ATOMIC(c)) {
@@ -2285,16 +2287,10 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
     /* Treat as readonly */
     c->readonly = 1;
 
-    // Convert iterator to long if it isn't, update our long iterator if it's
-    // set and >0, and finish if it's back to zero
-    if (Z_TYPE_P(z_it) != IS_LONG || Z_LVAL_P(z_it) < 0) {
-        convert_to_long(z_it);
-        it = 0;
-    } else if (Z_LVAL_P(z_it) != 0) {
-        it = Z_LVAL_P(z_it);
-    } else {
+    /* Get our scan cursor and return early if we're done */
+    cursor = redisGetScanCursor(z_it, &complted);
+    if (complted)
         RETURN_FALSE;
-    }
 
     // Apply any key prefix we have, get the slot
     key_free = redis_key_prefix(c->flags, &key, &key_len);
@@ -2314,7 +2310,7 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
         }
 
         // Create command
-        cmd_len = redis_fmt_scan_cmd(&cmd, type, key, key_len, it, pat, pat_len,
+        cmd_len = redis_fmt_scan_cmd(&cmd, type, key, key_len, cursor, pat, pat_len,
             count);
 
         // Send it off
@@ -2328,7 +2324,7 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
 
         // Read response
         if (cluster_scan_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, type,
-                              &it) == FAILURE)
+                              &cursor) == FAILURE)
         {
             CLUSTER_THROW_EXCEPTION("Couldn't read SCAN response", 0);
             if (key_free) efree(key);
@@ -2342,7 +2338,7 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
 
         // Free our command
         efree(cmd);
-    } while (c->flags->scan & REDIS_SCAN_RETRY && it != 0 && num_ele == 0);
+    } while (c->flags->scan & REDIS_SCAN_RETRY && cursor != 0 && num_ele == 0);
 
     // Free our pattern
     if (pat_free) efree(pat);
@@ -2351,7 +2347,7 @@ static void cluster_kscan_cmd(INTERNAL_FUNCTION_PARAMETERS,
     if (key_free) efree(key);
 
     // Update iterator reference
-    Z_LVAL_P(z_it) = it;
+    redisSetScanCursor(z_it, cursor);
 }
 
 static int redis_acl_op_readonly(zend_string *op) {
@@ -2445,9 +2441,11 @@ PHP_METHOD(RedisCluster, scan) {
     size_t pat_len = 0;
     int cmd_len;
     short slot;
-    zval *z_it, *z_node;
-    long it, num_ele, pat_free = 0;
+    zval *zcursor, *z_node;
+    long num_ele, pat_free = 0;
     zend_long count = 0;
+    zend_bool completed;
+    uint64_t cursor;
 
     /* Treat as read-only */
     c->readonly = CLUSTER_IS_ATOMIC(c);
@@ -2459,21 +2457,16 @@ PHP_METHOD(RedisCluster, scan) {
     }
 
     /* Parse arguments */
-    if (zend_parse_parameters(ZEND_NUM_ARGS(), "z/z|s!l", &z_it,
+    if (zend_parse_parameters(ZEND_NUM_ARGS(), "z/z|s!l", &zcursor,
                              &z_node, &pat, &pat_len, &count) == FAILURE)
     {
         RETURN_FALSE;
     }
 
-    /* Convert or update iterator */
-    if (Z_TYPE_P(z_it) != IS_LONG || Z_LVAL_P(z_it) < 0) {
-        convert_to_long(z_it);
-        it = 0;
-    } else if (Z_LVAL_P(z_it) != 0) {
-        it = Z_LVAL_P(z_it);
-    } else {
+    /* Get the scan cursor and return early if we're done */
+    cursor = redisGetScanCursor(zcursor, &completed);
+    if (completed)
         RETURN_FALSE;
-    }
 
     if (c->flags->scan & REDIS_SCAN_PREFIX) {
         pat_free = redis_key_prefix(c->flags, &pat, &pat_len);
@@ -2489,7 +2482,7 @@ PHP_METHOD(RedisCluster, scan) {
         }
 
         /* Construct our command */
-        cmd_len = redis_fmt_scan_cmd(&cmd, TYPE_SCAN, NULL, 0, it, pat, pat_len,
+        cmd_len = redis_fmt_scan_cmd(&cmd, TYPE_SCAN, NULL, 0, cursor, pat, pat_len,
             count);
 
         if ((slot = cluster_cmd_get_slot(c, z_node)) < 0) {
@@ -2505,7 +2498,7 @@ PHP_METHOD(RedisCluster, scan) {
         }
 
         if (cluster_scan_resp(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, TYPE_SCAN,
-                           &it) == FAILURE || Z_TYPE_P(return_value)!=IS_ARRAY)
+                              &cursor) == FAILURE || Z_TYPE_P(return_value) != IS_ARRAY)
         {
             CLUSTER_THROW_EXCEPTION("Couldn't process SCAN response from node", 0);
             efree(cmd);
@@ -2515,11 +2508,11 @@ PHP_METHOD(RedisCluster, scan) {
         efree(cmd);
 
         num_ele = zend_hash_num_elements(Z_ARRVAL_P(return_value));
-    } while (c->flags->scan & REDIS_SCAN_RETRY && it != 0 && num_ele == 0);
+    } while (c->flags->scan & REDIS_SCAN_RETRY && cursor != 0 && num_ele == 0);
 
     if (pat_free) efree(pat);
 
-    Z_LVAL_P(z_it) = it;
+    redisSetScanCursor(zcursor, cursor);
 }
 /* }}} */
 

redis_cluster.stub.php

@@ -488,7 +488,7 @@ class RedisCluster {
     /**
      * @see Redis::hscan
      */
-    public function hscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): array|bool;
+    public function hscan(string $key, null|int|string &$iterator, ?string $pattern = null, int $count = 0): array|bool;
 
     /**
      * @see https://redis.io/commands/hrandfield
@@ -787,7 +787,7 @@ class RedisCluster {
     /**
      * @see Redis::scan
      */
-    public function scan(?int &$iterator, string|array $key_or_address, ?string $pattern = null, int $count = 0): bool|array;
+    public function scan(null|int|string &$iterator, string|array $key_or_address, ?string $pattern = null, int $count = 0): bool|array;
 
     /**
      * @see Redis::scard
@@ -907,7 +907,7 @@ class RedisCluster {
     /**
      * @see Redis::sscan
      */
-    public function sscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): array|false;
+    public function sscan(string $key, null|int|string &$iterator, ?string $pattern = null, int $count = 0): array|false;
 
     /**
      * @see Redis::strlen
@@ -1154,7 +1154,7 @@ class RedisCluster {
     /**
      * @see Redis::zscan
      */
-    public function zscan(string $key, ?int &$iterator, ?string $pattern = null, int $count = 0): RedisCluster|bool|array;
+    public function zscan(string $key, null|int|string &$iterator, ?string $pattern = null, int $count = 0): RedisCluster|bool|array;
 
     /**
      * @see Redis::zscore

redis_cluster_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 35b71fe87bbd8df3a7495e14be957b18c3241a19 */
+ * Stub hash: c19108e54b637b6c76a529c1285104a0c38da220 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_TYPE_INFO(0, name, IS_STRING, 1)
@@ -406,7 +406,7 @@ ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_hscan, 0, 2, MAY_BE_ARRAY|MAY_BE_BOOL)
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
-	ZEND_ARG_TYPE_INFO(1, iterator, IS_LONG, 1)
+	ZEND_ARG_TYPE_MASK(1, iterator, MAY_BE_NULL|MAY_BE_LONG|MAY_BE_STRING, NULL)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, pattern, IS_STRING, 1, "null")
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()
@@ -660,7 +660,7 @@ ZEND_END_ARG_INFO()
 #define arginfo_class_RedisCluster_save arginfo_class_RedisCluster_bgrewriteaof
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_scan, 0, 2, MAY_BE_BOOL|MAY_BE_ARRAY)
-	ZEND_ARG_TYPE_INFO(1, iterator, IS_LONG, 1)
+	ZEND_ARG_TYPE_MASK(1, iterator, MAY_BE_NULL|MAY_BE_LONG|MAY_BE_STRING, NULL)
 	ZEND_ARG_TYPE_MASK(0, key_or_address, MAY_BE_STRING|MAY_BE_ARRAY, NULL)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, pattern, IS_STRING, 1, "null")
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
@@ -762,7 +762,7 @@ ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_class_RedisCluster_sscan, 0, 2, MAY_BE_ARRAY|MAY_BE_FALSE)
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
-	ZEND_ARG_TYPE_INFO(1, iterator, IS_LONG, 1)
+	ZEND_ARG_TYPE_MASK(1, iterator, MAY_BE_NULL|MAY_BE_LONG|MAY_BE_STRING, NULL)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, pattern, IS_STRING, 1, "null")
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()
@@ -1004,7 +1004,7 @@ ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_class_RedisCluster_zscan, 0, 2, RedisCluster, MAY_BE_BOOL|MAY_BE_ARRAY)
 	ZEND_ARG_TYPE_INFO(0, key, IS_STRING, 0)
-	ZEND_ARG_TYPE_INFO(1, iterator, IS_LONG, 1)
+	ZEND_ARG_TYPE_MASK(1, iterator, MAY_BE_NULL|MAY_BE_LONG|MAY_BE_STRING, NULL)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, pattern, IS_STRING, 1, "null")
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, count, IS_LONG, 0, "0")
 ZEND_END_ARG_INFO()

redis_cluster_legacy_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 35b71fe87bbd8df3a7495e14be957b18c3241a19 */
+ * Stub hash: c19108e54b637b6c76a529c1285104a0c38da220 */
 
 ZEND_BEGIN_ARG_INFO_EX(arginfo_class_RedisCluster___construct, 0, 0, 1)
 	ZEND_ARG_INFO(0, name)