Redis ACL loading clobbers user preventing multiple user lines for same user

Attempting to "build" a user with config files as follows:

user steve reset
user steve on +@all ~*
user steve on >mypassword

(above is contrived example, in reality these lines are from different files imported in a controlled sequence)

The above config results in steve user being unable to access any keys or any commands.

I believe this logic in acl.c is flawed:

        user *u = ACLCreateUser(username,sdslen(username));
        if (!u) {
            u = ACLGetUserByName(username,sdslen(username));
            serverAssert(u != NULL);
            ACLSetUser(u,"reset",-1);
        }

Shouldn't the first call be to ACLGetUserByName, and then if not found (!u), ACLCreateUser called?

Comment From: stevelipinski

Upon further review, maybe this code is fine, but the reset is making it unable to define a user in multiple lines as shown in the example conf code. This seems like different behavior than acl setuser which allows one to "append" say passwords or other aspects of the rules. Can this reset call be removed from this case where users are loaded from config file?

Comment From: madolson

@stevelipinski Yeah, I agree that the behavior is weird, I would also expect having the same ACL user on multiple lines to either throw an error OR be cumulative. Silently reseting it seems wrong. We don't seem to define the behavior anywhere either, but I suppose the intention was that you define everything on one line. I would be inclined to just throw an error at this point, there is a lot of risk to changing the rules to cumulative.

@yossigo Any thoughts about this?