From 66be30f7fc2f7f6378eedc8d7b219db18addbc06 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Tue, 8 Feb 2022 10:01:35 +0200
Subject: Handle key-spec flags with modules (#10237)

- add COMMAND GETKEYSANDFLAGS sub-command
- add RM_KeyAtPosWithFlags and GetCommandKeysWithFlags
- RM_KeyAtPos and RM_CreateCommand set flags requiring full access for keys
- RM_CreateCommand set VARIABLE_FLAGS
- expose `variable_flags` flag in COMMAND INFO key-specs
- getKeysFromCommandWithSpecs prefers key-specs over getkeys-api
- add tests for all of these
---
 src/db.c | 50 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

(limited to 'src/db.c')

diff --git a/src/db.c b/src/db.c
index 218eaf567..11665fa1d 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1821,31 +1821,47 @@ invalid_spec:
  * associated with how Redis will access the key.
  *
  * 'cmd' must be point to the corresponding entry into the redisCommand
- * table, according to the command name in argv[0].
- *
- * This function uses the command's key specs, which contain the key-spec flags,
- * (e.g. RO / RW) and only resorts to the command-specific helper function if
- * any of the keys-specs are marked as INCOMPLETE. */
+ * table, according to the command name in argv[0]. */
 int getKeysFromCommandWithSpecs(struct redisCommand *cmd, robj **argv, int argc, int search_flags, getKeysResult *result) {
-    if (cmd->flags & CMD_MODULE_GETKEYS) {
+    /* The command has at least one key-spec not marked as CHANNEL */
+    int has_keyspec = (getAllKeySpecsFlags(cmd, 1) & CMD_KEY_CHANNEL);
+    /* The command has at least one key-spec marked as VARIABLE_FLAGS */
+    int has_varflags = (getAllKeySpecsFlags(cmd, 0) & CMD_KEY_VARIABLE_FLAGS);
+
+    /* Flags indicating that we have a getkeys callback */
+    int has_module_getkeys = cmd->flags & CMD_MODULE_GETKEYS;
+    int has_native_getkeys = !(cmd->flags & CMD_MODULE) && cmd->getkeys_proc;
+
+    /* The key-spec that's auto generated by RM_CreateCommand sets VARIABLE_FLAGS since no flags are given.
+     * If the module provides getkeys callback, we'll prefer it, but if it didn't, we'll use key-spec anyway. */
+    if ((cmd->flags & CMD_MODULE) && has_varflags && !has_module_getkeys)
+        has_varflags = 0;
+
+    /* We prefer key-specs if there are any, and their flags are reliable. */
+    if (has_keyspec && !has_varflags) {
+        int ret = getKeysUsingKeySpecs(cmd,argv,argc,search_flags,result);
+        if (ret >= 0)
+            return ret;
+        /* If the specs returned with an error (probably an INVALID or INCOMPLETE spec),
+         * fallback to the callback method. */
+    }
+
+    /* Resort to getkeys callback methods. */
+    if (has_module_getkeys)
         return moduleGetCommandKeysViaAPI(cmd,argv,argc,result);
-    } else {
-        if (!(getAllKeySpecsFlags(cmd, 0) & CMD_KEY_VARIABLE_FLAGS)) {
-            int ret = getKeysUsingKeySpecs(cmd,argv,argc,search_flags,result);
-            if (ret >= 0)
-                return ret;
-        }
-        if (!(cmd->flags & CMD_MODULE) && cmd->getkeys_proc)
-            return cmd->getkeys_proc(cmd,argv,argc,result);
-        return 0;
-    }
+
+    /* We use native getkeys as a last resort, since not all these native getkeys provide
+     * flags properly (only the ones that correspond to INVALID, INCOMPLETE or VARIABLE_FLAGS do.*/
+    if (has_native_getkeys)
+        return cmd->getkeys_proc(cmd,argv,argc,result);
+    return 0;
 }
 
 /* This function returns a sanity check if the command may have keys. */
 int doesCommandHaveKeys(struct redisCommand *cmd) {
     return (!(cmd->flags & CMD_MODULE) && cmd->getkeys_proc) || /* has getkeys_proc (non modules) */
         (cmd->flags & CMD_MODULE_GETKEYS) ||                    /* module with GETKEYS */
-        (getAllKeySpecsFlags(cmd, 1) & CMD_KEY_CHANNEL);       /* has at least one key-spec not marked as CHANNEL */
+        (getAllKeySpecsFlags(cmd, 1) & CMD_KEY_CHANNEL);        /* has at least one key-spec not marked as CHANNEL */
 }
 
 /* The base case is to use the keys position as given in the command table
-- 
cgit v1.2.1