Fix DUMP binary data corruption in pipeline with decode_responses=True (#1884)#4006
Fix DUMP binary data corruption in pipeline with decode_responses=True (#1884)#4006r266-tech wants to merge 1 commit intoredis:masterfrom
Conversation
When decode_responses=True, the EXEC response in _execute_transaction was decoded as a single bulk response, causing NEVER_DECODE commands (e.g. DUMP) to have their binary data decoded into strings. Fix: read the EXEC response with disable_decoding=True and selectively decode each element based on the command's NEVER_DECODE option. Fixes redis#1884
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
| if NEVER_DECODE in options: | ||
| options.pop(NEVER_DECODE) | ||
| elif connection.encoder.decode_responses: | ||
| r = connection.encoder.decode(r) |
There was a problem hiding this comment.
Non-bytes responses silently skip decoding in transactions
High Severity
Reading the EXEC response with disable_decoding=True leaves ALL nested data as raw bytes, but the selective decoding only handles the case where r itself is bytes. Commands returning lists (e.g., LRANGE, SMEMBERS, MGET, HGETALL, KEYS) produce list-of-bytes responses where isinstance(r, list) is true, so the isinstance(r, bytes) check is false and no decoding occurs. This silently returns bytes instead of strings for all non-scalar command responses in transactional pipelines with decode_responses=True.
Additional Locations (1)
|
Hi @r266-tech, thank you for your contribution! Can you please check the test failures and the cursor bot comment? |


Problem
When
decode_responses=True, usingDUMPin a pipeline (both transactional and non-transactional) corrupts the binary data because the EXEC response is decoded as a single bulk response.The
NEVER_DECODEmechanism works correctly for non-transactional pipelines (where each command is parsed individually), but fails in transactional pipelines where all responses arrive as one multi-bulk from EXEC.Root Cause
In
Pipeline._execute_transaction, the EXEC response is parsed viaconnection.read_response()withoutdisable_decoding=True, so the entire multi-bulk response (including DUMP's binary data) gets decoded whendecode_responses=True.Fix
disable_decoding=Trueto preserve binary dataNEVER_DECODEin its optionsThis mirrors how non-transactional pipelines work — each command's options are checked individually.
Changes
redis/client.py: Modified_execute_transactionin syncPipelineclassredis/asyncio/client.py: Modified_execute_transactionin asyncPipelineclasstests/test_pipeline.py: Added 2 tests (non-transactional + transactional pipeline with DUMP + decode_responses)tests/test_asyncio/test_pipeline.py: Added 2 async tests (same scenarios)Tests
All existing pipeline tests pass (sync: 31 passed, async: 66 passed), plus the 4 new regression tests.
Fixes #1884
Note
Medium Risk
Touches core transactional pipeline response parsing/decoding for both sync and asyncio clients; could affect how bytes vs strings are returned for other commands under
decode_responses=True. Added regression tests reduce risk but behavior change is in a central execution path.Overview
Fixes binary response corruption for
DUMP(and otherNEVER_DECODEcommands) when running pipelines withdecode_responses=True, including transactional pipelines.Pipeline._execute_transaction(sync + asyncio) now reads theEXECmulti-bulk withdisable_decoding=Trueand then selectively decodes each element unless the queued command opted intoNEVER_DECODE.Adds sync and asyncio regression tests covering
DUMPin both transactional and non-transactional pipelines underdecode_responses=True.Written by Cursor Bugbot for commit c037f74. This will update automatically on new commits. Configure here.