From 5324d70311237e0b91bf0924b001ef174af4475b Mon Sep 17 00:00:00 2001 From: mgravell Date: Mon, 3 Aug 2026 14:58:45 +0100 Subject: [PATCH 1/6] PhysicalConnection transport mode: push-feed IO via Tunnel.ConnectTransportAsync (SER009) When a Tunnel yields a DuplexTransport, PhysicalConnection now runs with no socket, no Stream, no SslStream and no reader thread: the transport is acquired BEFORE socket creation (the widest form of the existing connectTo=null no-socket pattern), outbound rides the existing _output seam through a TransportWriter : BufferedStreamWriter adapter (the transport IS an IBufferWriter with explicit Flush, so every call site is untouched), and inbound is PUSH -- TransportFeed commits transport-owned spans into the same CycleBuffer + CommitAndParseFrames the pull loops use, on the transport's threads. One copy, zero thread hops, no read syscall surface on this side. TLS contract: a transport tunnel owns TLS; config.Ssl=true alongside one throws NotSupportedException rather than silently double-encrypting or silently skipping. Teardown disposes the transport in Shutdown alongside socket/output, and Dispose treats an owned transport like an owned socket. Gated end-to-end from the SocketSet side (bench/tunnel-selftest, mux + mux-tls cells): a real ConnectionMultiplexer over SocketSetTunnel against Garnet -- connect (HELLO handshake included), PING, SET/GET, 500-op pipelined burst, plaintext AND transport-terminated TLS, ALL PASS first run. The 129-test writer/round-trip/in-proc battery still passes (classic paths untouched). --- .../PhysicalConnection.Read.cs | 7 + .../PhysicalConnection.Transport.cs | 141 ++++++++++++++++++ src/StackExchange.Redis/PhysicalConnection.cs | 39 ++++- 3 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 src/StackExchange.Redis/PhysicalConnection.Transport.cs diff --git a/src/StackExchange.Redis/PhysicalConnection.Read.cs b/src/StackExchange.Redis/PhysicalConnection.Read.cs index 9052f6494..4349c60ad 100644 --- a/src/StackExchange.Redis/PhysicalConnection.Read.cs +++ b/src/StackExchange.Redis/PhysicalConnection.Read.cs @@ -29,6 +29,13 @@ internal static PhysicalConnection Dummy(Stream stream, BufferedStreamWriter.Wri internal void StartReading(CancellationToken cancellation = default) { + if (_transport is { } transport) + { + // push mode: no reader thread/Task -- the transport delivers into the parser directly + StartTransportReading(transport); + return; + } + if (cancellation.CanBeCanceled && cancellation != InputCancel) { cancellation.ThrowIfCancellationRequested(); diff --git a/src/StackExchange.Redis/PhysicalConnection.Transport.cs b/src/StackExchange.Redis/PhysicalConnection.Transport.cs new file mode 100644 index 000000000..f1e04a153 --- /dev/null +++ b/src/StackExchange.Redis/PhysicalConnection.Transport.cs @@ -0,0 +1,141 @@ +#pragma warning disable SER009 // experimental transport contract: this file IS the consumer side of it + +using System; +using System.Buffers; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using RESPite.Buffers; +using RESPite.Streams; +using RESPite.Transports; + +namespace StackExchange.Redis +{ + internal sealed partial class PhysicalConnection + { + // TRANSPORT MODE: when Tunnel.ConnectTransportAsync yields a DuplexTransport, there is no + // socket, no Stream and no SslStream — the tunnel owns connect and TLS end-to-end. Outbound + // rides the existing _output seam via an adapter (the transport IS an IBufferWriter with an + // explicit Flush, which is the BufferedStreamWriter contract minus the Stream); inbound is + // PUSH — the transport calls into the same CommitAndParseFrames the pull loops use, on the + // transport's own threads, so there is no reader thread, no read Task and no read syscall + // surface at all on this side. + private DuplexTransport? _transport; + + internal bool HasTransport => _transport is not null; + + private void InitTransportOutput(DuplexTransport transport) + { + _output = new TransportWriter(transport, OutputCancel); + + // Same reasoning as InitOutput: nothing awaits WriteComplete in production; observe it so + // a teardown-time write fault never becomes an UnobservedTaskException. + _output.WriteComplete.RedisFireAndForget(); +#if DEBUG + if (BridgeCouldBeNull?.Multiplexer?.RawConfig?.OutputLog is { } log) + { + _output.DebugSetLog(log); + } +#endif + } + + private void StartTransportReading(DuplexTransport transport) + { + _readStatus = ReadStatus.Init; + _readState = default; + _readBuffer = CycleBuffer.Create(pool: ReaderBufferPool); + transport.Start(new TransportFeed(this)); + } + + /// Outbound half: the transport is already the writer; this adapter exists only to + /// satisfy the seam every call site already uses. Staging + /// forwards straight through; hands the batch to the transport; the write + /// fault/completion surface is a simple once-only task. + private sealed class TransportWriter(DuplexTransport transport, CancellationToken cancellationToken) + : BufferedStreamWriter(Stream.Null, cancellationToken) + { + private readonly TaskCompletionSource _done = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public override Task WriteComplete => _done.Task; + + public override Memory GetMemory(int sizeHint = 0) => transport.GetMemory(sizeHint); + + public override Span GetSpan(int sizeHint = 0) => transport.GetSpan(sizeHint); + + public override void Advance(int count) + { + transport.Advance(count); + OnWritten(count); + } + + // A false return means the transport is closed; that failure surfaces via the receiver's + // OnClosed (which records the connection failure), so it is not duplicated here. + public override void Flush() => transport.Flush(); + + public override void Complete(Exception? exception = null) + { + try { transport.Flush(); } + catch { } + if (exception is null) + { + _done.TrySetResult(true); + } + else + { + _done.TrySetException(exception); + } + } + } + + /// Inbound half: transport-owned spans arrive on the transport's schedule and are + /// committed into the same + + /// machinery the pull loops use — one copy, zero thread hops, no reader loop. + private sealed class TransportFeed(PhysicalConnection connection) : TransportReceiver + { + public override bool OnReceived(ReadOnlySpan payload) + { + var conn = connection; + try + { + while (!payload.IsEmpty) + { + var space = conn._readBuffer.GetUncommittedMemory().Span; + var take = Math.Min(space.Length, payload.Length); + payload.Slice(0, take).CopyTo(space); + payload = payload.Slice(take); + + conn._readStatus = ReadStatus.TryParseResult; + if (!conn.CommitAndParseFrames(take)) + { + return false; + } + } + conn.UpdateLastReadTime(); + return !conn.ForceReconnect; + } + catch (Exception ex) + { + conn._readStatus = ReadStatus.Faulted; + conn.RecordConnectionFailed(ConnectionFailureType.InternalFailure, ex); + return false; + } + } + + public override void OnClosed(Exception? fault) + { + var conn = connection; + conn._readStatus = ReadStatus.RanToCompletion; + conn._readBuffer.Release(); + conn._readBuffer = default; + if (fault is null) + { + conn.RecordConnectionFailed(ConnectionFailureType.SocketClosed); + } + else + { + conn.RecordConnectionFailed(ConnectionFailureType.InternalFailure, fault); + } + } + } + } +} diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index e44f48f04..2f5321580 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -180,7 +180,21 @@ internal async Task BeginConnectAsync(ILogger? log) var connectTo = endpoint; if (tunnel is not null) { - connectTo = await tunnel.GetSocketConnectEndpointAsync(endpoint, CancellationToken.None).ForAwait(); +#pragma warning disable SER009 // experimental transport contract: this is the consuming call site + // A transport tunnel replaces the socket outright (the widest form of the existing + // connectTo=null no-socket pattern): connect and TLS belong to the tunnel, and the + // stream/SslStream machinery below never runs. + var transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, CancellationToken.None).ForAwait(); +#pragma warning restore SER009 + if (transport is not null) + { + _transport = transport; + connectTo = null; + } + else + { + connectTo = await tunnel.GetSocketConnectEndpointAsync(endpoint, CancellationToken.None).ForAwait(); + } } if (connectTo is not null) { @@ -314,6 +328,7 @@ internal void Shutdown(ConnectionFailureType failureType = ConnectionFailureType { var output = Interlocked.Exchange(ref _output, null); // compare to the critical read var socket = Interlocked.Exchange(ref _socket, null); + var transport = Interlocked.Exchange(ref _transport, null); if (output != null) { @@ -328,11 +343,16 @@ internal void Shutdown(ConnectionFailureType failureType = ConnectionFailureType try { socket.Close(); } catch { } try { socket.Dispose(); } catch { } } + + if (transport != null) + { + try { transport.DisposeAsync().AsTask().RedisFireAndForget(); } catch { } + } } public void Dispose() { - bool markDisposed = VolatileSocket != null; + bool markDisposed = VolatileSocket != null || _transport != null; Shutdown(); if (markDisposed) { @@ -1014,6 +1034,21 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log) // TLS: [Socket]<==[NetworkStream]<==[SslStream]<==[StreamConnection:IDuplexPipe] var config = bridge.Multiplexer.RawConfig; + if (_transport is { } transport) + { + // Transport mode: no socket, no stream, no SslStream. TLS is the tunnel's job; a + // config that ALSO asks for stream-level TLS is a contradiction, and it fails loud + // rather than silently double-encrypting or silently skipping. + if (config.Ssl) + { + throw new NotSupportedException("With a transport-returning Tunnel, TLS belongs to the tunnel; do not also set Ssl=true."); + } + InitTransportOutput(transport); + log?.LogInformationConnected(bridge.Name); + await bridge.OnConnectedAsync(this, log).ForAwait(); + return true; + } + var tunnel = config.Tunnel; if (tunnel is not null) { From 4c5a8afae51252a7d172ff6cadc00c2665ec0230 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 13 Aug 2026 13:10:26 +0100 Subject: [PATCH 2/6] Transport mode TLS: the transport sees the intent, and gets to disagree (SER009) A transport tunnel owns connect and TLS end-to-end, so two things were wrong: it could not SEE the TLS configuration (the same engine may be run with or without TLS - that cannot be assumed either way), and ConnectedAsync prejudiced against config.Ssl by throwing outright. Ssl=true is entirely valid in transport mode; the real question is whether the TRANSPORT disagrees. ConnectTransportAsync now takes a TlsOptions: a readonly struct wrapping the ConfigurationOptions, whose accessors proxy through to just the TLS-relevant settings (IsEnabled, SslHost, SslProtocols, CheckCertificateRevocation, the certificate validation/selection callbacks including their ambient env-var fallbacks, GetSslClientAuthenticationOptions on net6+, and ResolveHost, which applies the same SslHost-or-endpoint-host fallback our own TLS path uses). Zero-alloc, nothing snapshotted, and it cannot be used to reach - or change - anything else about the configuration. Backend-specific settings remain the tunnel constructor's business. DuplexTransport gains IsEncrypted (virtual, default false: the safe answer, so a TLS-terminating transport that forgets to override is refused rather than trusted). ConnectedAsync now fails only on actual disagreement - config.Ssl with a transport reporting plaintext - recorded as AuthenticationFailure like the SslStream path, not the generic InternalFailure catch. The converse (encrypted when the config did not demand it) is accepted and logged: more secure than asked for is fine. On the "actively assert ssl.IsEncrypted after the handshake" advice for the pre-existing SslStream path: it is moot, and the comment at the site now says so with both source links. On every TFM we target, SslStream.IsEncrypted (and IsSigned) is an alias for IsAuthenticated - "handshake completed, no exception" - which a successful await already proved; it says nothing about the bytes on the wire. What forbids plaintext there is the EncryptionPolicy.RequireEncryption we already pass. Rather than a tautological assert that implies a guarantee it does not make, net6+ now logs the NegotiatedCipherSuite alongside SslProtocol, so a surprising negotiation is visible. Two rough edges found while in here: LoggingTunnel never forwarded ConnectTransportAsync to its tail, so wrapping a transport tunnel silently bypassed the transport and fell back to a socket. It now forwards and wraps via a new virtual Log(DuplexTransport, ...) hook - LoggingDuplexTransport captures into the same .in/.out pair the stream path produces, so ReplayAsync/ValidateAsync read both unchanged. Outbound is captured in Advance (the last point the bytes are still ours; hence GetSpan routes through our GetMemory), inbound before the consumer sees it, and IsEncrypted forwards to the inner transport. Since our constructor clears Ssl from the live options so captures are plaintext, the tail is handed a Clone with the original intent restored - otherwise a logged transport would have quietly connected in the clear. The default hook returns the transport unchanged: a custom subclass that only implements stream logging connects unlogged rather than failing the connection. TransportFeed did not implement OnBatchEnd, which the contract asks for explicitly. TransportWriter now tracks bytes advanced since the last flush (approximate in the safe direction: a redundant flush attempt, never a missed one) and the callback flushes through PhysicalBridge.TryFlushStagedWrites, which takes the write lock with TryTakeInstant or does nothing at all. The lock is what makes staging exclusive, so an unguarded flush could hand a half-written frame to a transport mid-Advance; instant-or-nothing also declines safely if the burst is being pumped inline on a thread that already holds it. The only thing that can be staged there is an inbound-triggered write that did not demand a flush, so the common path is one volatile read. Solution builds clean on all TFMs (warnings-as-errors). net10.0 suite: 5666 pass / 8 fail, against 6 failures on a HEAD baseline in a temporary worktree - a disjoint set of names, all connect-timeout shaped, from running against a trimmed local server topology; each of the 8 passes in isolation. Note that transport mode itself has no test coverage in this repo at all, so the above is compile-and-review verified, not exercised. --- src/RESPite/PublicAPI/PublicAPI.Unshipped.txt | 1 + src/RESPite/Transports/DuplexTransport.cs | 9 + .../Configuration/LoggingTunnel.cs | 165 +++++++++++++++++- .../Configuration/TlsOptions.cs | 91 ++++++++++ .../Configuration/Tunnel.cs | 18 +- src/StackExchange.Redis/LoggerExtensions.cs | 14 ++ src/StackExchange.Redis/PhysicalBridge.cs | 30 ++++ .../PhysicalConnection.Transport.cs | 31 +++- src/StackExchange.Redis/PhysicalConnection.cs | 40 ++++- .../PublicAPI/PublicAPI.Unshipped.txt | 12 +- .../PublicAPI/net6.0/PublicAPI.Unshipped.txt | 1 + 11 files changed, 395 insertions(+), 17 deletions(-) create mode 100644 src/StackExchange.Redis/Configuration/TlsOptions.cs diff --git a/src/RESPite/PublicAPI/PublicAPI.Unshipped.txt b/src/RESPite/PublicAPI/PublicAPI.Unshipped.txt index 3b10f3da4..4c74fea9a 100644 --- a/src/RESPite/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/RESPite/PublicAPI/PublicAPI.Unshipped.txt @@ -7,6 +7,7 @@ [SER009]abstract RESPite.Transports.DuplexTransport.GetMemory(int sizeHint = 0) -> System.Memory [SER009]virtual RESPite.Transports.DuplexTransport.GetSpan(int sizeHint = 0) -> System.Span [SER009]abstract RESPite.Transports.DuplexTransport.Start(RESPite.Transports.TransportReceiver! receiver) -> void +[SER009]virtual RESPite.Transports.DuplexTransport.IsEncrypted.get -> bool [SER009]RESPite.Transports.TransportReceiver [SER009]RESPite.Transports.TransportReceiver.TransportReceiver() -> void [SER009]abstract RESPite.Transports.TransportReceiver.OnReceived(System.ReadOnlySpan payload) -> bool diff --git a/src/RESPite/Transports/DuplexTransport.cs b/src/RESPite/Transports/DuplexTransport.cs index 2701d60b6..68e7996f2 100644 --- a/src/RESPite/Transports/DuplexTransport.cs +++ b/src/RESPite/Transports/DuplexTransport.cs @@ -47,6 +47,15 @@ public abstract class DuplexTransport : IBufferWriter, IAsyncDisposable /// delivery runs on the transport's schedule and threads. public abstract void Start(TransportReceiver receiver); + /// + /// Whether this transport's bytes are encrypted on the wire. Since the transport owns connect and + /// TLS end-to-end, this is the only thing a consumer can ask; it is the transport's assertion, and + /// the default is the safe answer (no). A transport that terminates TLS itself MUST override this, + /// otherwise a configuration that demands TLS will refuse to use it. + /// + /// Valid once the transport is connected; consumers check it before first use. + public virtual bool IsEncrypted => false; + public abstract ValueTask DisposeAsync(); } diff --git a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs index 42a85b710..32d5567cc 100644 --- a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs +++ b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs @@ -1,4 +1,6 @@ -using System; +#pragma warning disable SER009 // experimental transport contract: this file logs the consumer side of it + +using System; using System.Buffers; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -12,6 +14,7 @@ using RESPite; using RESPite.Buffers; using RESPite.Messages; +using RESPite.Transports; using static StackExchange.Redis.PhysicalConnection; namespace StackExchange.Redis.Configuration; @@ -315,6 +318,18 @@ internal DirectoryLoggingTunnel(string path, ConfigurationOptions? options = nul } protected override Stream Log(Stream stream, EndPoint endpoint, ConnectionType connectionType) + { + CreateLogFiles(endpoint, connectionType, out var reads, out var writes); + return new LoggingDuplexStream(stream, reads, writes); + } + + protected override DuplexTransport Log(DuplexTransport transport, EndPoint endpoint, ConnectionType connectionType) + { + CreateLogFiles(endpoint, connectionType, out var reads, out var writes); + return new LoggingDuplexTransport(transport, reads, writes); + } + + private void CreateLogFiles(EndPoint endpoint, ConnectionType connectionType, out Stream reads, out Stream writes) { int index = Interlocked.Increment(ref _nextIndex); var name = $"{Format.ToString(endpoint)} {connectionType} {index}.tmp"; @@ -323,9 +338,8 @@ protected override Stream Log(Stream stream, EndPoint endpoint, ConnectionType c name = name.Replace(c, ' '); } name = Path.Combine(path, name); - var reads = File.Create(Path.ChangeExtension(name, ".in")); - var writes = File.Create(Path.ChangeExtension(name, ".out")); - return new LoggingDuplexStream(stream, reads, writes); + reads = File.Create(Path.ChangeExtension(name, ".in")); + writes = File.Create(Path.ChangeExtension(name, ".out")); } private static readonly char[] InvalidChars = Path.GetInvalidFileNameChars(); @@ -352,6 +366,35 @@ protected override Stream Log(Stream stream, EndPoint endpoint, ConnectionType c /// protected abstract Stream Log(Stream stream, EndPoint endpoint, ConnectionType connectionType); + /// + public override async ValueTask ConnectTransportAsync(EndPoint endpoint, ConnectionType connectionType, TlsOptions tls, CancellationToken cancellationToken) + { + if (_tail is null) return null; // nothing to wrap; the socket path applies + + // in transport mode the tail owns connect *and* TLS, so it needs the original TLS intent - which + // our constructor deliberately cleared from the live options, since when we do the handshake + // ourselves the library must not also wrap the connection. Note that what we log here is + // therefore the plaintext RESP either way: we sit above the tail's own encryption. + if (_ssl && !tls.IsEnabled) + { + var forTail = _options.Clone(); + forTail.Ssl = true; + tls = new TlsOptions(forTail); + } + + var transport = await _tail.ConnectTransportAsync(endpoint, connectionType, tls, cancellationToken).ForAwait(); + return transport is null ? null : Log(transport, endpoint, connectionType); + } + + /// + /// Perform logging on the provided transport (push-mode transports, i.e. tunnels that supply an + /// entire rather than a socket or ). + /// + /// The default implementation returns the transport unchanged, i.e. connects but does not + /// log; override it (typically returning a ) to capture traffic + /// in transport mode. + protected virtual DuplexTransport Log(DuplexTransport transport, EndPoint endpoint, ConnectionType connectionType) => transport; + /// public override ValueTask BeforeSocketConnectAsync(EndPoint endPoint, ConnectionType connectionType, Socket? socket, CancellationToken cancellationToken) { @@ -486,6 +529,120 @@ public static string DefaultFormatResponse(RedisResult value) } #pragma warning disable CS1591 // Missing XML comment for publicly visible type or member + /// + /// Captures the traffic of a push-mode into a pair of streams, in the + /// same format produces, so the same replay/validate tooling reads + /// both. Outbound bytes are captured at (the last point at which they are still + /// ours to read); inbound bytes are captured as they are delivered, before the consumer sees them. + /// + protected sealed class LoggingDuplexTransport : DuplexTransport + { + private readonly DuplexTransport _inner; + private readonly Stream _reads, _writes; + private Memory _pending; // the buffer most recently handed out, so Advance can log from it + private volatile bool _logsClosed; + + internal LoggingDuplexTransport(DuplexTransport inner, Stream reads, Stream writes) + { + _inner = inner; + _reads = reads; + _writes = writes; + } + + // the wrapper adds capture, not encryption; whether the bytes are encrypted is the inner + // transport's assertion to make + public override bool IsEncrypted => _inner.IsEncrypted; + + public override Memory GetMemory(int sizeHint = 0) => _pending = _inner.GetMemory(sizeHint); + + // deliberately NOT delegating to _inner.GetSpan: we need the memory to still be reachable at + // Advance, which is the only point where we learn how much of it was actually written + public override Span GetSpan(int sizeHint = 0) => GetMemory(sizeHint).Span; + + public override void Advance(int count) + { + if (count > 0 && !_logsClosed) + { + // capture *before* advancing; from Advance onwards the bytes belong to the transport, + // which is free to send and recycle them + WriteLog(_writes, _pending.Span.Slice(0, count)); + } + _pending = default; + _inner.Advance(count); + } + + public override bool Flush() + { + // capture first, so the log is never behind the wire + if (!_logsClosed) _writes.Flush(); + return _inner.Flush(); + } + + public override void Start(TransportReceiver receiver) => _inner.Start(new LoggingReceiver(this, receiver)); + + public override async ValueTask DisposeAsync() + { + try + { + await _inner.DisposeAsync().ForAwait(); + } + finally + { + CloseLogs(); + } + } + + private void CloseLogs() + { + if (_logsClosed) return; + _logsClosed = true; + try { _reads.Flush(); } catch { } + try { _reads.Dispose(); } catch { } + try { _writes.Flush(); } catch { } + try { _writes.Dispose(); } catch { } + } + + private static void WriteLog(Stream target, ReadOnlySpan payload) + { +#if NET + target.Write(payload); +#else + var lease = ArrayPool.Shared.Rent(payload.Length); + payload.CopyTo(lease); + target.Write(lease, 0, payload.Length); + ArrayPool.Shared.Return(lease); +#endif + } + + private sealed class LoggingReceiver(LoggingDuplexTransport parent, TransportReceiver tail) : TransportReceiver + { + public override bool OnReceived(ReadOnlySpan payload) + { + // capture before handing on, so a capture exists even if the consumer faults on it + if (!parent._logsClosed) + { + WriteLog(parent._reads, payload); + parent._reads.Flush(); + } + return tail.OnReceived(payload); + } + + public override void OnBatchEnd() => tail.OnBatchEnd(); + + public override void OnClosed(Exception? fault) + { + try + { + tail.OnClosed(fault); + } + finally + { + parent.CloseLogs(); + } + } + } + } + protected sealed class LoggingDuplexStream : Stream { private readonly Stream _inner, _reads, _writes; diff --git a/src/StackExchange.Redis/Configuration/TlsOptions.cs b/src/StackExchange.Redis/Configuration/TlsOptions.cs new file mode 100644 index 000000000..81a1bda1b --- /dev/null +++ b/src/StackExchange.Redis/Configuration/TlsOptions.cs @@ -0,0 +1,91 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Net; +using System.Net.Security; +using System.Security.Authentication; + +namespace StackExchange.Redis.Configuration; + +/// +/// A read-only view over just the TLS-relevant parts of a , for +/// transports that own TLS themselves (see ): a transport +/// cannot honour an intent it cannot see, but it has no business with the rest of the configuration. +/// +/// +/// This wraps the configuration rather than copying it: the accessors proxy through, so the cost of +/// passing this around is one reference, and nothing here can mutate the configuration or reach +/// anything beyond TLS. A default instance reports "no TLS". +/// +[Experimental(RESPite.Experiments.Transport, UrlFormat = RESPite.Experiments.UrlFormat)] +public readonly struct TlsOptions +{ + private readonly ConfigurationOptions? _options; + + /// + /// Create a view over the TLS settings of the supplied configuration. + /// + public TlsOptions(ConfigurationOptions options) + => _options = options ?? throw new ArgumentNullException(nameof(options)); + + /// + /// Whether TLS is required for this connection; corresponds to . + /// A transport that returns false from + /// when this is set will not be used. + /// + public bool IsEnabled => _options?.Ssl ?? false; + + /// + /// The configured TLS host name, used for SNI and certificate validation; corresponds to + /// . Usually prefer , which + /// applies the same endpoint fallback the library's own TLS path uses. + /// + public string? SslHost => _options?.SslHost; + + /// + /// The permitted TLS protocol versions, or null for the platform default; corresponds to + /// . + /// + public SslProtocols? SslProtocols => _options?.SslProtocols; + + /// + /// Whether the certificate revocation list should be checked during authentication; corresponds to + /// . + /// + public bool CheckCertificateRevocation => _options?.CheckCertificateRevocation ?? false; + + /// + /// The callback that validates the server certificate, or null for the platform default. + /// + /// This includes the ambient environment fallback (SERedis_IssuerCertPath), so a + /// transport that uses this behaves as the library's own TLS path does. + public RemoteCertificateValidationCallback? CertificateValidationCallback + => _options is null ? null : _options.CertificateValidationCallback ?? PhysicalConnection.GetAmbientIssuerCertificateCallback(); + + /// + /// The callback that selects the client certificate, or null for the platform default. + /// + /// This includes the ambient environment fallback (SERedis_ClientCertPfxPath etc), so + /// a transport that uses this behaves as the library's own TLS path does. + public LocalCertificateSelectionCallback? CertificateSelectionCallback + => _options is null ? null : _options.CertificateSelectionCallback ?? PhysicalConnection.GetAmbientClientCertificateCallback(); + +#if NET + /// + /// The caller-supplied authentication options for the given host, if any; corresponds to + /// . When this returns non-null it + /// supersedes the individual values here, exactly as it does on the library's own TLS path. + /// + public SslClientAuthenticationOptions? GetSslClientAuthenticationOptions(string host) + => _options?.SslClientAuthenticationOptions?.Invoke(host); +#endif + + /// + /// The TLS host name to use for the given endpoint: the configured if there is + /// one, otherwise the host portion of the endpoint - which is what the library's own TLS path does. + /// + public string ResolveHost(EndPoint endpoint) + { + var host = SslHost; + return host.IsNullOrWhiteSpace() ? Format.ToStringHostOnly(endpoint) : host!; + } +} diff --git a/src/StackExchange.Redis/Configuration/Tunnel.cs b/src/StackExchange.Redis/Configuration/Tunnel.cs index 72cdbc4d2..85438a54f 100644 --- a/src/StackExchange.Redis/Configuration/Tunnel.cs +++ b/src/StackExchange.Redis/Configuration/Tunnel.cs @@ -37,14 +37,28 @@ public abstract class Tunnel public virtual ValueTask BeforeAuthenticateAsync(EndPoint endpoint, ConnectionType connectionType, Socket? socket, CancellationToken cancellationToken) => default; /// - /// Optionally supply the ENTIRE transport for this connection — the same hijack as + /// Optionally supply the ENTIRE transport for this connection - the same hijack as /// one level deeper: instead of yielding a /// over a socket the library owns, yield a /// the tunnel owns, and no socket is created at /// all. Return null (the default for every existing tunnel) for the standard socket path. /// + /// The logical endpoint being connected to. + /// Whether this is the interactive or subscription connection. + /// + /// The TLS intent this transport must honour: since the transport owns connect and TLS + /// end-to-end, the library does not (and cannot) apply + /// and friends on its behalf. + /// + /// Cancellation for the connect attempt. + /// + /// A transport that establishes TLS must report that via + /// ; if + /// is set and the transport reports otherwise, the + /// connection is failed rather than used. + /// [Experimental(RESPite.Experiments.Transport, UrlFormat = RESPite.Experiments.UrlFormat)] - public virtual ValueTask ConnectTransportAsync(EndPoint endpoint, ConnectionType connectionType, CancellationToken cancellationToken) => default; + public virtual ValueTask ConnectTransportAsync(EndPoint endpoint, ConnectionType connectionType, TlsOptions tls, CancellationToken cancellationToken) => default; private sealed class HttpProxyTunnel : Tunnel { diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index 381ee4426..b137f1316 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -709,4 +709,18 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) EventId = 109, Message = "Service name not defined.")] internal static partial void LogInformationServiceNameNotDefined(this ILogger logger); + +#if NET + [LoggerMessage( + Level = LogLevel.Information, + EventId = 110, + Message = "TLS connection established successfully using protocol: {SslProtocol}, cipher suite: {CipherSuite}")] + internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); +#endif + + [LoggerMessage( + Level = LogLevel.Information, + EventId = 111, + Message = "{BridgeName}: Transport connected (encrypted: {IsEncrypted})")] + internal static partial void LogInformationTransportConnected(this ILogger logger, string bridgeName, bool isEncrypted); } diff --git a/src/StackExchange.Redis/PhysicalBridge.cs b/src/StackExchange.Redis/PhysicalBridge.cs index 049824536..051460dd5 100644 --- a/src/StackExchange.Redis/PhysicalBridge.cs +++ b/src/StackExchange.Redis/PhysicalBridge.cs @@ -1236,6 +1236,36 @@ private void ProcessBridgeBacklog() } } + /// + /// Flush anything earlier writes left staged, but only if that can be done without waiting. + /// + /// + /// Called from a push transport's batch-end callback, which runs on a transport thread that must + /// not block. The write lock is what makes staging exclusive - a flush that raced a stage could + /// hand a half-written frame to a transport mid-Advance - so this either gets the lock + /// instantly or does nothing, leaving the flush to whoever does hold it. + /// + internal void TryFlushStagedWrites(PhysicalConnection physical) + { + if (!_singleWriter.TryTakeInstant()) return; + try + { + if (physical is { HasOutputPipe: true }) + { + physical.Flush(); + } + } + catch + { + // opportunistic: a flush failure here means the transport is going away, and that is + // reported through the receiver's OnClosed + } + finally + { + _singleWriter.Release(); + } + } + public bool HasPendingCallerFacingItems() { if (BacklogHasItems) diff --git a/src/StackExchange.Redis/PhysicalConnection.Transport.cs b/src/StackExchange.Redis/PhysicalConnection.Transport.cs index f1e04a153..75431d7e9 100644 --- a/src/StackExchange.Redis/PhysicalConnection.Transport.cs +++ b/src/StackExchange.Redis/PhysicalConnection.Transport.cs @@ -55,9 +55,16 @@ private sealed class TransportWriter(DuplexTransport transport, CancellationToke : BufferedStreamWriter(Stream.Null, cancellationToken) { private readonly TaskCompletionSource _done = new(TaskCreationOptions.RunContinuationsAsynchronously); + private int _staged; // bytes advanced since the last flush; see HasStagedBytes public override Task WriteComplete => _done.Task; + /// Whether anything has been advanced but not yet flushed; writes that do not demand + /// a flush (fire-and-forget) leave bytes here for a later flush to pick up. Deliberately + /// approximate in the safe direction: it can over-report (a concurrent stage during a flush + /// stays counted), which costs a redundant flush attempt, never a missed one. + public bool HasStagedBytes => Volatile.Read(ref _staged) != 0; + public override Memory GetMemory(int sizeHint = 0) => transport.GetMemory(sizeHint); public override Span GetSpan(int sizeHint = 0) => transport.GetSpan(sizeHint); @@ -65,16 +72,22 @@ private sealed class TransportWriter(DuplexTransport transport, CancellationToke public override void Advance(int count) { transport.Advance(count); + Interlocked.Add(ref _staged, count); OnWritten(count); } // A false return means the transport is closed; that failure surfaces via the receiver's // OnClosed (which records the connection failure), so it is not duplicated here. - public override void Flush() => transport.Flush(); + public override void Flush() + { + var flushing = Volatile.Read(ref _staged); + transport.Flush(); + Interlocked.Add(ref _staged, -flushing); + } public override void Complete(Exception? exception = null) { - try { transport.Flush(); } + try { Flush(); } catch { } if (exception is null) { @@ -121,6 +134,20 @@ public override bool OnReceived(ReadOnlySpan payload) } } + public override void OnBatchEnd() + { + // The contract asks for ONE flush per delivery burst rather than per OnReceived. The only + // thing that can be staged-but-unflushed here is a write issued by inbound processing + // that did not demand a flush (typically a fire-and-forget command from a completion + // continuation running inline on this thread), so check before reaching for the lock - + // the common case is nothing staged and nothing to do. + var conn = connection; + if (conn._output is TransportWriter { HasStagedBytes: true } && conn.BridgeCouldBeNull is { } bridge) + { + bridge.TryFlushStagedWrites(conn); + } + } + public override void OnClosed(Exception? fault) { var conn = connection; diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 2f5321580..8998d8ebe 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -183,8 +183,9 @@ internal async Task BeginConnectAsync(ILogger? log) #pragma warning disable SER009 // experimental transport contract: this is the consuming call site // A transport tunnel replaces the socket outright (the widest form of the existing // connectTo=null no-socket pattern): connect and TLS belong to the tunnel, and the - // stream/SslStream machinery below never runs. - var transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, CancellationToken.None).ForAwait(); + // stream/SslStream machinery below never runs. The TLS intent goes with it precisely + // because TLS is now the tunnel's job: it cannot honour an intent it cannot see. + var transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, new(rawConfig), CancellationToken.None).ForAwait(); #pragma warning restore SER009 if (transport is not null) { @@ -1036,15 +1037,25 @@ internal async ValueTask ConnectedAsync(Socket? socket, ILogger? log) if (_transport is { } transport) { - // Transport mode: no socket, no stream, no SslStream. TLS is the tunnel's job; a - // config that ALSO asks for stream-level TLS is a contradiction, and it fails loud - // rather than silently double-encrypting or silently skipping. - if (config.Ssl) + // Transport mode: no socket, no stream, no SslStream. TLS is the transport's job + // (it saw the config on ConnectTransportAsync), so the only question here is whether + // the transport DISAGREES with the configured intent. Configured TLS plus a + // transport that reports plaintext is a hard failure: silently sending credentials + // in the clear is exactly what Ssl=true forbids. The converse - a transport that is + // encrypted when the config did not demand it - is fine, and merely noted. + if (config.Ssl && !transport.IsEncrypted) { - throw new NotSupportedException("With a transport-returning Tunnel, TLS belongs to the tunnel; do not also set Ssl=true."); + var mismatch = new RedisConnectionException( + ConnectionFailureType.AuthenticationFailure, + CommandFlags.None, + "TLS was requested (Ssl=true), but the transport supplied by the tunnel reports an unencrypted connection."); + RecordConnectionFailed(ConnectionFailureType.AuthenticationFailure, mismatch, isInitialConnect: true); + bridge.Multiplexer.Trace("Transport is not encrypted"); + return false; } + InitTransportOutput(transport); - log?.LogInformationConnected(bridge.Name); + log?.LogInformationTransportConnected(bridge.Name, transport.IsEncrypted); await bridge.OnConnectedAsync(this, log).ForAwait(); return true; } @@ -1099,7 +1110,20 @@ static Stream DemandSocketStream(Socket? socket) bridge.Multiplexer.Logger?.LogErrorConnectionIssue(ex, ex.Message); throw; } + // Note on the "assert ssl.IsEncrypted after the handshake" advice: on every TFM we + // target, SslStream.IsEncrypted (and IsSigned) is literally an alias for + // IsAuthenticated, i.e. "handshake completed, no exception" - which is exactly what + // the await above already proved, so asserting it would guarantee nothing about the + // bytes on the wire. What actually forbids plaintext here is the + // EncryptionPolicy.RequireEncryption passed to the ctor above; instead of a + // tautological assert we log what was negotiated, so a surprise is visible. + // .NET: https://github.com/dotnet/dotnet/blob/b0f34d51fccc69fd334253924abd8d6853fad7aa/src/runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs#L475 + // netfx: https://github.com/microsoft/referencesource/blob/main/System/net/System/Net/SecureProtocols/SslStream.cs#L312-L334 +#if NET + log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol, ssl.NegotiatedCipherSuite); +#else log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol); +#endif } catch (AuthenticationException authexception) { diff --git a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt index ddf237e1a..232f28b5f 100644 --- a/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/PublicAPI.Unshipped.txt @@ -1,2 +1,12 @@ #nullable enable -[SER009]virtual StackExchange.Redis.Configuration.Tunnel.ConnectTransportAsync(System.Net.EndPoint! endpoint, StackExchange.Redis.ConnectionType connectionType, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask +[SER009]StackExchange.Redis.Configuration.TlsOptions +[SER009]StackExchange.Redis.Configuration.TlsOptions.TlsOptions() -> void +[SER009]StackExchange.Redis.Configuration.TlsOptions.TlsOptions(StackExchange.Redis.ConfigurationOptions! options) -> void +[SER009]StackExchange.Redis.Configuration.TlsOptions.IsEnabled.get -> bool +[SER009]StackExchange.Redis.Configuration.TlsOptions.SslHost.get -> string? +[SER009]StackExchange.Redis.Configuration.TlsOptions.SslProtocols.get -> System.Security.Authentication.SslProtocols? +[SER009]StackExchange.Redis.Configuration.TlsOptions.CheckCertificateRevocation.get -> bool +[SER009]StackExchange.Redis.Configuration.TlsOptions.CertificateValidationCallback.get -> System.Net.Security.RemoteCertificateValidationCallback? +[SER009]StackExchange.Redis.Configuration.TlsOptions.CertificateSelectionCallback.get -> System.Net.Security.LocalCertificateSelectionCallback? +[SER009]StackExchange.Redis.Configuration.TlsOptions.ResolveHost(System.Net.EndPoint! endpoint) -> string! +[SER009]virtual StackExchange.Redis.Configuration.Tunnel.ConnectTransportAsync(System.Net.EndPoint! endpoint, StackExchange.Redis.ConnectionType connectionType, StackExchange.Redis.Configuration.TlsOptions tls, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask diff --git a/src/StackExchange.Redis/PublicAPI/net6.0/PublicAPI.Unshipped.txt b/src/StackExchange.Redis/PublicAPI/net6.0/PublicAPI.Unshipped.txt index ab058de62..494bb1b62 100644 --- a/src/StackExchange.Redis/PublicAPI/net6.0/PublicAPI.Unshipped.txt +++ b/src/StackExchange.Redis/PublicAPI/net6.0/PublicAPI.Unshipped.txt @@ -1 +1,2 @@ #nullable enable +[SER009]StackExchange.Redis.Configuration.TlsOptions.GetSslClientAuthenticationOptions(string! host) -> System.Net.Security.SslClientAuthenticationOptions? From 79336b66edaad2b6a820e89283adbca77cf2efdd Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 13 Aug 2026 13:22:21 +0100 Subject: [PATCH 3/6] TLA casing: LogInformationTlsConnectionEstablished/ConfiguringTls, not TLS Framework convention is Pascal casing for acronyms of three or more letters (only two-letter ones stay upper: IPEndPoint, IOException), and the BCL is consistent about this exact acronym: SslStream, SslProtocols.Tls13, SslClientAuthenticationOptions, TlsCipherSuite. The repo already leaned that way - TlsHandshakeAsync, and the new TlsOptions - leaving these two internal LoggerMessage partials as the only TLS-cased identifiers in src. Internal, so this is rename-only: no API file, no compat surface, six call sites. Prose is untouched - "Configuring TLS" in a log message is English, where the upper-case acronym is correct. --- src/StackExchange.Redis/LoggerExtensions.cs | 6 +++--- src/StackExchange.Redis/PhysicalConnection.cs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index b137f1316..67d558055 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -652,13 +652,13 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) Level = LogLevel.Information, EventId = 98, Message = "Configuring TLS")] - internal static partial void LogInformationConfiguringTLS(this ILogger logger); + internal static partial void LogInformationConfiguringTls(this ILogger logger); [LoggerMessage( Level = LogLevel.Information, EventId = 99, Message = "TLS connection established successfully using protocol: {SslProtocol}")] - internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol); + internal static partial void LogInformationTlsConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol); [LoggerMessage( Level = LogLevel.Information, @@ -715,7 +715,7 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) Level = LogLevel.Information, EventId = 110, Message = "TLS connection established successfully using protocol: {SslProtocol}, cipher suite: {CipherSuite}")] - internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); + internal static partial void LogInformationTlsConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); #endif [LoggerMessage( diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 8998d8ebe..bec84520d 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -1071,7 +1071,7 @@ static Stream DemandSocketStream(Socket? socket) if (config.Ssl) { - log?.LogInformationConfiguringTLS(); + log?.LogInformationConfiguringTls(); var host = config.SslHost; if (host.IsNullOrWhiteSpace()) { @@ -1120,9 +1120,9 @@ static Stream DemandSocketStream(Socket? socket) // .NET: https://github.com/dotnet/dotnet/blob/b0f34d51fccc69fd334253924abd8d6853fad7aa/src/runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs#L475 // netfx: https://github.com/microsoft/referencesource/blob/main/System/net/System/Net/SecureProtocols/SslStream.cs#L312-L334 #if NET - log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol, ssl.NegotiatedCipherSuite); + log?.LogInformationTlsConnectionEstablished(ssl.SslProtocol, ssl.NegotiatedCipherSuite); #else - log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol); + log?.LogInformationTlsConnectionEstablished(ssl.SslProtocol); #endif } catch (AuthenticationException authexception) From 36f7c93930a2776043aeb9b58f6c1f529b2125d3 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 13 Aug 2026 13:29:34 +0100 Subject: [PATCH 4/6] NoWarn SER009 solution-wide; drop the per-file pragmas The experimental gate exists to stop *randoms* leaning on an unstable transport seam, not to make our own consuming code apologise for itself in every file - and SER001/004/005/007/008 are already handled this way in the root props, so SER009 was the odd one out. That removes the file-level disables in PhysicalConnection.Transport.cs and LoggingTunnel.cs and the disable/restore pair around the ConnectTransportAsync call site; the [SER009] prefixes in the PublicAPI files are unrelated and stay. Also: both files this branch introduced were missing the utf-8-bom the .editorconfig asks for. --- Directory.Build.props | 2 +- src/StackExchange.Redis/Configuration/LoggingTunnel.cs | 4 +--- src/StackExchange.Redis/Configuration/TlsOptions.cs | 2 +- src/StackExchange.Redis/PhysicalConnection.Transport.cs | 4 +--- src/StackExchange.Redis/PhysicalConnection.cs | 2 -- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 62c89b137..86c8c6ccc 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -11,7 +11,7 @@ $(MSBuildThisFileDirectory)Shared.ruleset NETSDK1069 - $(NoWarn);NU5105;NU1507;SER001;SER004;SER005;SER007;SER008 + $(NoWarn);NU5105;NU1507;SER001;SER004;SER005;SER007;SER008;SER009 https://github.com/StackExchange/StackExchange.Redis/releases https://stackexchange.github.io/StackExchange.Redis/ MIT diff --git a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs index 32d5567cc..edb5512db 100644 --- a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs +++ b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs @@ -1,6 +1,4 @@ -#pragma warning disable SER009 // experimental transport contract: this file logs the consumer side of it - -using System; +using System; using System.Buffers; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; diff --git a/src/StackExchange.Redis/Configuration/TlsOptions.cs b/src/StackExchange.Redis/Configuration/TlsOptions.cs index 81a1bda1b..4ede76ba0 100644 --- a/src/StackExchange.Redis/Configuration/TlsOptions.cs +++ b/src/StackExchange.Redis/Configuration/TlsOptions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Diagnostics.CodeAnalysis; using System.Net; using System.Net.Security; diff --git a/src/StackExchange.Redis/PhysicalConnection.Transport.cs b/src/StackExchange.Redis/PhysicalConnection.Transport.cs index 75431d7e9..a0258166a 100644 --- a/src/StackExchange.Redis/PhysicalConnection.Transport.cs +++ b/src/StackExchange.Redis/PhysicalConnection.Transport.cs @@ -1,6 +1,4 @@ -#pragma warning disable SER009 // experimental transport contract: this file IS the consumer side of it - -using System; +using System; using System.Buffers; using System.IO; using System.Threading; diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index bec84520d..90e35e249 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -180,13 +180,11 @@ internal async Task BeginConnectAsync(ILogger? log) var connectTo = endpoint; if (tunnel is not null) { -#pragma warning disable SER009 // experimental transport contract: this is the consuming call site // A transport tunnel replaces the socket outright (the widest form of the existing // connectTo=null no-socket pattern): connect and TLS belong to the tunnel, and the // stream/SslStream machinery below never runs. The TLS intent goes with it precisely // because TLS is now the tunnel's job: it cannot honour an intent it cannot see. var transport = await tunnel.ConnectTransportAsync(endpoint, bridge.ConnectionType, new(rawConfig), CancellationToken.None).ForAwait(); -#pragma warning restore SER009 if (transport is not null) { _transport = transport; From a5eea5b064e8cd4dee0abce1e0e042248e57d806 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 13 Aug 2026 13:33:44 +0100 Subject: [PATCH 5/6] TlsOptions.WithTls(): force the intent, do not clone the configuration LoggingTunnel needed the tail to see TLS after its own constructor cleared Ssl from the live options, and cloning ConfigurationOptions per connect attempt to carry one bit was a poor trade. TlsOptions now has a private _forceTls alongside the wrapped options, a private (options, forceTls) constructor, and an internal WithTls() - so the call site is one line and allocation-free: `if (_ssl) tls = tls.WithTls();`. IsEnabled ORs the flag over the configuration; every other accessor is untouched, since only the intent differs, never the host/protocols/callbacks. The public constructor chains to the private one (with the null check as a throw expression) rather than assigning a subset and letting the compiler zero-init the rest via an implicit : this(). Internal member and private constructor, so no PublicAPI change. Builds clean on all TFMs with RunAnalyzers=true; Parse/Config/SSL batteries pass (310). --- .../Configuration/LoggingTunnel.cs | 7 +------ .../Configuration/TlsOptions.cs | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs index edb5512db..a76231cd7 100644 --- a/src/StackExchange.Redis/Configuration/LoggingTunnel.cs +++ b/src/StackExchange.Redis/Configuration/LoggingTunnel.cs @@ -373,12 +373,7 @@ private void CreateLogFiles(EndPoint endpoint, ConnectionType connectionType, ou // our constructor deliberately cleared from the live options, since when we do the handshake // ourselves the library must not also wrap the connection. Note that what we log here is // therefore the plaintext RESP either way: we sit above the tail's own encryption. - if (_ssl && !tls.IsEnabled) - { - var forTail = _options.Clone(); - forTail.Ssl = true; - tls = new TlsOptions(forTail); - } + if (_ssl) tls = tls.WithTls(); var transport = await _tail.ConnectTransportAsync(endpoint, connectionType, tls, cancellationToken).ForAwait(); return transport is null ? null : Log(transport, endpoint, connectionType); diff --git a/src/StackExchange.Redis/Configuration/TlsOptions.cs b/src/StackExchange.Redis/Configuration/TlsOptions.cs index 4ede76ba0..2e0d9f533 100644 --- a/src/StackExchange.Redis/Configuration/TlsOptions.cs +++ b/src/StackExchange.Redis/Configuration/TlsOptions.cs @@ -20,19 +20,35 @@ namespace StackExchange.Redis.Configuration; public readonly struct TlsOptions { private readonly ConfigurationOptions? _options; + private readonly bool _forceTls; /// /// Create a view over the TLS settings of the supplied configuration. /// public TlsOptions(ConfigurationOptions options) - => _options = options ?? throw new ArgumentNullException(nameof(options)); + : this(options ?? throw new ArgumentNullException(nameof(options)), forceTls: false) + { + } + + private TlsOptions(ConfigurationOptions? options, bool forceTls) + { + _options = options; + _forceTls = forceTls; + } /// /// Whether TLS is required for this connection; corresponds to . /// A transport that returns false from /// when this is set will not be used. /// - public bool IsEnabled => _options?.Ssl ?? false; + public bool IsEnabled => _forceTls || (_options?.Ssl ?? false); + + /// + /// The same view, but with TLS required regardless of what the configuration says - for the intermediate + /// tunnel that cleared for its own reasons but is chaining to a + /// tail that owns the handshake. Everything else (host, protocols, callbacks) is unchanged. + /// + internal TlsOptions WithTls() => new(_options, forceTls: true); /// /// The configured TLS host name, used for SNI and certificate validation; corresponds to From ea75a8bd78bca35d1f62bec1fa49b052bba2906f Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 13 Aug 2026 13:48:46 +0100 Subject: [PATCH 6/6] Revert the TLS logger renames: the method name IS the shipped event name The generated half settles it - the source generator emits new EventId(, nameof()), so the method name lands in EventId.Name and reaches consumers as the event name ("EventName" in most structured sinks), where it can be filtered or alerted on. Nothing is free at 8M+ downloads a day, so the awkward spelling stays: LogInformationConfiguringTLS and LogInformationTLSConnectionEstablished are back, with a note at the top of the generated section stating that renaming these is a telemetry change rather than a refactor (and that EventName can pin an old name if one ever must move), plus a marker at the two TLA-offending declarations so nobody tidies them again. While applying the same reasoning to the code added on this branch: the net6+ cipher-suite variant now declares EventId 99 - the same event as its sibling, one logical "TLS connection established" carrying an extra property where the platform can report it - instead of minting 110 and thereby reporting the same event under different ids on netfx and modern TFMs. The generator is happy to share an id across overloads; verified in the emitted source. LogInformationTransportConnected takes 110, and since both of those ids are new on this unshipped branch, no consumer ever saw either. Builds clean on all TFMs with RunAnalyzers=true; Config/SSL/Parse batteries pass (310). --- src/StackExchange.Redis/LoggerExtensions.cs | 24 ++++++++++++++----- src/StackExchange.Redis/PhysicalConnection.cs | 6 ++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/StackExchange.Redis/LoggerExtensions.cs b/src/StackExchange.Redis/LoggerExtensions.cs index 67d558055..552a38115 100644 --- a/src/StackExchange.Redis/LoggerExtensions.cs +++ b/src/StackExchange.Redis/LoggerExtensions.cs @@ -61,7 +61,13 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) log.LogInformationThreadPoolStats(composed); } - // Generated LoggerMessage methods + // Generated LoggerMessage methods. + // + // NOTE: renaming any method here is NOT a refactor. The generator emits the method name as the event + // name -- new EventId(, nameof()) -- so the name reaches consumers' telemetry (EventId.Name; + // "EventName" in most structured sinks), where it may be filtered or alerted on. At our download volume + // that is not a cost worth paying for tidiness: treat these names as fixed once shipped, however + // awkwardly they read. If a name genuinely must change, pin the old one via the attribute's EventName. [LoggerMessage( Level = LogLevel.Error, Message = "Connection failed: {EndPoint} ({ConnectionType}, {FailureType}): {ErrorMessage}")] @@ -648,17 +654,20 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) Message = "{EndPoint}: (socket shutdown)")] internal static partial void LogErrorSocketShutdown(this ILogger logger, Exception exception, EndPointLogValue endPoint); + // "TLS" here breaks the usual TLA convention (compare SslStream, TlsCipherSuite, and TlsOptions in this + // repo), and is kept that way ON PURPOSE: these names shipped, so they are the event names our consumers + // already see. See the note at the top of the generated section. [LoggerMessage( Level = LogLevel.Information, EventId = 98, Message = "Configuring TLS")] - internal static partial void LogInformationConfiguringTls(this ILogger logger); + internal static partial void LogInformationConfiguringTLS(this ILogger logger); [LoggerMessage( Level = LogLevel.Information, EventId = 99, Message = "TLS connection established successfully using protocol: {SslProtocol}")] - internal static partial void LogInformationTlsConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol); + internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol); [LoggerMessage( Level = LogLevel.Information, @@ -711,16 +720,19 @@ internal static void LogWithThreadPoolStats(this ILogger? log, string message) internal static partial void LogInformationServiceNameNotDefined(this ILogger logger); #if NET + // deliberately the SAME event as its sibling above (id 99, same name): one logical event that + // simply carries an extra property where the platform can report it. A new id would mean "TLS connection + // established" arrived under one id on netfx and another on modern TFMs, for no gain to anyone. [LoggerMessage( Level = LogLevel.Information, - EventId = 110, + EventId = 99, Message = "TLS connection established successfully using protocol: {SslProtocol}, cipher suite: {CipherSuite}")] - internal static partial void LogInformationTlsConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); + internal static partial void LogInformationTLSConnectionEstablished(this ILogger logger, System.Security.Authentication.SslProtocols sslProtocol, System.Net.Security.TlsCipherSuite cipherSuite); #endif [LoggerMessage( Level = LogLevel.Information, - EventId = 111, + EventId = 110, Message = "{BridgeName}: Transport connected (encrypted: {IsEncrypted})")] internal static partial void LogInformationTransportConnected(this ILogger logger, string bridgeName, bool isEncrypted); } diff --git a/src/StackExchange.Redis/PhysicalConnection.cs b/src/StackExchange.Redis/PhysicalConnection.cs index 90e35e249..0d93eefeb 100644 --- a/src/StackExchange.Redis/PhysicalConnection.cs +++ b/src/StackExchange.Redis/PhysicalConnection.cs @@ -1069,7 +1069,7 @@ static Stream DemandSocketStream(Socket? socket) if (config.Ssl) { - log?.LogInformationConfiguringTls(); + log?.LogInformationConfiguringTLS(); var host = config.SslHost; if (host.IsNullOrWhiteSpace()) { @@ -1118,9 +1118,9 @@ static Stream DemandSocketStream(Socket? socket) // .NET: https://github.com/dotnet/dotnet/blob/b0f34d51fccc69fd334253924abd8d6853fad7aa/src/runtime/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs#L475 // netfx: https://github.com/microsoft/referencesource/blob/main/System/net/System/Net/SecureProtocols/SslStream.cs#L312-L334 #if NET - log?.LogInformationTlsConnectionEstablished(ssl.SslProtocol, ssl.NegotiatedCipherSuite); + log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol, ssl.NegotiatedCipherSuite); #else - log?.LogInformationTlsConnectionEstablished(ssl.SslProtocol); + log?.LogInformationTLSConnectionEstablished(ssl.SslProtocol); #endif } catch (AuthenticationException authexception)