From 0bfb6d55826ce128b8752430b4499146450deef4 Mon Sep 17 00:00:00 2001
From: YaacovHazan <31382944+YaacovHazan@users.noreply.github.com>
Date: Sun, 11 Jun 2023 23:12:05 +0300
Subject: [PATCH] Reset command duration for rejected command. (#12247)

In 7.2, After 971b177fa we make sure (assert) that
the duration has been recorded when resetting the client.

This is not true for rejected commands.
The use case I found is a blocking command that an ACL rule changed before
it was unblocked, and while reprocessing it, the command rejected and triggered the assert.

The PR reset the command duration inside rejectCommand / rejectCommandSds.

Co-authored-by: Oran Agra <oran@redislabs.com>
---
 src/server.c       |  3 +++
 tests/unit/acl.tcl | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/src/server.c b/src/server.c
index dba3c6262..84fb1aa48 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3714,9 +3714,11 @@ void call(client *c, int flags) {
  * various pre-execution checks. it returns the appropriate error to the client.
  * If there's a transaction is flags it as dirty, and if the command is EXEC,
  * it aborts the transaction.
+ * The duration is reset, since we reject the command, and it did not record.
  * Note: 'reply' is expected to end with \r\n */
 void rejectCommand(client *c, robj *reply) {
     flagTransaction(c);
+    c->duration = 0;
     if (c->cmd) c->cmd->rejected_calls++;
     if (c->cmd && c->cmd->proc == execCommand) {
         execCommandAbort(c, reply->ptr);
@@ -3728,6 +3730,7 @@ void rejectCommand(client *c, robj *reply) {
 
 void rejectCommandSds(client *c, sds s) {
     flagTransaction(c);
+    c->duration = 0;
     if (c->cmd) c->cmd->rejected_calls++;
     if (c->cmd && c->cmd->proc == execCommand) {
         execCommandAbort(c, s);
diff --git a/tests/unit/acl.tcl b/tests/unit/acl.tcl
index 4d8c77b9f..6dcee8b94 100644
--- a/tests/unit/acl.tcl
+++ b/tests/unit/acl.tcl
@@ -323,6 +323,23 @@ start_server {tags {"acl external:skip"}} {
         $rd close
     } {0}
 
+    test {blocked command gets rejected when reprocessed after permission change} {
+        r auth default ""
+        r config resetstat
+        set rd [redis_deferring_client]
+        r ACL setuser psuser reset on nopass +@all allkeys
+        $rd AUTH psuser pspass
+        $rd read
+        $rd BLPOP list1 0
+        wait_for_blocked_client
+        r ACL setuser psuser resetkeys
+        r LPUSH list1 foo
+        assert_error {*NOPERM No permissions to access a key*} {$rd read}
+        $rd ping
+        $rd close
+        assert_match {*calls=0,usec=0,*,rejected_calls=1,failed_calls=0} [cmdrstat blpop r]
+    }
+
     test {Users can be configured to authenticate with any password} {
         r ACL setuser newuser nopass
         r AUTH newuser zipzapblabla