Refactor InfoDetail parsing

[LeaFrock]

Motivation

Close #669

Changes

• Add InfoDetailsParser
• Delete SpanExtensions
• Add GetInfoAsync(string section) & GetInfoCategorizedAsync(string section)

Checklist

• Code compiles without warnings (TreatWarningsAsErrors is enabled)
• Tests pass locally (dotnet test)
• New code has test coverage
• No breaking changes to public API (or documented in PR description)

[imperugo]

Thanks @LeaFrock — the structural part is exactly what #669 asked for, and the chain worked out as planned: after #664 left SpanExtensions.cs holding only EnumerateLines, this empties it and the file goes away. InfoDetail on a primary constructor is tidy, and I appreciate that this one ships tests for the new API.

Two blockers though, and the second one is serious.

1. dict.Add where the old code used TryAdd — this is the red CI

ParseAsDictionary uses dict.Add(detail.Key, detail.Value). The ParseInfo it replaces used TryAdd:

foreach (var detail in data)
    result.TryAdd(detail.Key, detail.InfoValue);

That wasn't incidental — INFO emits duplicate keys. CI is failing on it right now:

System.ArgumentException : An item with the same key has already been added. Key: module
Failed Info_Should_Return_Valid_Information_Async

Note which test that is: not one of the new ones, but the existing coverage for GetInfoAsync(). So this is a regression on shipped public API — any server with more than one module loaded now throws, and Redis 8 loads several by default. TryAdd restores it.

2. The section parameter is a Lua injection

var script = $"return redis.call('INFO','{section}')";

A caller-supplied string goes straight into the script body. I ran this against a local Redis 8 rather than assuming:

section = "Server') and redis.call('SET','injected','pwned') --"

generated script:
  return redis.call('INFO','Server') and redis.call('SET','injected','pwned') --')

result:
  EXISTS injected -> 1
  GET injected    -> "pwned"

Arbitrary Redis commands from a parameter an application may well be passing through from its own input. It's the same shape we removed from HashGetAllAsyncAtOneTimeAsync in #659, and worse here — that one at least took a hash key, this takes a free-form string.

Passing the section as an argument closes it, and I verified both halves:

private const string InfoScript = "return redis.call('INFO')";
private const string InfoSectionScript = "return redis.call('INFO', ARGV[1])";

public async Task<Dictionary<string, string>> GetInfoAsync(string section)
{
    var result = string.IsNullOrWhiteSpace(section)
        ? await Database.ScriptEvaluateAsync(InfoScript).ConfigureAwait(false)
        : await Database.ScriptEvaluateAsync(InfoSectionScript, values: [section]).ConfigureAwait(false);

    return InfoDetailsParser.ParseAsDictionary(result.ToString());
}

EVAL "return redis.call('INFO', ARGV[1])" 0 "Server"
  -> # Server / redis_version:8.10.0 / ...                     (works)

EVAL "return redis.call('INFO', ARGV[1])" 0 "Server') and redis.call('SET','injected','pwned') --"
  -> (empty), EXISTS injected -> 0                              (inert)

It also fixes something the interpolated version costs us quietly: with the section baked into the script text, every distinct section is a distinct script, so the server-side script cache never hits and each call pays a full Lua parse. One constant script with ARGV[1] is cached once. Same reasoning as #659.

Smaller things

ParseAsTreeRows is dead code — nothing calls it. It also carries a latent crash: sections[^1].Add(detail) goes out of range if a key:value line arrives before any # Section header, since sections is still empty. Real Redis always leads with the header, but a compatible server (Valkey, Dragonfly, KeyDB) or a proxy needn't. Either drop the method or guard the case — I'd drop it and add it back when something needs it.

The line separator got strict. The old parser split on '\n' and then Trim()ed, so it handled \n and \r\n alike. This one requires literal "\r\n", and when it doesn't match it doesn't fail — it returns an empty dictionary. A silent empty result is harder to diagnose than a throw, and INFO is exactly the call people reach for when they're already debugging something.

The #if NET9_0_OR_GREATER split. Three methods, two bodies each, all running the same line loop — around 120 lines doing what one private line enumerator would do in 40. Not blocking, but it's a cost paid on every future edit to this file, and the two branches can drift without the compiler noticing.

Otherwise

The new interface methods are additive, consistent with how we shipped VectorSet in 12.1 and the IDistributedCache adapter in 12.5, so no versioning concern there.

Happy to push the TryAdd and ARGV[1] fixes onto this branch myself if you'd rather not — same as #664. Just say which you want to take.

[LeaFrock]

Thanks a lot. Right after I got the basic functionality working, my mom suddenly came down with a fever and cold, which took up all my time for further testing and debugging—that’s also why I marked the PR as Draft. Now, let me address your points one by one:

That wasn't incidental — INFO emits duplicate keys.

That is indeed unexpected. Given the circumstances, I agree we should follow your approach.

The section parameter is a Lua injection

I had considered this; whenever a database allows arbitrary strings, SQL injection is always the first thing that comes to mind. However, I’m not deeply familiar with Lua, so thank you for the heads-up and the explanation. I now understand the purpose of those optional parameters in ScriptEvaluateAsync. Your point about server-side command caching also reminds me of similar behaviors in databases. It’s great to see a library author thinking so thoroughly about these edge cases.

Real Redis always leads with the header, but a compatible server (Valkey, Dragonfly, KeyDB) or a proxy needn't. Either drop the method or guard the case — I'd drop it and add it back when something needs it.

This method isn't currently used. I kept it because it was part of the original implementation—it preserved the raw structure of INFO, including sections, the parent-child hierarchy of key-value pairs, and the output order. I thought we might need this raw, complete data in the future. However, per Occam’s razor, I agree we should remove it for now.

The line separator got strict.

The root cause is the explicit format specification in the Redis documentation:

a map of info fields, one field per line in the form of <field>:<value> where the value can be a comma separated map like <key>=<val>.
 Also contains section header lines starting with # and blank lines. 
Lines can contain a section name (starting with a # character) or a property.
 All the properties are in the form of field:value terminated by \r\n.

it returns an empty dictionary. A silent empty result is harder to diagnose than a throw ...

I have a slight disagreement here: I believe the code’s behavior should align strictly with the documentation. While using \n might maximize compatibility, it could also mask server-side errors and mislead debugging efforts. If a user calls this API and gets an empty dictionary, that anomaly itself should serve as a red flag.

but it's a cost paid on every future edit to this file, and the two branches can drift without the compiler noticing.

You are absolutely right. Let me explain my reasoning so you can make the final call without worrying about my objections:

1. I aimed to keep the parsing overhead minimal. By splitting the logic into two separate methods for different data structures, I reduced performance costs despite the similar high-level structure.
2. The .NET 9 conditional compilation was added because the new version is significantly cleaner and easier to read. This helps contributors quickly grasp the intent, and as we drop support for older frameworks, we can eventually keep only the concise version (admittedly, I’m thinking a bit long-term here).
3. This code is very stable (much like the old parser). For code edited this infrequently, the added maintenance cost feels justified.

I have addressed the two blockers you mentioned and removed the redundant methods from the Parser. If anything else needs fixing (including tests), please feel free to push directly to my branch. To be frank, I’m running very low on free time and will likely need to hand over the reins to you. That said, if you need my input on anything, I’m just a message away and will do my best to respond promptly.

[LeaFrock]

Adding a note: I still recommend introducing a breaking change in v14 to convert InfoDetail into a read-only struct. Memory-wise, this results in a more compact layout, which benefits the current JSON serialization scenarios. Semantically, since this data originates from the server, the type definition should reflect its read-only nature.