Skip to content

Marc/resp3#477

Merged
mgravell merged 3 commits into
redis:marc/resp3from
mgravell:marc/resp3
May 2, 2026
Merged

Marc/resp3#477
mgravell merged 3 commits into
redis:marc/resp3from
mgravell:marc/resp3

Conversation

@mgravell
Copy link
Copy Markdown
Collaborator

@mgravell mgravell commented May 2, 2026

Note

Medium Risk
Changes parsing logic for Redis responses (RESP2 vs RESP3) which can affect runtime behavior across modules, plus updates test skip/version detection that may change which integration tests run. Risk is moderate due to protocol-variant handling and potential null/shape assumptions in response parsing.

Overview
Updates ResponseParser to correctly parse both RESP2 and RESP3 shapes for FT.CONFIG results (ToConfigDictionary) and RediSearch profiling output (ToProfileSearchResult) when RESP3 returns keyed maps instead of ordered/jagged arrays.

Refactors test skip attributes to expose their underlying SkipIfRedisCore and adds AbstractNRedisStackTest.AssertVersion() to re-check skip conditions using the actual server version discovered from the connection; several integration tests now call this to avoid running against unsupported Redis versions, and logging is made optional for tests that don’t provide an output helper.

Reviewed by Cursor Bugbot for commit 9a4de54. Bugbot is set up for automated code reviews on this repo. Configure here.

@mgravell mgravell merged commit e16236f into redis:marc/resp3 May 2, 2026
4 of 29 checks passed
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented May 2, 2026

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9a4de54. Configure here.

switch (results[i].ToString())
{
case "Results":
results[i + 1].ToSearchResult(q);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESP3 search result never assigned to variable

High Severity

In ToProfileSearchResult, the RESP3 branch calls results[i + 1].ToSearchResult(q) but discards the return value instead of assigning it to searchResult. This means searchResult stays null for all RESP3 responses, and the method returns a tuple with a null first element (masked by !). The RESP2 branch correctly does searchResult = results[0].ToSearchResult(q).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9a4de54. Configure here.

{
// RESP2: jagged; [ [key, value] ]
dict = new(value.Length);
for (int i = 0 ; i < value.Length ; i += 2)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESP2 config parsing skips every other entry

High Severity

In ToConfigDictionary, the RESP2 branch uses i += 2 but the data structure is a jagged array [ [key, value], [key, value], ... ] where each element is already a key-value pair. The original code iterated over every element with foreach. Using i += 2 skips every other config entry. The step should be i += 1 (or i++).

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9a4de54. Configure here.

mgravell added a commit that referenced this pull request May 11, 2026
* fix RESP3 mismatches

* dotnet format

* Spoof RESP2 MRANGE results from RESP3

* fix double-incr

* nits

* fix rule parsing

* fix info (flag) parsing

* fix JSON APIs

* fix spellcheck parsing

* fix content from info result

* Marc/resp3 (#477)

* ft.config get

* fix brittle TS.INFO tests (server version logic)

* more runtime server version checks; profiled RESP3 layout

* Marc/resp3 (#478)

* ft.config get

* fix brittle TS.INFO tests (server version logic)

* more runtime server version checks; profiled RESP3 layout

* dotnet format

* the bot is right

* - fix more search RESP3 behaviours
- normalize time series test namespaces

* deps

* note GHATL pin

* help me understand CI fail

* add some key randomness to TestJsonSetNotExistAsync

* add GetKeys helper to tell me what is missing in older servers

* handle v7 RESP3 profile layout

* dotnet format

* more v7 fixings

* and another

* make the bugbot happy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant