custom tunnel should not break ConfigurationOptions.ToString()

Author: mgravell

src/StackExchange.Redis/Configuration/Tunnel.cs

@@ -21,6 +21,8 @@ public abstract class Tunnel
         /// <remarks><c>null</c> should be returned if a socket is not required for this endpoint.</remarks>
         public virtual ValueTask<EndPoint?> GetSocketConnectEndpointAsync(EndPoint endpoint, CancellationToken cancellationToken) => new(endpoint);
 
+        internal virtual bool IsInbuilt => false; // only inbuilt tunnels get added to config strings
+
         /// <summary>
         /// Allows modification of a <see cref="Socket"/> between creation and connection.
         /// Passed in is the endpoint we're connecting to, which type of connection it is, and the socket itself.
@@ -33,8 +35,6 @@ public abstract class Tunnel
         /// the entire data flow can be intercepted, providing entire custom transports.
         /// </summary>
         public virtual ValueTask<Stream?> BeforeAuthenticateAsync(EndPoint endpoint, ConnectionType connectionType, Socket? socket, CancellationToken cancellationToken) => default;
-        /// <inheritdoc/>
-        public abstract override string ToString();
 
         private sealed class HttpProxyTunnel : Tunnel
         {
@@ -102,6 +102,7 @@ static void SafeAbort(object? obj)
                 return default; // no need for custom stream wrapper here
             }
 
+            internal override bool IsInbuilt => true;
             public override string ToString() => "http:" + Format.ToString(Proxy);
         }
 

src/StackExchange.Redis/ConfigurationOptions.cs

@@ -731,7 +731,7 @@ public string ToString(bool includePassword)
             Append(sb, OptionKeys.ConfigCheckSeconds, configCheckSeconds);
             Append(sb, OptionKeys.ResponseTimeout, responseTimeout);
             Append(sb, OptionKeys.DefaultDatabase, DefaultDatabase);
-            if (Tunnel is Tunnel tunnel)
+            if (Tunnel is { IsInbuilt: true } tunnel)
             {
                 Append(sb, OptionKeys.Tunnel, tunnel.ToString());
             }

src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt

@@ -182,7 +182,6 @@ StackExchange.Redis.Configuration.DefaultOptionsProvider.ClientName.get -> strin
 StackExchange.Redis.Configuration.DefaultOptionsProvider.DefaultOptionsProvider() -> void
 StackExchange.Redis.Configuration.Tunnel
 StackExchange.Redis.Configuration.Tunnel.Tunnel() -> void
-override abstract StackExchange.Redis.Configuration.Tunnel.ToString() -> string!
 static StackExchange.Redis.Configuration.Tunnel.HttpProxy(System.Net.EndPoint! proxy) -> StackExchange.Redis.Configuration.Tunnel!
 virtual StackExchange.Redis.Configuration.Tunnel.BeforeAuthenticateAsync(System.Net.EndPoint! endpoint, StackExchange.Redis.ConnectionType connectionType, System.Net.Sockets.Socket? socket, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask<System.IO.Stream?>
 virtual StackExchange.Redis.Configuration.Tunnel.BeforeSocketConnectAsync(System.Net.EndPoint! endPoint, StackExchange.Redis.ConnectionType connectionType, System.Net.Sockets.Socket? socket, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask

tests/StackExchange.Redis.Tests/Config.cs

@@ -1,4 +1,5 @@
-using System;
+using StackExchange.Redis.Configuration;
+using System;
 using System.Globalization;
 using System.IO;
 using System.IO.Pipelines;
@@ -613,7 +614,7 @@ public async Task MutableOptions()
     }
 
     [Fact]
-    public void HttpTunnel()
+    public void HttpTunnelCanRoundtrip()
     {
         var config = ConfigurationOptions.Parse("127.0.0.1:6380,tunnel=http:somewhere:22");
         var ip = Assert.IsType<IPEndPoint>(Assert.Single(config.EndPoints));
@@ -626,4 +627,19 @@ public void HttpTunnel()
         var cs = config.ToString();
         Assert.Equal("127.0.0.1:6380,tunnel=http:somewhere:22", cs);
     }
+
+    private class CustomTunnel : Tunnel { }
+
+    [Fact]
+    public void CustomTunnelCanRoundtripMinusTunnel()
+    {
+        // we don't expect to be able to parse custom tunnels, but we should still be able to round-trip
+        // the rest of the config, which means ignoring them *in both directions* (unless first party)
+        var options = ConfigurationOptions.Parse("127.0.0.1,Ssl=true");
+        options.Tunnel = new CustomTunnel();
+        var cs = options.ToString();
+        Assert.Equal("127.0.0.1,ssl=True", cs);
+        options = ConfigurationOptions.Parse(cs);
+        Assert.Null(options.Tunnel);
+    }
 }