diff --git a/src/StackExchange.Redis/Message.cs b/src/StackExchange.Redis/Message.cs index 17ded6c20..8b4470de2 100644 --- a/src/StackExchange.Redis/Message.cs +++ b/src/StackExchange.Redis/Message.cs @@ -7,6 +7,7 @@ using System.Text; using System.Threading; using Microsoft.Extensions.Logging; +using RESPite; using RESPite.Internal; using RESPite.Messages; using StackExchange.Redis.Profiling; @@ -171,6 +172,29 @@ internal void PrepareToResend(ServerEndPoint resendTo, bool isMoved) public RedisCommand Command => command; public virtual string CommandAndKey => Command.ToString(); + [AsciiHash(nameof(SubCommandMetadata))] + internal enum SubCommand + { + [AsciiHash("")] + Unknown = 0, + [AsciiHash("GETNAME")] + GetName, + [AsciiHash("ID")] + Id, + [AsciiHash("INFO")] + Info, + [AsciiHash("SETINFO")] + SetInfo, + [AsciiHash("SETNAME")] + SetName, + } + + protected virtual bool TryGetSubCommand(out SubCommand subCommand) + { + subCommand = SubCommand.Unknown; + return false; + } + /// /// Things with the potential to cause harm, or to reveal configuration information. /// @@ -180,6 +204,21 @@ public bool IsAdmin { switch (Command) { + case RedisCommand.CLIENT when TryGetSubCommand(out var subCommand): + switch (subCommand) + { + case SubCommand.GetName: + case SubCommand.SetName: + case SubCommand.Id: + case SubCommand.Info: + case SubCommand.SetInfo: + return false; + } + return true; + /* possible? reasonable? + case RedisCommand.CONFIG when TryGetSubCommand(out var subCommand): + // allow .Get? + */ case RedisCommand.BGREWRITEAOF: case RedisCommand.BGSAVE: case RedisCommand.CLIENT: @@ -691,7 +730,7 @@ internal bool ComputeResult(PhysicalConnection connection, ref RespReader reader } catch (Exception ex) { - connection.OnDetailLog($"{ex.GetType().Name}: {ex.Message}"); + connection?.OnDetailLog($"{ex.GetType().Name}: {ex.Message}"); ex.Data.Add("got", prefix.ToString()); connection?.BridgeCouldBeNull?.Multiplexer?.OnMessageFaulted(this, ex); box?.SetException(ex); @@ -2017,5 +2056,40 @@ private UnknownMessage() : base(0, CommandFlags.None, RedisCommand.UNKNOWN) { } } public void SetNoFlush() => Flags |= NoFlushFlag; + + internal static partial class SubCommandMetadata + { + [AsciiHash(CaseSensitive = false)] + internal static partial bool TryParse(ReadOnlySpan value, out SubCommand subCommand); + + [AsciiHash(CaseSensitive = false)] + internal static partial bool TryParse(ReadOnlySpan value, out SubCommand subCommand); + + internal static bool TryGetSubCommand(in RedisValue value, out SubCommand subCommand) + { + switch (value.Type) + { + case RedisValue.StorageType.ByteArray: + case RedisValue.StorageType.MemoryManager: + case RedisValue.StorageType.ShortBlob: + // all three contiguous byte-blob kinds expose their bytes directly + // (the discard here *must* be stack-local; that's the "Unsafe" in this API) + return TryParse(value.UnsafeRawSpan(out _), out subCommand); + case RedisValue.StorageType.String: + // char-backed: parse the chars directly, no UTF8 round-trip + return TryParse(value.RawString().AsSpan(), out subCommand); + case RedisValue.StorageType.Sequence when value.GetByteCount() <= BufferBytes: + // non-contiguous: normalize into a small stack buffer + // (sub-commands are short, so anything longer cannot match) + Span tmp = stackalloc byte[BufferBytes]; + var len = value.CopyTo(tmp); + return TryParse(tmp.Slice(0, len), out subCommand); + // numeric / null / unknown are never a sub-command (e.g. it is never `123`); + // if that ever changes, revisit + } + subCommand = SubCommand.Unknown; + return false; + } + } } } diff --git a/src/StackExchange.Redis/RedisDatabase.cs b/src/StackExchange.Redis/RedisDatabase.cs index 21518eaa7..96390ac1e 100644 --- a/src/StackExchange.Redis/RedisDatabase.cs +++ b/src/StackExchange.Redis/RedisDatabase.cs @@ -5613,6 +5613,24 @@ public override int GetHashSlot(ServerSelectionStrategy serverSelectionStrategy) return slot; } public override int ArgCount => _args.Count; + + protected override bool TryGetSubCommand(out SubCommand subCommand) + { + // the sub-command (if any) is the first argument after the command itself, + // e.g. CLIENT [GETNAME]; ad-hoc Execute args are boxed objects, so normalize + // the first one to a RedisValue before probing it against the known sub-commands + foreach (object arg in _args) + { + var value = RedisValue.TryParse(arg, out var valid); + if (valid) + { + return SubCommandMetadata.TryGetSubCommand(value, out subCommand); + } + break; // only the first argument is a sub-command candidate + } + subCommand = SubCommand.Unknown; + return false; + } } private sealed class ScriptEvalMessage : Message, IMultiMessage diff --git a/tests/StackExchange.Redis.Tests/LibraryNameSuffixAdminTests.cs b/tests/StackExchange.Redis.Tests/LibraryNameSuffixAdminTests.cs new file mode 100644 index 000000000..eafb99acb --- /dev/null +++ b/tests/StackExchange.Redis.Tests/LibraryNameSuffixAdminTests.cs @@ -0,0 +1,54 @@ +using System.Threading.Tasks; +using Xunit; + +namespace StackExchange.Redis.Tests; + +public class LibraryNameSuffixAdminTests(InProcServerFixture fixture) +{ + private ConfigurationOptions NoAdminConfig() + { + // start from the shared in-process server config, but explicitly *disable* admin mode + var options = fixture.Config.Clone(); + options.AllowAdmin = false; + Assert.False(options.AllowAdmin); + return options; + } + + [Fact] + public async Task AddLibraryNameSuffixWorksWithoutAdmin() + { + await using var conn = await ConnectionMultiplexer.ConnectAsync(NoAdminConfig()); + + // internally this fixes up connected servers via CLIENT SETINFO (best-effort); the + // CLIENT SETINFO sub-command is not admin; don't report it (telemetry, etc) + conn.AddLibraryNameSuffix("mysuffix"); + } + + [Fact] + public async Task ClientSubCommandsViaExecuteDoNotRequireAdmin() + { + await using var conn = await ConnectionMultiplexer.ConnectAsync(NoAdminConfig()); + var server = conn.GetServer(conn.GetEndPoints()[0]); + + // none of these CLIENT sub-commands are admin, so they must not trip the admin-mode guard + // even though AllowAdmin is disabled (regression: the ad-hoc ExecuteMessage previously did + // not expose its sub-command to Message.IsAdmin, so CLIENT was treated as wholesale-admin) + var id = server.Execute("CLIENT", "ID"); + Assert.True((long)id > 0); + + Assert.Equal("OK", (string?)server.Execute("CLIENT", "SETNAME", "roundtrip")); + Assert.Equal("roundtrip", (string?)server.Execute("CLIENT", "GETNAME")); + } + + [Fact] + public async Task AdminClientSubCommandsStillRequireAdmin() + { + await using var conn = await ConnectionMultiplexer.ConnectAsync(NoAdminConfig()); + var server = conn.GetServer(conn.GetEndPoints()[0]); + + // CLIENT LIST is a genuine admin sub-command (not in the allow-list), so it must still be + // blocked when AllowAdmin is disabled - the fix must not blanket-allow every CLIENT usage + var ex = Assert.Throws(() => server.Execute("CLIENT", "LIST")); + Assert.Contains("admin mode", ex.Message); + } +}