Read HELLO "server" before trusting "version" as the Redis API version

src/StackExchange.Redis/Enums/ProductVariant.cs

@@ -1,48 +1,71 @@
-// ReSharper disable once CheckNamespace
+using System;
+using RESPite;
+
+// ReSharper disable once CheckNamespace
 namespace StackExchange.Redis;
 
 /// <summary>
 /// Indicates the flavor of RESP-server being used, if known. Unknown variants will be reported as <see cref="Redis"/>.
 /// 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.
 /// </summary>
 public enum ProductVariant
 {
     /// <summary>
     /// The original Redis server. This is also the default value if the variant is unknown.
     /// </summary>
+    [AsciiHash("redis")]
     Redis,
 
     /// <summary>
     /// <a href="https://valkey.io/">Valkey</a> is a fork of open-source Redis associated with AWS.
     /// </summary>
+    [AsciiHash("valkey")]
     Valkey,
 
     /// <summary>
     /// <a href="https://microsoft.github.io/garnet/">Garnet</a> is a Redis-compatible server from Microsoft.
     /// </summary>
+    [AsciiHash("garnet")]
     Garnet,
 
     /// <summary>
     /// <a href="https://www.dragonflydb.io/">Dragonfly</a>
     /// </summary>
+    [AsciiHash("dragonfly")]
     Dragonfly,
 
     /// <summary>
     /// <a href="https://www.memurai.com/">Memurai</a>
     /// </summary>
+    [AsciiHash("memurai")]
     Memurai,
 
     /// <summary>
     /// <a href="https://redict.io/">Redict</a>
     /// </summary>
+    [AsciiHash("redict")]
     Redict,
 
     /// <summary>
     /// <a href="https://keydb.dev">KeyDB</a>
     /// </summary>
+    [AsciiHash("keydb")]
     KeyDB,
 
     // if you want to add another variant here, please open an issue with the details (variant name, INFO output, etc.)
 }
+
+/// <summary>
+/// Metadata and parsing methods for <see cref="ProductVariant"/>.
+/// </summary>
+internal static partial class ProductVariantMetadata
+{
+    /// <summary>
+    /// Parses the <c>server</c> value reported by <c>HELLO</c> (<c>redis</c>, <c>valkey</c>, etc); this is the vendor's
+    /// name rather than a protocol token, so the match is case-insensitive.
+    /// </summary>
+    [AsciiHash(CaseSensitive = false)]
+    internal static partial bool TryParse(ReadOnlySpan<byte> value, out ProductVariant variant);
+}

src/StackExchange.Redis/Format.cs

@@ -623,6 +623,51 @@ internal static bool TryParseVersion(ReadOnlySpan<char> input, [NotNullWhen(true
 #endif
         }
 
+        /// <summary>
+        /// Parses a UTF-8 version (<c>7.4.3</c>, say) directly from the wire. This must give exactly the same result as
+        /// the <c>char</c> overload for the same text (including <see cref="Version"/>'s -1 for unspecified parts, and
+        /// its tolerance of whitespace and signs), because callers used to go via that overload: <c>net10.0</c> fast-paths
+        /// the built-in UTF-8 <c>Version.TryParse</c>; everything else transcodes and defers to the <c>char</c> overload
+        /// (which, on .NET Framework, allocates a string: that's fine).
+        /// </summary>
+        internal static bool TryParseVersion(ReadOnlySpan<byte> input, [NotNullWhen(true)] out Version? version)
+        {
+#if NET10_0_OR_GREATER
+            if (Version.TryParse(input, out version)) return true;
+            // 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 = null;
+                return false;
+            }
+            char[]? lease = null;
+            int maxChars = Encoding.UTF8.GetMaxCharCount(input.Length);
+            Span<char> chars = maxChars <= 64 ? stackalloc char[64] : (lease = ArrayPool<char>.Shared.Rent(maxChars));
+            try
+            {
+                int len;
+#if NET
+                len = Encoding.UTF8.GetChars(input, chars);
+#else
+                unsafe
+                {
+                    fixed (byte* bPtr = &MemoryMarshal.GetReference(input))
+                    fixed (char* cPtr = &MemoryMarshal.GetReference(chars))
+                    {
+                        len = Encoding.UTF8.GetChars(bPtr, input.Length, cPtr, chars.Length);
+                    }
+                }
+#endif
+                return TryParseVersion(chars.Slice(0, len), out version);
+            }
+            finally
+            {
+                if (lease is not null) ArrayPool<char>.Shared.Return(lease);
+            }
+        }
+
         internal static bool TryParseVersion(string? input, [NotNullWhen(true)] out Version? version)
         {
             if (input is not null)

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,
 
     /// <summary>
-    /// Redis server version.
+    /// The server product: <c>redis</c> for Redis and for servers that present as Redis (Garnet); Valkey reports <c>valkey</c>.
+    /// </summary>
+    [AsciiHash("server")]
+    Server,
+
+    /// <summary>
+    /// Server version. This is only the Redis API version when <see cref="Server"/> is <c>redis</c>; Valkey reports its own
+    /// version here (<c>9.1.2</c>, say, while <c>INFO</c> still gives <c>redis_version:7.2.4</c>).
     /// </summary>
     [AsciiHash("version")]
     Version,
 
+    /// <summary>
+    /// Garnet's own version; Garnet reports <c>server</c> as <c>redis</c> and <see cref="Version"/> as the Redis API version.
+    /// </summary>
+    [AsciiHash("garnet_version")]
+    GarnetVersion,
+
+    /// <summary>
+    /// Valkey's own version, if reported alongside <see cref="Server"/>.
+    /// </summary>
+    [AsciiHash("valkey_version")]
+    ValkeyVersion,
+
+    /// <summary>
+    /// Dragonfly's own version, if reported.
+    /// </summary>
+    [AsciiHash("dragonfly_version")]
+    DragonflyVersion,
+
+    /// <summary>
+    /// Memurai's own version, if reported.
+    /// </summary>
+    [AsciiHash("memurai_version")]
+    MemuraiVersion,
+
+    /// <summary>
+    /// Redict's own version, if reported.
+    /// </summary>
+    [AsciiHash("redict_version")]
+    RedictVersion,
+
     /// <summary>
     /// Protocol version (2 or 3).
     /// </summary>

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,

src/StackExchange.Redis/ResultProcessor.cs

@@ -1152,6 +1152,17 @@ 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? vendorVersion = null;
+                    // 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;
+
                     var iter = reader.AggregateChildren();
                     while (iter.MoveNext())
                     {
@@ -1167,9 +1178,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;
+                                    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 +1246,39 @@ when server.ServerType.SupportsAutoConfigure()
                         }
                     }
 
+                    if (serverIsRedis)
+                    {
+                        // "version" is the Redis API version; a "{vendor}_version" field, if any, identifies the product
+                        Version? version;
+                        unsafe
+                        {
+                            // (the null test is for the compiler: the function-pointer form loses the NotNullWhen annotation)
+                            if (helloVersion.IsScalar && 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))
+                        {
+                            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.IsScalar ? helloVersion.ReadString() : null;
+                        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;
                 }

tests/StackExchange.Redis.Tests/FormatTests.cs

@@ -8,6 +8,76 @@ 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, -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)]
+    // 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)]
+    [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)
+    {
+        bool success = Format.TryParseVersion(Encoding.UTF8.GetBytes(input), out var fromBytes);
+        Assert.Equal(major is not null, success);
+        Assert.Equal(success, Format.TryParseVersion(input.AsSpan(), out var 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<object?[]> EndpointData()
     {
         // note: the 3rd arg is for formatting; null means "expect the original string"