From 05a8cbfbdfd1e47e866924ceae59df5babffb464 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 09:29:03 +0100 Subject: [PATCH 1/7] Read HELLO "server" before trusting "version" as the Redis API version (#3278) Valkey reports its own version as HELLO "version" ("server" => "valkey", "version" => "9.1.2") while INFO still says redis_version:7.2.4; Garnet keeps "version" as the Redis API version ("server" => "redis") and adds "garnet_version". Both the interactive and subscription connections issue HELLO, so the detected version depended on whether the last reply processed was HELLO or INFO, and version-gated commands (DELEX for LockRelease) were sent to servers that don't have them. Now "version" only sets the Redis API version when "server" is redis; for a recognised non-Redis server it feeds the product variant instead, and for an unrecognised server it is ignored. A "_version" HELLO field sets the product variant in the Garnet shape. The in-process test server can emulate both measured shapes, and a HELLO-only test (INFO disabled) pins the rule down on RESP2 and RESP3. --- .../Enums/ProductVariant.cs | 26 ++++- src/StackExchange.Redis/HelloField.cs | 41 +++++++- src/StackExchange.Redis/LoggerExtensions.cs | 12 +++ src/StackExchange.Redis/ResultProcessor.cs | 73 +++++++++++++- .../ProductVariantUnitTests.cs | 96 +++++++++++++++++-- .../ValkeyUnitTests.cs | 8 +- .../StackExchange.Redis.Server/RedisServer.cs | 49 +++++++--- 7 files changed, 278 insertions(+), 27 deletions(-) diff --git a/src/StackExchange.Redis/Enums/ProductVariant.cs b/src/StackExchange.Redis/Enums/ProductVariant.cs index fd3a17086c..6b6c046d20 100644 --- a/src/StackExchange.Redis/Enums/ProductVariant.cs +++ b/src/StackExchange.Redis/Enums/ProductVariant.cs @@ -1,48 +1,70 @@ -// ReSharper disable once CheckNamespace +using System; +using RESPite; + +// ReSharper disable once CheckNamespace namespace StackExchange.Redis; /// /// Indicates the flavor of RESP-server being used, if known. Unknown variants will be reported as . /// Inclusion (or omission) in this list does not imply support for any given variant; nor does it indicate any specific /// relationship with the vendor or rights to use the name. It is provided solely for informational purposes. Identification -/// is not guaranteed, and is based on the server's self-reporting (typically via `INFO`), which may be incomplete or misleading. +/// is not guaranteed, and is based on the server's self-reporting (via `INFO` and `HELLO`), which may be incomplete or misleading. /// public enum ProductVariant { /// /// The original Redis server. This is also the default value if the variant is unknown. /// + [AsciiHash("redis")] Redis, /// /// Valkey is a fork of open-source Redis associated with AWS. /// + [AsciiHash("valkey")] Valkey, /// /// Garnet is a Redis-compatible server from Microsoft. /// + [AsciiHash("garnet")] Garnet, /// /// Dragonfly /// + [AsciiHash("dragonfly")] Dragonfly, /// /// Memurai /// + [AsciiHash("memurai")] Memurai, /// /// Redict /// + [AsciiHash("redict")] Redict, /// /// KeyDB /// + [AsciiHash("keydb")] KeyDB, // if you want to add another variant here, please open an issue with the details (variant name, INFO output, etc.) } + +/// +/// Metadata and parsing methods for . +/// +internal static partial class ProductVariantMetadata +{ + /// + /// Parses the server value reported by HELLO (redis, valkey, etc). + /// + [AsciiHash] + internal static partial bool TryParse(ReadOnlySpan value, out ProductVariant variant); +} diff --git a/src/StackExchange.Redis/HelloField.cs b/src/StackExchange.Redis/HelloField.cs index e5d730ce49..a7a5c37e84 100644 --- a/src/StackExchange.Redis/HelloField.cs +++ b/src/StackExchange.Redis/HelloField.cs @@ -1,4 +1,4 @@ -using System; +using System; using RESPite; namespace StackExchange.Redis; @@ -15,11 +15,48 @@ internal enum HelloField Unknown = 0, /// - /// Redis server version. + /// The server product: redis for Redis and for servers that present as Redis (Garnet); Valkey reports valkey. + /// + [AsciiHash("server")] + Server, + + /// + /// Server version. This is only the Redis API version when is redis; Valkey reports its own + /// version here (9.1.2, say, while INFO still gives redis_version:7.2.4). /// [AsciiHash("version")] Version, + /// + /// Garnet's own version; Garnet reports server as redis and as the Redis API version. + /// + [AsciiHash("garnet_version")] + GarnetVersion, + + /// + /// Valkey's own version, if reported alongside . + /// + [AsciiHash("valkey_version")] + ValkeyVersion, + + /// + /// Dragonfly's own version, if reported. + /// + [AsciiHash("dragonfly_version")] + DragonflyVersion, + + /// + /// Memurai's own version, if reported. + /// + [AsciiHash("memurai_version")] + MemuraiVersion, + + /// + /// Redict's own version, if reported. + /// + [AsciiHash("redict_version")] + RedictVersion, + /// /// Protocol version (2 or 3). /// diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index 917d71ed63..df661e32ec 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -642,6 +642,18 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) Message = "{Server}: Auto-configured (HELLO) role: {Role}")] internal static partial void LogInformationAutoConfiguredHelloRole(this ILogger logger, ServerEndPointLogValue server, string role); + [LoggerMessage( + Level = LogLevel.Information, + EventId = 131, + Message = "{Server}: Auto-configured (HELLO) product: {Variant} {Version}")] + internal static partial void LogInformationAutoConfiguredHelloProductVariant(this ILogger logger, ServerEndPointLogValue server, ProductVariant variant, string version); + + [LoggerMessage( + Level = LogLevel.Information, + EventId = 132, + Message = "{Server}: Auto-configured (HELLO) unrecognized server '{ServerName}'; ignoring its version")] + internal static partial void LogInformationAutoConfiguredHelloUnknownServer(this ILogger logger, ServerEndPointLogValue server, string serverName); + // PhysicalBridge logging methods [LoggerMessage( Level = LogLevel.Information, diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index 6e8d17fee5..96ae5201ac 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/src/StackExchange.Redis/ResultProcessor.cs @@ -1152,6 +1152,14 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes // Handle HELLO command (returns array/map of key-value pairs) if (message?.Command == RedisCommand.HELLO && reader.IsAggregate) { + // "version" means different things depending on "server": Redis (and Garnet, which presents as + // "redis" and adds "garnet_version") report the Redis API version, but Valkey reports *its own* + // version ("server" => "valkey", "version" => "9.1.2", while INFO still says redis_version:7.2.4), + // so we can only decide what to do with "version" once the whole map has been read (#3278). + bool serverIsRedis = true; // absent means redis (older servers) + ProductVariant? helloVariant = null, vendorVariant = null; + string? helloVersion = null, vendorVersion = null; + var iter = reader.AggregateChildren(); while (iter.MoveNext()) { @@ -1167,9 +1175,40 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes switch (field) { - case HelloField.Version when iter.Value.TryParseScalar(Format.TryParseVersion!, out Version version): - server.Version = version; - Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); + case HelloField.Server: + if (iter.Value.TryParseScalar(&ProductVariantMetadata.TryParse, out ProductVariant parsedVariant)) + { + helloVariant = parsedVariant; + serverIsRedis = parsedVariant is ProductVariant.Redis; + } + else + { + serverIsRedis = false; + Log?.LogInformationAutoConfiguredHelloUnknownServer(new(server), iter.Value.ReadString() ?? ""); + } + break; + case HelloField.Version: + helloVersion = iter.Value.ReadString(); + break; + case HelloField.GarnetVersion: + vendorVariant = ProductVariant.Garnet; + vendorVersion = iter.Value.ReadString(); + break; + case HelloField.ValkeyVersion: + vendorVariant = ProductVariant.Valkey; + vendorVersion = iter.Value.ReadString(); + break; + case HelloField.DragonflyVersion: + vendorVariant = ProductVariant.Dragonfly; + vendorVersion = iter.Value.ReadString(); + break; + case HelloField.MemuraiVersion: + vendorVariant = ProductVariant.Memurai; + vendorVersion = iter.Value.ReadString(); + break; + case HelloField.RedictVersion: + vendorVariant = ProductVariant.Redict; + vendorVersion = iter.Value.ReadString(); break; case HelloField.Proto when iter.Value.TryReadInt64(out var i64): connection.SetProtocol(i64 >= 3 ? RedisProtocol.Resp3 : RedisProtocol.Resp2); @@ -1204,6 +1243,34 @@ when server.ServerType.SupportsAutoConfigure() } } + if (serverIsRedis) + { + // "version" is the Redis API version; a "{vendor}_version" field, if any, identifies the product + if (Format.TryParseVersion(helloVersion, out Version? version)) + { + server.Version = version; + Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); + } + if (vendorVariant is { } fromField && !string.IsNullOrWhiteSpace(vendorVersion)) + { + server.SetProductVariant(fromField, vendorVersion!); + Log?.LogInformationAutoConfiguredHelloProductVariant(new(server), fromField, vendorVersion!); + } + } + else if (helloVariant is { } fromServer) + { + // a known non-Redis server: "version" is the vendor's own version, and the Redis API version + // (if the server reports one at all) comes from INFO's redis_version + var productVersion = vendorVariant == fromServer && !string.IsNullOrWhiteSpace(vendorVersion) + ? vendorVersion! : helloVersion; + if (!string.IsNullOrWhiteSpace(productVersion)) + { + server.SetProductVariant(fromServer, productVersion!); + Log?.LogInformationAutoConfiguredHelloProductVariant(new(server), fromServer, productVersion!); + } + } + + // (an unrecognized server falls through: we can't know what its "version" means, so we leave it alone) SetResult(message, true); return true; } diff --git a/tests/StackExchange.Redis.Tests/ProductVariantUnitTests.cs b/tests/StackExchange.Redis.Tests/ProductVariantUnitTests.cs index 828df34354..bd4a6e54df 100644 --- a/tests/StackExchange.Redis.Tests/ProductVariantUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ProductVariantUnitTests.cs @@ -1,10 +1,13 @@ -using System.Linq; +using System; +using System.Collections.Generic; +using System.Linq; using System.Text; using System.Threading.Tasks; using Xunit; namespace StackExchange.Redis.Tests; +[RunPerProtocol] public class ProductVariantUnitTests(ITestOutputHelper log) { [Theory] @@ -14,19 +17,76 @@ public class ProductVariantUnitTests(ITestOutputHelper log) public async Task DetectProductVariant(ProductVariant variant) { using var serverObj = new ProductServer(variant, log); - using var conn = await serverObj.ConnectAsync(withPubSub: false); + // with pub/sub, both connections issue HELLO, in either order relative to the interactive INFO + using var conn = await serverObj.ConnectAsync(withPubSub: true); var serverApi = conn.GetServer(conn.GetEndPoints().First()); serverApi.Ping(); var reportedProduct = serverApi.GetProductVariant(out var reportedVersion); Assert.Equal(variant, reportedProduct); - log.WriteLine($"Detected {reportedProduct} version: {reportedVersion}"); + log.WriteLine($"Detected {reportedProduct} version: {reportedVersion}; redis API version: {serverApi.Version}"); if (variant == ProductVariant.Redis) { Assert.Equal(serverObj.VersionString, reportedVersion); } else { - Assert.Equal("1.2.3-preview4", reportedVersion); + Assert.Equal(ProductServer.VendorVersion, reportedVersion); + } + // the Redis API version must not be confused with the vendor's own version (#3278) + Assert.Equal(serverObj.RedisVersion, serverApi.Version); + } + + [Theory] + [InlineData(ProductVariant.Redis)] + [InlineData(ProductVariant.Valkey)] + [InlineData(ProductVariant.Garnet)] + public async Task DetectProductVariantFromHelloAlone(ProductVariant variant) + { + // with INFO disabled, HELLO is the only source of version data, so this pins down what we take from it: + // "version" is the Redis API version only when "server" is redis (Redis, Garnet); Valkey repurposes it (#3278) + using var serverObj = new ProductServer(variant, log); + var config = serverObj.GetClientConfig(withPubSub: false); + var commands = serverObj.GetCommands(); + commands.Remove(nameof(RedisCommand.INFO)); + config.CommandMap = CommandMap.Create(commands); + var fallbackVersion = new Version(6, 0, 0); // HELLO exists from 6.0, and the fake server reports 7.0.0 + config.DefaultVersion = fallbackVersion; + + await using var conn = await ConnectionMultiplexer.ConnectAsync(config); + var serverApi = conn.GetServer(conn.GetEndPoints().First()); + await serverApi.PingAsync(); + Assert.Equal(variant, serverApi.GetProductVariant(out var reportedVersion)); + log.WriteLine($"Detected {variant} version: {reportedVersion}; redis API version: {serverApi.Version}"); + switch (variant) + { + case ProductVariant.Redis: + Assert.Equal(serverObj.RedisVersion, serverApi.Version); + break; + case ProductVariant.Garnet: + Assert.Equal(serverObj.RedisVersion, serverApi.Version); + Assert.Equal(ProductServer.VendorVersion, reportedVersion); + break; + case ProductVariant.Valkey: + Assert.Equal(fallbackVersion, serverApi.Version); // HELLO told us nothing about the Redis API version + Assert.Equal(ProductServer.VendorVersion, reportedVersion); + break; + } + } + + [Fact] + public async Task ValkeyVersionIsStableAcrossConnections() + { + // #3278: Valkey reports its own version as HELLO "version", so the detected Redis API version used to + // depend on whether the (subscription) HELLO or the (interactive) INFO reply was processed last + using var serverObj = new ProductServer(ProductVariant.Valkey, log); + for (int i = 0; i < 10; i++) + { + await using var conn = await serverObj.ConnectAsync(withPubSub: true); + var serverApi = conn.GetServer(conn.GetEndPoints().First()); + await serverApi.PingAsync(); + Assert.Equal(serverObj.RedisVersion, serverApi.Version); + Assert.Equal(ProductVariant.Valkey, serverApi.GetProductVariant(out var reportedVersion)); + Assert.Equal(ProductServer.VendorVersion, reportedVersion); } } @@ -73,6 +133,8 @@ public async Task MultiDbSupportMatchesProductVariantAndServerType(ProductVarian private sealed class ProductServer : InProcessTestServer { + public const string VendorVersion = "9.1.2"; // must parse as a Version, or the HELLO-version confusion goes unnoticed + private readonly ProductVariant _variant; public ProductServer(ProductVariant variant, ITestOutputHelper log, ServerType serverType = ServerType.Standalone) @@ -90,16 +152,38 @@ protected override void Info(StringBuilder sb, string section, Server.RedisClien switch (_variant) { case ProductVariant.Garnet: - sb.AppendLine("garnet_version:1.2.3-preview4"); + sb.AppendLine("garnet_version:" + VendorVersion) + .AppendLine("server_name:garnet"); break; case ProductVariant.Valkey: - sb.AppendLine("valkey_version:1.2.3-preview4") + sb.AppendLine("valkey_version:" + VendorVersion) .AppendLine("server_name:valkey"); break; } } } + // HELLO shapes as measured against real servers (#3278): + // Garnet 2.2.2: server => redis, version => 7.4.3 (the Redis API version), garnet_version => 2.2.2 + // Valkey 9.1.2: server => valkey, version => 9.1.2 (Valkey's own version; INFO has redis_version:7.2.4) + protected override string HelloServerName => _variant switch + { + ProductVariant.Valkey => "valkey", + _ => "redis", + }; + + protected override string HelloVersionString => _variant switch + { + ProductVariant.Valkey => VendorVersion, + _ => VersionString, + }; + + protected override IReadOnlyList> HelloExtraFields => _variant switch + { + ProductVariant.Garnet => [new("garnet_version", VendorVersion)], + _ => [], + }; + protected override bool SupportMultiDb(out string err) { switch (_variant) diff --git a/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs b/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs index d9a704a04d..b4b532687e 100644 --- a/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs @@ -4,6 +4,7 @@ namespace StackExchange.Redis.Tests; +[RunPerProtocol] public class ValkeyUnitTests(ITestOutputHelper log) { [Theory] @@ -16,7 +17,8 @@ public async Task IdentifyValkeyCluster(ServerType type) var serverApi = client.GetServer(server.DefaultEndPoint); Assert.Equal(type, serverApi.ServerType); Assert.Equal(ProductVariant.Valkey, serverApi.GetProductVariant(out var version)); - Assert.Equal("8.1", version); + Assert.Equal("8.1", version); + Assert.Equal(server.RedisVersion, serverApi.Version); // not the valkey version from HELLO (#3278) } private sealed class InProcessValkeyLikeServer(ITestOutputHelper log) : InProcessTestServer(log) @@ -24,6 +26,10 @@ private sealed class InProcessValkeyLikeServer(ITestOutputHelper log) : InProces // see https://github.com/StackExchange/StackExchange.Redis/pull/3050 protected override string ServerModeKey => "server_mode"; + // Valkey reports its own version as HELLO "version" (see #3278) + protected override string HelloServerName => "valkey"; + protected override string HelloVersionString => "8.1"; + protected override void Info(StringBuilder sb, string section, Server.RedisClient? client = null) { base.Info(sb, section, client); diff --git a/toys/StackExchange.Redis.Server/RedisServer.cs b/toys/StackExchange.Redis.Server/RedisServer.cs index 7fe8c46a93..beb4aea651 100644 --- a/toys/StackExchange.Redis.Server/RedisServer.cs +++ b/toys/StackExchange.Redis.Server/RedisServer.cs @@ -625,24 +625,47 @@ protected virtual TypedRedisValue Hello(RedisClient client, in RedisRequest requ client.IsAuthenticated = isAuthed; client.Name = name; - var reply = TypedRedisValue.Rent(14, out var span, RespPrefix.Map); + var extras = HelloExtraFields; + var reply = TypedRedisValue.Rent(14 + (2 * extras.Count), out var span, RespPrefix.Map); span[0] = TypedRedisValue.BulkString("server"); - span[1] = TypedRedisValue.BulkString("redis"); + span[1] = TypedRedisValue.BulkString(HelloServerName); span[2] = TypedRedisValue.BulkString("version"); - span[3] = TypedRedisValue.BulkString(VersionString); - span[4] = TypedRedisValue.BulkString("proto"); - span[5] = TypedRedisValue.Integer(proto32); - span[6] = TypedRedisValue.BulkString("id"); - span[7] = TypedRedisValue.Integer(client.Id); - span[8] = TypedRedisValue.BulkString("mode"); - span[9] = TypedRedisValue.BulkString(ServerModeValue); - span[10] = TypedRedisValue.BulkString("role"); - span[11] = TypedRedisValue.BulkString(GetPrimaryOf(client?.Node) is null ? "master" : "replica"); - span[12] = TypedRedisValue.BulkString("modules"); - span[13] = TypedRedisValue.EmptyArray(RespPrefix.Array); + span[3] = TypedRedisValue.BulkString(HelloVersionString); + int index = 4; + foreach (var pair in extras) + { + span[index++] = TypedRedisValue.BulkString(pair.Key); + span[index++] = TypedRedisValue.BulkString(pair.Value); + } + span[index++] = TypedRedisValue.BulkString("proto"); + span[index++] = TypedRedisValue.Integer(proto32); + span[index++] = TypedRedisValue.BulkString("id"); + span[index++] = TypedRedisValue.Integer(client.Id); + span[index++] = TypedRedisValue.BulkString("mode"); + span[index++] = TypedRedisValue.BulkString(ServerModeValue); + span[index++] = TypedRedisValue.BulkString("role"); + span[index++] = TypedRedisValue.BulkString(GetPrimaryOf(client?.Node) is null ? "master" : "replica"); + span[index++] = TypedRedisValue.BulkString("modules"); + span[index] = TypedRedisValue.EmptyArray(RespPrefix.Array); return reply; } + /// + /// The server reported by HELLO; Redis (and Garnet) report redis, Valkey reports valkey. + /// + protected virtual string HelloServerName => "redis"; + + /// + /// The version reported by HELLO; this is the Redis API version when + /// is redis, but Valkey reports its own version here. + /// + protected virtual string HelloVersionString => VersionString; + + /// + /// Additional fields reported by HELLO (Garnet adds garnet_version, for example). + /// + protected virtual IReadOnlyList> HelloExtraFields => []; + [RedisCommand(-3)] protected virtual TypedRedisValue Sadd(RedisClient client, in RedisRequest request) { From c101b0d21531fd21a19bbf9118169fa5401ca9c4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 09:38:04 +0100 Subject: [PATCH 2/7] Snapshot the HELLO "version" reader instead of allocating a string Parse it as a Version (no string) once we know "server" is redis; only the vendor path, which stores the string anyway, materializes it. --- src/StackExchange.Redis/ResultProcessor.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index 96ae5201ac..db7e0a8f84 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/src/StackExchange.Redis/ResultProcessor.cs @@ -1158,7 +1158,10 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes // so we can only decide what to do with "version" once the whole map has been read (#3278). bool serverIsRedis = true; // absent means redis (older servers) ProductVariant? helloVariant = null, vendorVariant = null; - string? helloVersion = null, vendorVersion = null; + string? vendorVersion = null; + // a snapshot of the "version" value, so we can read it as a Version or a string once we know which + RespReader helloVersion = default; + bool hasHelloVersion = false; var iter = reader.AggregateChildren(); while (iter.MoveNext()) @@ -1188,7 +1191,8 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes } break; case HelloField.Version: - helloVersion = iter.Value.ReadString(); + helloVersion = iter.Value; + hasHelloVersion = true; break; case HelloField.GarnetVersion: vendorVariant = ProductVariant.Garnet; @@ -1246,7 +1250,7 @@ when server.ServerType.SupportsAutoConfigure() if (serverIsRedis) { // "version" is the Redis API version; a "{vendor}_version" field, if any, identifies the product - if (Format.TryParseVersion(helloVersion, out Version? version)) + if (hasHelloVersion && helloVersion.TryParseScalar(Format.TryParseVersion!, out Version version)) { server.Version = version; Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); @@ -1262,7 +1266,7 @@ when server.ServerType.SupportsAutoConfigure() // a known non-Redis server: "version" is the vendor's own version, and the Redis API version // (if the server reports one at all) comes from INFO's redis_version var productVersion = vendorVariant == fromServer && !string.IsNullOrWhiteSpace(vendorVersion) - ? vendorVersion! : helloVersion; + ? vendorVersion! : hasHelloVersion ? helloVersion.ReadString() : null; if (!string.IsNullOrWhiteSpace(productVersion)) { server.SetProductVariant(fromServer, productVersion!); From f723b8d635f23ce4db1dc9fd1c17ddbfa91e00ba Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 09:42:27 +0100 Subject: [PATCH 3/7] Parse the HELLO "version" straight from UTF-8 Adds Format.TryParseVersion(ReadOnlySpan): net10.0 has a built-in UTF-8 Version.TryParse; elsewhere a Utf8Parser loop over the dotted components, so no char transcoding and, downlevel, no string. The HELLO handler uses it via the function-pointer TryParseScalar overload; a parity test keeps it in step with the char/string overloads. --- src/StackExchange.Redis/Format.cs | 51 +++++++++++++++++++ src/StackExchange.Redis/ResultProcessor.cs | 11 ++-- .../StackExchange.Redis.Tests/FormatTests.cs | 29 +++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/src/StackExchange.Redis/Format.cs b/src/StackExchange.Redis/Format.cs index a57bea0cda..655e415ce6 100644 --- a/src/StackExchange.Redis/Format.cs +++ b/src/StackExchange.Redis/Format.cs @@ -623,6 +623,57 @@ internal static bool TryParseVersion(ReadOnlySpan input, [NotNullWhen(true #endif } + /// + /// Parses a UTF-8 version (7.4.3, say) directly from the wire, allowing a major-only version like + /// the other overloads; net10.0 has a built-in UTF-8 Version.TryParse, + /// everything else parses the dotted components itself rather than transcoding to chars (and, downlevel, + /// to a string). + /// + internal static bool TryParseVersion(ReadOnlySpan input, [NotNullWhen(true)] out Version? version) + { +#if NET10_0_OR_GREATER + if (Version.TryParse(input, out version)) return true; + if (Utf8Parser.TryParse(input, out int i32, out int bytes, standardFormat: 'D') && bytes == input.Length && i32 >= 0) + { + version = new(i32, 0); + return true; + } + version = null; + return false; +#else + Span parts = stackalloc int[4]; + int count = 0; + while (true) + { + if (count == parts.Length + || !Utf8Parser.TryParse(input, out int i32, out int bytes, standardFormat: 'D') + || bytes == 0 || i32 < 0) + { + version = null; + return false; + } + parts[count++] = i32; + input = input.Slice(bytes); + if (input.IsEmpty) break; + if (input[0] != (byte)'.' || input.Length == 1) + { + // something other than a separator, or a trailing separator + version = null; + return false; + } + input = input.Slice(1); + } + version = count switch + { + 1 => new(parts[0], 0), + 2 => new(parts[0], parts[1]), + 3 => new(parts[0], parts[1], parts[2]), + _ => new(parts[0], parts[1], parts[2], parts[3]), + }; + return true; +#endif + } + internal static bool TryParseVersion(string? input, [NotNullWhen(true)] out Version? version) { if (input is not null) diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index db7e0a8f84..28815da7b8 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/src/StackExchange.Redis/ResultProcessor.cs @@ -1250,10 +1250,15 @@ when server.ServerType.SupportsAutoConfigure() if (serverIsRedis) { // "version" is the Redis API version; a "{vendor}_version" field, if any, identifies the product - if (hasHelloVersion && helloVersion.TryParseScalar(Format.TryParseVersion!, out Version version)) + Version? version; + unsafe { - server.Version = version; - Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); + // (the null test is for the compiler: the function-pointer form loses the NotNullWhen annotation) + if (hasHelloVersion && helloVersion.TryParseScalar(&Format.TryParseVersion, out version) && version is not null) + { + server.Version = version; + Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); + } } if (vendorVariant is { } fromField && !string.IsNullOrWhiteSpace(vendorVersion)) { diff --git a/tests/StackExchange.Redis.Tests/FormatTests.cs b/tests/StackExchange.Redis.Tests/FormatTests.cs index 1e41dcecfe..9b9e3b67c9 100644 --- a/tests/StackExchange.Redis.Tests/FormatTests.cs +++ b/tests/StackExchange.Redis.Tests/FormatTests.cs @@ -8,6 +8,35 @@ namespace StackExchange.Redis.Tests; public class FormatTests(ITestOutputHelper output) : TestBase(output) { + [Theory] + [InlineData("7", "7.0")] + [InlineData("7.4", "7.4")] + [InlineData("7.4.3", "7.4.3")] + [InlineData("9.1.2", "9.1.2")] + [InlineData("7.4.3.1", "7.4.3.1")] + [InlineData("255.255.65535.65535", "255.255.65535.65535")] + [InlineData("0.0", "0.0")] + [InlineData("", null)] + [InlineData(".", null)] + [InlineData("7.", null)] + [InlineData(".7", null)] + [InlineData("7..4", null)] + [InlineData("1.2.3.4.5", null)] + [InlineData("9.1.2-rc1", null)] + [InlineData("-1.2", null)] + [InlineData("a.b", null)] + [InlineData("1.2.3-preview4", null)] + public void TryParseVersionUtf8MatchesChars(string input, string? expected) + { + // the UTF-8 overload is what the HELLO handler uses; keep it in step with the char/string overloads + bool success = Format.TryParseVersion(Encoding.UTF8.GetBytes(input), out var fromBytes); + Assert.Equal(expected is not null, success); + Assert.Equal(expected, fromBytes?.ToString()); + + Assert.Equal(success, Format.TryParseVersion(input.AsSpan(), out var fromChars)); + Assert.Equal(fromBytes, fromChars); + } + public static IEnumerable EndpointData() { // note: the 3rd arg is for formatting; null means "expect the original string" From 4bdf6fe193ba9778533be45ea1d3fc5807a11ca2 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 09:43:44 +0100 Subject: [PATCH 4/7] Use IsScalar on the snapshot reader instead of a separate flag --- src/StackExchange.Redis/ResultProcessor.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/StackExchange.Redis/ResultProcessor.cs b/src/StackExchange.Redis/ResultProcessor.cs index 28815da7b8..cd69923123 100644 --- a/src/StackExchange.Redis/ResultProcessor.cs +++ b/src/StackExchange.Redis/ResultProcessor.cs @@ -1159,9 +1159,9 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes bool serverIsRedis = true; // absent means redis (older servers) ProductVariant? helloVariant = null, vendorVariant = null; string? vendorVersion = null; - // a snapshot of the "version" value, so we can read it as a Version or a string once we know which + // a snapshot of the "version" value (IsScalar when present), so we can read it as a Version or a + // string once we know which it is RespReader helloVersion = default; - bool hasHelloVersion = false; var iter = reader.AggregateChildren(); while (iter.MoveNext()) @@ -1192,7 +1192,6 @@ protected override bool SetResultCore(PhysicalConnection connection, Message mes break; case HelloField.Version: helloVersion = iter.Value; - hasHelloVersion = true; break; case HelloField.GarnetVersion: vendorVariant = ProductVariant.Garnet; @@ -1254,7 +1253,7 @@ when server.ServerType.SupportsAutoConfigure() unsafe { // (the null test is for the compiler: the function-pointer form loses the NotNullWhen annotation) - if (hasHelloVersion && helloVersion.TryParseScalar(&Format.TryParseVersion, out version) && version is not null) + if (helloVersion.IsScalar && helloVersion.TryParseScalar(&Format.TryParseVersion, out version) && version is not null) { server.Version = version; Log?.LogInformationAutoConfiguredHelloServerVersion(new(server), version); @@ -1271,7 +1270,7 @@ when server.ServerType.SupportsAutoConfigure() // a known non-Redis server: "version" is the vendor's own version, and the Redis API version // (if the server reports one at all) comes from INFO's redis_version var productVersion = vendorVariant == fromServer && !string.IsNullOrWhiteSpace(vendorVersion) - ? vendorVersion! : hasHelloVersion ? helloVersion.ReadString() : null; + ? vendorVersion! : helloVersion.IsScalar ? helloVersion.ReadString() : null; if (!string.IsNullOrWhiteSpace(productVersion)) { server.SetProductVariant(fromServer, productVersion!); From f2ce83659ecd3c5aefbe657b30e34f3f9dfad806 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 09:45:55 +0100 Subject: [PATCH 5/7] Match the HELLO "server" name case-insensitively --- src/StackExchange.Redis/Enums/ProductVariant.cs | 5 +++-- tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/StackExchange.Redis/Enums/ProductVariant.cs b/src/StackExchange.Redis/Enums/ProductVariant.cs index 6b6c046d20..ca85ee8c38 100644 --- a/src/StackExchange.Redis/Enums/ProductVariant.cs +++ b/src/StackExchange.Redis/Enums/ProductVariant.cs @@ -63,8 +63,9 @@ public enum ProductVariant internal static partial class ProductVariantMetadata { /// - /// Parses the server value reported by HELLO (redis, valkey, etc). + /// Parses the server value reported by HELLO (redis, valkey, etc); this is the vendor's + /// name rather than a protocol token, so the match is case-insensitive. /// - [AsciiHash] + [AsciiHash(CaseSensitive = false)] internal static partial bool TryParse(ReadOnlySpan value, out ProductVariant variant); } diff --git a/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs b/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs index b4b532687e..0ec3fc0c49 100644 --- a/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs +++ b/tests/StackExchange.Redis.Tests/ValkeyUnitTests.cs @@ -26,8 +26,8 @@ private sealed class InProcessValkeyLikeServer(ITestOutputHelper log) : InProces // see https://github.com/StackExchange/StackExchange.Redis/pull/3050 protected override string ServerModeKey => "server_mode"; - // Valkey reports its own version as HELLO "version" (see #3278) - protected override string HelloServerName => "valkey"; + // Valkey reports its own version as HELLO "version" (see #3278); the name match is case-insensitive + protected override string HelloServerName => "Valkey"; protected override string HelloVersionString => "8.1"; protected override void Info(StringBuilder sb, string section, Server.RedisClient? client = null) From 73a1ef6d5199903f73bead2e95d0b850341d3b31 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 10:12:06 +0100 Subject: [PATCH 6/7] Pin the raw Version parts (unspecified = -1) for the UTF-8 parser, and the gate normalisation System.Version treats "7.4" as (7, 4, -1, -1) and "7.4.0" as (7, 4, 0, -1); the UTF-8 overload must produce the same raw parts as the char/string overloads, and IsAtLeast/IsEqual must then treat the unspecified parts as zero. --- .../StackExchange.Redis.Tests/FormatTests.cs | 80 +++++++++++++------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/tests/StackExchange.Redis.Tests/FormatTests.cs b/tests/StackExchange.Redis.Tests/FormatTests.cs index 9b9e3b67c9..f85bce81c1 100644 --- a/tests/StackExchange.Redis.Tests/FormatTests.cs +++ b/tests/StackExchange.Redis.Tests/FormatTests.cs @@ -8,33 +8,67 @@ namespace StackExchange.Redis.Tests; public class FormatTests(ITestOutputHelper output) : TestBase(output) { + // System.Version is awkward: unspecified parts are -1, so "7.4" is (7, 4, -1, -1) and "7.4.0" is (7, 4, 0, -1), + // and the inbuilt operators order them differently; the UTF-8 overload (what the HELLO handler uses) must + // produce exactly the same raw parts as the char/string overloads, and the version gates must then treat the + // unspecified parts as zero (see VersionExtensions.IsAtLeast). [Theory] - [InlineData("7", "7.0")] - [InlineData("7.4", "7.4")] - [InlineData("7.4.3", "7.4.3")] - [InlineData("9.1.2", "9.1.2")] - [InlineData("7.4.3.1", "7.4.3.1")] - [InlineData("255.255.65535.65535", "255.255.65535.65535")] - [InlineData("0.0", "0.0")] - [InlineData("", null)] - [InlineData(".", null)] - [InlineData("7.", null)] - [InlineData(".7", null)] - [InlineData("7..4", null)] - [InlineData("1.2.3.4.5", null)] - [InlineData("9.1.2-rc1", null)] - [InlineData("-1.2", null)] - [InlineData("a.b", null)] - [InlineData("1.2.3-preview4", null)] - public void TryParseVersionUtf8MatchesChars(string input, string? expected) + [InlineData("7", 7, 0, -1, -1)] // major-only is our extension; System.Version rejects it + [InlineData("7.4", 7, 4, -1, -1)] + [InlineData("7.4.0", 7, 4, 0, -1)] + [InlineData("7.4.3", 7, 4, 3, -1)] + [InlineData("7.4.3.0", 7, 4, 3, 0)] + [InlineData("9.1.2", 9, 1, 2, -1)] + [InlineData("7.4.3.1", 7, 4, 3, 1)] + [InlineData("255.255.65535.65535", 255, 255, 65535, 65535)] + [InlineData("0.0", 0, 0, -1, -1)] + [InlineData("007.040", 7, 40, -1, -1)] + [InlineData("", null, null, null, null)] + [InlineData(".", null, null, null, null)] + [InlineData("7.", null, null, null, null)] + [InlineData(".7", null, null, null, null)] + [InlineData("7..4", null, null, null, null)] + [InlineData("1.2.3.4.5", null, null, null, null)] + [InlineData("9.1.2-rc1", null, null, null, null)] + [InlineData("-1.2", null, null, null, null)] + [InlineData("a.b", null, null, null, null)] + [InlineData("1.2.3-preview4", null, null, null, null)] + public void TryParseVersionUtf8MatchesChars(string input, int? major, int? minor, int? build, int? revision) { - // the UTF-8 overload is what the HELLO handler uses; keep it in step with the char/string overloads bool success = Format.TryParseVersion(Encoding.UTF8.GetBytes(input), out var fromBytes); - Assert.Equal(expected is not null, success); - Assert.Equal(expected, fromBytes?.ToString()); - + Assert.Equal(major is not null, success); Assert.Equal(success, Format.TryParseVersion(input.AsSpan(), out var fromChars)); - Assert.Equal(fromBytes, fromChars); + Assert.Equal(success, Format.TryParseVersion(input, out var fromString)); + + if (success) + { + Assert.NotNull(fromBytes); + Assert.Equal((major, minor, build, revision), (fromBytes.Major, fromBytes.Minor, fromBytes.Build, fromBytes.Revision)); + Assert.Equal(fromBytes, fromChars); // Version.Equals compares all four raw parts, -1 included + Assert.Equal(fromBytes, fromString); + } + else + { + Assert.Null(fromBytes); + Assert.Null(fromChars); + Assert.Null(fromString); + } + } + + [Theory] + [InlineData("7.4", "7.4.0", true, true)] // the unspecified parts count as zero for the gates + [InlineData("7.4.0", "7.4", true, true)] + [InlineData("7.4", "7.4.0.0", true, true)] + [InlineData("7.4.3", "7.4", true, false)] + [InlineData("7.4", "7.4.3", false, false)] + [InlineData("8", "7.4.3", true, false)] + [InlineData("7.4.3", "8", false, false)] + public void ParsedVersionGatesIgnoreUnspecifiedParts(string server, string required, bool atLeast, bool equal) + { + Assert.True(Format.TryParseVersion(Encoding.UTF8.GetBytes(server), out var x)); + Assert.True(Format.TryParseVersion(Encoding.UTF8.GetBytes(required), out var y)); + Assert.Equal(atLeast, x.IsAtLeast(y)); + Assert.Equal(equal, x.IsEqual(y)); } public static IEnumerable EndpointData() From 705b2ca867d62d719ce2ad3541442a2b8c0210cb Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Fri, 16 Oct 2026 10:23:41 +0100 Subject: [PATCH 7/7] UTF-8 TryParseVersion: defer to the char overload for exact parity The goal is identical results to the pre-existing char path for identical text, whatever System.Version does with it (-1 for unspecified parts, whitespace and sign tolerance per part), not a tidier parser. net10.0 fast-paths the built-in UTF-8 Version.TryParse; anything it rejects, and every other TFM, transcodes and calls the char overload. On .NET Framework that still allocates a string, which is acceptable. The parity test now covers the whitespace/sign cases. --- src/StackExchange.Redis/Format.cs | 66 +++++++++---------- .../StackExchange.Redis.Tests/FormatTests.cs | 7 ++ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/StackExchange.Redis/Format.cs b/src/StackExchange.Redis/Format.cs index 655e415ce6..8907182dfc 100644 --- a/src/StackExchange.Redis/Format.cs +++ b/src/StackExchange.Redis/Format.cs @@ -624,54 +624,48 @@ internal static bool TryParseVersion(ReadOnlySpan input, [NotNullWhen(true } /// - /// Parses a UTF-8 version (7.4.3, say) directly from the wire, allowing a major-only version like - /// the other overloads; net10.0 has a built-in UTF-8 Version.TryParse, - /// everything else parses the dotted components itself rather than transcoding to chars (and, downlevel, - /// to a string). + /// Parses a UTF-8 version (7.4.3, say) directly from the wire. This must give exactly the same result as + /// the char overload for the same text (including 's -1 for unspecified parts, and + /// its tolerance of whitespace and signs), because callers used to go via that overload: net10.0 fast-paths + /// the built-in UTF-8 Version.TryParse; everything else transcodes and defers to the char overload + /// (which, on .NET Framework, allocates a string: that's fine). /// internal static bool TryParseVersion(ReadOnlySpan input, [NotNullWhen(true)] out Version? version) { #if NET10_0_OR_GREATER if (Version.TryParse(input, out version)) return true; - if (Utf8Parser.TryParse(input, out int i32, out int bytes, standardFormat: 'D') && bytes == input.Length && i32 >= 0) + // anything else (including our major-only extension) defers to the char overload, so the edge cases + // (whitespace, signs, etc) resolve identically +#endif + if (input.IsEmpty) { - version = new(i32, 0); - return true; + version = null; + return false; } - version = null; - return false; -#else - Span parts = stackalloc int[4]; - int count = 0; - while (true) + char[]? lease = null; + int maxChars = Encoding.UTF8.GetMaxCharCount(input.Length); + Span chars = maxChars <= 64 ? stackalloc char[64] : (lease = ArrayPool.Shared.Rent(maxChars)); + try { - if (count == parts.Length - || !Utf8Parser.TryParse(input, out int i32, out int bytes, standardFormat: 'D') - || bytes == 0 || i32 < 0) - { - version = null; - return false; - } - parts[count++] = i32; - input = input.Slice(bytes); - if (input.IsEmpty) break; - if (input[0] != (byte)'.' || input.Length == 1) + int len; +#if NET + len = Encoding.UTF8.GetChars(input, chars); +#else + unsafe { - // something other than a separator, or a trailing separator - version = null; - return false; + fixed (byte* bPtr = &MemoryMarshal.GetReference(input)) + fixed (char* cPtr = &MemoryMarshal.GetReference(chars)) + { + len = Encoding.UTF8.GetChars(bPtr, input.Length, cPtr, chars.Length); + } } - input = input.Slice(1); +#endif + return TryParseVersion(chars.Slice(0, len), out version); } - version = count switch + finally { - 1 => new(parts[0], 0), - 2 => new(parts[0], parts[1]), - 3 => new(parts[0], parts[1], parts[2]), - _ => new(parts[0], parts[1], parts[2], parts[3]), - }; - return true; -#endif + if (lease is not null) ArrayPool.Shared.Return(lease); + } } internal static bool TryParseVersion(string? input, [NotNullWhen(true)] out Version? version) diff --git a/tests/StackExchange.Redis.Tests/FormatTests.cs b/tests/StackExchange.Redis.Tests/FormatTests.cs index f85bce81c1..886ea794eb 100644 --- a/tests/StackExchange.Redis.Tests/FormatTests.cs +++ b/tests/StackExchange.Redis.Tests/FormatTests.cs @@ -23,6 +23,13 @@ public class FormatTests(ITestOutputHelper output) : TestBase(output) [InlineData("255.255.65535.65535", 255, 255, 65535, 65535)] [InlineData("0.0", 0, 0, -1, -1)] [InlineData("007.040", 7, 40, -1, -1)] + // System.Version uses int.TryParse per part, which tolerates whitespace and a sign; we inherit that, not fix it + [InlineData(" 7.4", 7, 4, -1, -1)] + [InlineData("7.4 ", 7, 4, -1, -1)] + [InlineData("7. 4", 7, 4, -1, -1)] + [InlineData("+7.4", 7, 4, -1, -1)] + [InlineData("7.+4", 7, 4, -1, -1)] + [InlineData(" 7 ", 7, 0, -1, -1)] [InlineData("", null, null, null, null)] [InlineData(".", null, null, null, null)] [InlineData("7.", null, null, null, null)]