ACL: better define name, and the idea of reserved ID.

src/acl.c

@@ -122,7 +122,7 @@ user *ACLCreateUser(const char *name, size_t namelen) {
  * bit. The function returns C_ERR in case the specified ID overflows
  * the bitmap in the user representation. */
 int ACLGetCommandBitCoordinates(unsigned long id, uint64_t *word, uint64_t *bit) {
-    if (id >= USER_MAX_COMMAND_BIT) return C_ERR;
+    if (id >= USER_COMMAND_BITS_COUNT) return C_ERR;
     *word = id / sizeof(uint64_t) / 8;
     *bit = 1 << (id % (sizeof(uint64_t) * 8));
     return C_OK;
@@ -317,7 +317,7 @@ int ACLSetUser(user *u, const char *op, ssize_t oplen) {
             /* If this is the first subcommand to be configured for
              * this user, we have to allocate the subcommands array. */
             if (u->allowed_subcommands == NULL) {
-                u->allowed_subcommands = zcalloc(USER_MAX_COMMAND_BIT *
+                u->allowed_subcommands = zcalloc(USER_COMMAND_BITS_COUNT *
                                          sizeof(sds*));
             }
 
@@ -439,7 +439,18 @@ unsigned long ACLGetCommandID(const char *cmdname) {
     raxInsert(map,(unsigned char*)lowername,strlen(lowername),
               (void*)nextid,NULL);
     sdsfree(lowername);
-    return nextid++;
+    nextid++;
+
+    /* We never assign the last bit in the user commands bitmap structure,
+     * this way we can later check if this bit is set, understanding if the
+     * current ACL for the user was created starting with a +@all to add all
+     * the possible commands and just subtracting other single commands or
+     * categories, or if, instead, the ACL was created just adding commands
+     * and command categories from scratch, not allowing future commands by
+     * default (loaded via modules). This is useful when rewriting the ACLs
+     * with ACL SAVE. */
+    if (nextid == USER_COMMAND_BITS_COUNT-1) nextid++;
+    return nextid;
 }
 
 /* Return an username by its name, or NULL if the user does not exist. */

src/server.h

@@ -712,8 +712,10 @@ typedef struct readyList {
 /* This structure represents a Redis user. This is useful for ACLs, the
  * user is associated to the connection after the connection is authenticated.
  * If there is no associated user, the connection uses the default user. */
-#define USER_MAX_COMMAND_BIT 1024       /* The first *not valid* bit that
+#define USER_COMMAND_BITS_COUNT 1024    /* The total number of command bits
-                                           would overflow. So check for >= */
+                                           in the user structure. The last valid
+                                           command ID we can set in the user
+                                           is USER_COMMAND_BITS_COUNT-1. */
 #define USER_FLAG_ENABLED (1<<0)        /* The user is active. */
 #define USER_FLAG_ALLKEYS (1<<1)        /* The user can mention any key. */
 #define USER_FLAG_ALLCOMMANDS (1<<2)    /* The user can run all commands. */
@@ -734,13 +736,13 @@ typedef struct user {
      * If the bit for a given command is NOT set and the command has
      * subcommands, Redis will also check allowed_subcommands in order to
      * understand if the command can be executed. */
-    uint64_t allowed_commands[USER_MAX_COMMAND_BIT/64];
+    uint64_t allowed_commands[USER_COMMAND_BITS_COUNT/64];
 
     /* This array points, for each command ID (corresponding to the command
      * bit set in allowed_commands), to an array of SDS strings, terminated by
      * a NULL pointer, with all the sub commands that can be executed for
      * this command. When no subcommands matching is used, the field is just
-     * set to NULL to avoid allocating USER_MAX_COMMAND_BIT pointers. */
+     * set to NULL to avoid allocating USER_COMMAND_BITS_COUNT pointers. */
     sds **allowed_subcommands;
     list *passwords; /* A list of SDS valid passwords for this user. */
     list *patterns;  /* A list of allowed key patterns. If this field is NULL