Skip to content

Commit 88d16d0

Browse files
authored
Decode ACL LOG string values on the default RESP3 legacy callback (#4201)
`parse_acl_log_resp3_to_resp2_legacy` decoded the map keys but left the string values as bytes, so `Redis().acl_log()` (the default: RESP3 wire with legacy responses) returned e.g. `reason=b'auth'` while `Redis(protocol=2).acl_log()` returned `reason='auth'`, breaking the documented RESP2-compatible shape. The base RESP2 callback (`pairs_to_dict(log, True, True)`) and the unified RESP3 callback both decode string values; mirror that here. Non-string values (ints) pass through `str_if_bytes` unchanged.
1 parent 599fd75 commit 88d16d0

2 files changed

Lines changed: 56 additions & 1 deletion

File tree

redis/_parsers/helpers.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1341,7 +1341,7 @@ def parse_acl_log_resp3_to_resp2_legacy(response, **options):
13411341
data = []
13421342
for log in response:
13431343
if isinstance(log, dict):
1344-
log_data = {str_if_bytes(k): v for k, v in log.items()}
1344+
log_data = {str_if_bytes(k): str_if_bytes(v) for k, v in log.items()}
13451345
else:
13461346
log_data = pairs_to_dict(log, True, True)
13471347
client_info = log_data.get("client-info", "")

tests/test_parsers/test_helpers.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import pytest
22

33
from redis._parsers.helpers import (
4+
parse_acl_log,
5+
parse_acl_log_resp3_to_resp2_legacy,
46
parse_client_list,
57
parse_command,
68
parse_info,
@@ -143,3 +145,56 @@ def test_parse_sentinel_masters_resp3_returns_master_dict():
143145
assert masters["redis-py-test"]["flags"] == {"master"}
144146
assert masters["redis-py-test"]["is_master"] is True
145147
assert masters["redis-py-test"]["is_sdown"] is False
148+
149+
150+
@pytest.mark.fixed_client
151+
def test_parse_acl_log_resp3_legacy_decodes_string_values():
152+
# On a RESP3 connection with the default legacy_responses=True, each ACL
153+
# LOG entry arrives as a map. The scalar string fields (reason, context,
154+
# object, username) are bulk strings and must be decoded to ``str`` so the
155+
# result matches what parse_acl_log() produces from a RESP2 connection.
156+
client_info = b"id=3 addr=127.0.0.1:52654 name= age=0 user=someuser"
157+
resp3_entry = {
158+
b"count": 1,
159+
b"reason": b"auth",
160+
b"context": b"toplevel",
161+
b"object": b"AUTH",
162+
b"username": b"someuser",
163+
b"age-seconds": b"8.038",
164+
b"client-info": client_info,
165+
b"entry-id": 0,
166+
b"timestamp-created": 1700000000000,
167+
b"timestamp-last-updated": 1700000000000,
168+
}
169+
170+
parsed = parse_acl_log_resp3_to_resp2_legacy([resp3_entry])[0]
171+
172+
assert parsed["reason"] == "auth"
173+
assert parsed["context"] == "toplevel"
174+
assert parsed["object"] == "AUTH"
175+
assert parsed["username"] == "someuser"
176+
177+
# Must match the RESP2-wire legacy shape exactly (the default-config promise).
178+
resp2_entry = [
179+
b"count",
180+
1,
181+
b"reason",
182+
b"auth",
183+
b"context",
184+
b"toplevel",
185+
b"object",
186+
b"AUTH",
187+
b"username",
188+
b"someuser",
189+
b"age-seconds",
190+
b"8.038",
191+
b"client-info",
192+
client_info,
193+
b"entry-id",
194+
0,
195+
b"timestamp-created",
196+
1700000000000,
197+
b"timestamp-last-updated",
198+
1700000000000,
199+
]
200+
assert parse_acl_log([resp2_entry]) == [parsed]

0 commit comments

Comments
 (0)