ACL SETUSER not retaining argument order

[RohanNagar]

Bug Report

Current Behavior

Currently, when calling ACL SETUSER, the order that the commands are built are unexpected.

If I want to issue the redis command acl setuser default on -@connection +hello

then I would use the lettuce code for the setuser args:

AclSetuserArgs.Builder.on().removeCategory(AclCategory.CONNECTION).addCommand(CommandType.HELLO);

However, this ends up building this redis command: ACL SETUSER DEFAULT ON +HELLO -@CONNECTION

This is not the correct behavior as redis will interpret this to mean that hello is not an allowed command (since hello is part of the connection category).

Here is an example from redis-cli:

127.0.0.1:6379>acl setuser testOne on -@connection +hello
OK
127.0.0.1:6379>acl list
1) "user testOne on &* -@all +hello"
127.0.0.1:6379>acl setuser testTwo on +hello -@connection
OK
127.0.0.1:6379>acl list
1) "user testOne on &* -@all +hello"
2) "user testTwo on &* -@all"

As you can see the behavior is different based on the ordering of the arguments.

Input Code

Details above.

Expected behavior/code

I expect that the order I build the arguments using AclSetuserArgs should be preserved when building the command that is sent to redis.

Environment

• Lettuce version(s): 6.1.4.RELEASE
• Redis version: 6.2.1

Possible Solution

Within AclSetuserArgs.Builder we could build the string command as we go using a StringBuilder? Then the args.build() method would just call toString() on the StringBuilder.

Additional context

Just as an FYI this is different from #1846 (a bug fix that I opened today related to setting the nocommands option on SETUSER.

[mp911de]

Thanks for bringing this up. I wasn't aware that Redis considers the order of ACL items. Right now, AclSetuserArgs stores command additions/removals and category additions/removals in individual fields hence it doesn't preserve the order. We cannot use StringBuilder as we need to encode each argument individually which is a requirement of the Redis protocol. What we could do is having commands captured as instruction in a List (AddCommand, RemoveCommand, AddAclCategory, RemoveAclCategory) to retain the actual order in which the command should be constructed.

We could do this also for all the other arguments passed to AclSetuserArgs. Do you want to have a look at a possible fix?

[RohanNagar]

Thank you for the suggestion! I have attempted a fix in #1848 - please take a look and let me know your thoughts.