MOD-8036 fix logic error in command registration with optional ACL ca…

…tegories (#396) * initial commit * inline formats * asserting that optional is not None if exists * asserting that optional is not None if exists
RedisLabsModules · Nov 5, 2024 · 9c012ce · 9c012ce
1 parent 670f9a4
commit 9c012ce
Showing 1 changed file with 18 additions and 14 deletions.
diff --git a/src/macros.rs b/src/macros.rs
@@ -58,29 +58,33 @@ macro_rules! redis_command {
             return $crate::raw::Status::Err as c_int;
         }
 
-        let mandatory_acl_categories = AclCategory::from($mandatory_acl_categories);
+        let mandatory = AclCategory::from($mandatory_acl_categories);
         if let Some(RM_SetCommandACLCategories) = $crate::raw::RedisModule_SetCommandACLCategories {
+            let mut optional_failed = true;
             let mut acl_categories = CString::default();
             $(
-                let optional_acl_categories = AclCategory::from($optional_acl_categories);
-                if mandatory_acl_categories != AclCategory::None && optional_acl_categories != AclCategory::None {
-                    acl_categories = CString::new(format!("{} {}", mandatory_acl_categories, optional_acl_categories)).unwrap();
-                } else if optional_acl_categories != AclCategory::None {
-                    acl_categories = CString::new(format!("{}", $optional_acl_categories)).unwrap();
+                let optional = AclCategory::from($optional_acl_categories);
+                assert!(optional != AclCategory::None);
+                optional_failed = false;
+                if mandatory != AclCategory::None {
+                    acl_categories = CString::new(format!("{mandatory} {optional}")).unwrap();
+                } else {
+                    acl_categories = CString::new(format!("{optional}")).unwrap();
                 }
                 // Warn if optional ACL categories are not set, but don't fail.
                 if RM_SetCommandACLCategories(command, acl_categories.as_ptr()) == $crate::raw::Status::Err as c_int {
+                    optional_failed = true;
                     $crate::raw::redis_log(
                         $ctx,
                         &format!(
                             "Warning: failed to set command `{}` ACL categories `{}`",
                             $command_name, acl_categories.to_str().unwrap()
                         ),
                     );
-                } else
+                }
             )?
-            if mandatory_acl_categories != AclCategory::None {
-                acl_categories = CString::new(format!("{}", mandatory_acl_categories)).unwrap();
+            if optional_failed && mandatory != AclCategory::None {
+                acl_categories = CString::new(format!("{mandatory}")).unwrap();
 
                 // Fail if mandatory ACL categories are not set.
                 if RM_SetCommandACLCategories(command, acl_categories.as_ptr())
@@ -89,14 +93,14 @@ macro_rules! redis_command {
                     $crate::raw::redis_log(
                         $ctx,
                         &format!(
-                            "Error: failed to set command `{}` mandatory ACL categories `{}`",
-                            $command_name, mandatory_acl_categories
+                            "Error: failed to set command `{}` mandatory ACL categories `{mandatory}`",
+                            $command_name
                         ),
                     );
                     return $crate::raw::Status::Err as c_int;
                 }
             }
-        } else if mandatory_acl_categories != AclCategory::None {
+        } else if mandatory != AclCategory::None {
             $crate::raw::redis_log(
                 $ctx,
                 "Error: Redis version does not support ACL categories",
@@ -317,9 +321,9 @@ macro_rules! redis_module {
                     if let Some(RM_AddACLCategory) = raw::RedisModule_AddACLCategory {
                         let module_acl_category = AclCategory::from($module_acl_category);
                         if module_acl_category != AclCategory::None {
-                            let category = CString::new(format!("{}", $module_acl_category)).unwrap();
+                            let category = CString::new(format!("{module_acl_category}")).unwrap();
                             if RM_AddACLCategory(ctx, category.as_ptr()) == raw::Status::Err as c_int {
-                                raw::redis_log(ctx, &format!("Error: failed to add ACL category `{}`", $module_acl_category));
+                                raw::redis_log(ctx, &format!("Error: failed to add ACL category `{module_acl_category}`"));
                                 return raw::Status::Err as c_int;
                             }
                         }