Skip to content

Commit 671e834

Browse files
committed
Fix missing response when AUTH is errored inside a transaction
When we add f8a5a4f, a module auth can add reply in its callback, or not add reply and just return an error robj in its callback, and we have to figure a way to determine whether it has added the reply. Before we are using `clientHasPendingReplies` and it had a issue, the auth might be used inside a transaction or pipeline which already has some pending replies in the client reply list. It causes us to not add the error reply to auth. After valkey-io#1819, now we have a buffered_reply client flag, it indicates the reply for the current command was buffered, either in client::reply or in client::buf. We can use this flag to check. Fixes valkey-io#2106. Signed-off-by: Binbin <[email protected]>
1 parent d37dc52 commit 671e834

File tree

2 files changed

+7
-1
lines changed

2 files changed

+7
-1
lines changed

src/acl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1439,7 +1439,7 @@ int ACLCheckUserCredentials(robj *username, robj *password) {
14391439
/* If `err` is provided, this is added as an error reply to the client.
14401440
* Otherwise, the standard Auth error is added as a reply. */
14411441
void addAuthErrReply(client *c, robj *err) {
1442-
if (clientHasPendingReplies(c)) return;
1442+
if (c->flag.buffered_reply) return;
14431443
if (!err) {
14441444
addReplyError(c, "-WRONGPASS invalid username-password pair or user is disabled.");
14451445
return;

tests/unit/multi.tcl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -955,6 +955,12 @@ start_server {tags {"multi"}} {
955955
$rd close
956956
}
957957

958+
test "AUTH errored inside MULTI will add the reply" {
959+
r config set requirepass ""
960+
r multi
961+
r auth no-user foobar
962+
assert_error {WRONGPASS invalid username-password pair or user is disabled.} {r exec}
963+
}
958964
}
959965

960966
start_server {overrides {appendonly {yes} appendfilename {appendonly.aof} appendfsync always} tags {external:skip}} {

0 commit comments

Comments
 (0)