Fix HTTP tunnel (#2448)

According to https://datatracker.ietf.org/doc/html/draft-luotonen-web-proxy-tunneling-01#section-3.2, the expected response to a `CONNECT` should be `HTTP/1.1 200 Connection established` instead of `HTTP/1.1 200 OK`. Allow both responses for now (as still a lot of proxy implementations fail to follow the standard).

A HTTP(S), SOCKS, etc. proxy is traditionally specified by an URI of form `scheme://[user:pass@]host[:port]`. Accept both formats for backwards compatibility.

Author: flobernd

docs/ReleaseNotes.md

@@ -11,6 +11,7 @@ Current package versions:
 - Fix [#2426](https://github.com/StackExchange/StackExchange.Redis/issues/2426): Don't restrict multi-slot operations on Envoy proxy; let the proxy decide ([#2428 by mgravell](https://github.com/StackExchange/StackExchange.Redis/pull/2428))
 - Add: Support for `User`/`Password` in `DefaultOptionsProvider` to support token rotation scenarios ([#2445 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2445))
 - Fix [#2449](https://github.com/StackExchange/StackExchange.Redis/issues/2449): Resolve AOT trim warnings in `TryGetAzureRoleInstanceIdNoThrow` ([#2451 by eerhardt](https://github.com/StackExchange/StackExchange.Redis/pull/2451))
+- Adds: Support for `HTTP/1.1 200 Connection established` in HTTP Tunnel ([#2448 by flobernd](https://github.com/StackExchange/StackExchange.Redis/pull/2448))
 
 ## 2.6.104
 

src/StackExchange.Redis/Configuration/Tunnel.cs

@@ -49,10 +49,10 @@ private sealed class HttpProxyTunnel : Tunnel
                 {
                     var encoding = Encoding.ASCII;
                     var ep = Format.ToString(endpoint);
-                    const string Prefix = "CONNECT ", Suffix = " HTTP/1.1\r\n\r\n", ExpectedResponse = "HTTP/1.1 200 OK\r\n\r\n";
+                    const string Prefix = "CONNECT ", Suffix = " HTTP/1.1\r\n\r\n", ExpectedResponse1 = "HTTP/1.1 200 OK\r\n\r\n", ExpectedResponse2 = "HTTP/1.1 200 Connection established\r\n\r\n";
                     byte[] chunk = ArrayPool<byte>.Shared.Rent(Math.Max(
                         encoding.GetByteCount(Prefix) + encoding.GetByteCount(ep) + encoding.GetByteCount(Suffix),
-                        encoding.GetByteCount(ExpectedResponse)
+                        Math.Max(encoding.GetByteCount(ExpectedResponse1), encoding.GetByteCount(ExpectedResponse2))
                     ));
                     var offset = 0;
                     offset += encoding.GetBytes(Prefix, 0, Prefix.Length, chunk, offset);
@@ -76,10 +76,11 @@ static void SafeAbort(object? obj)
                         await args;
 
                         // we expect to see: "HTTP/1.1 200 OK\n"; note our buffer is definitely big enough already
-                        int toRead = encoding.GetByteCount(ExpectedResponse), read;
+                        int toRead = Math.Max(encoding.GetByteCount(ExpectedResponse1), encoding.GetByteCount(ExpectedResponse2)), read;
                         offset = 0;
 
-                        while (toRead > 0)
+                        var actualResponse = "";
+                        while (toRead > 0 && !actualResponse.EndsWith("\r\n\r\n"))
                         {
                             args.SetBuffer(chunk, offset, toRead);
                             if (!socket.ReceiveAsync(args)) args.Complete();
@@ -88,11 +89,12 @@ static void SafeAbort(object? obj)
                             if (read <= 0) break; // EOF (since we're never doing zero-length reads)
                             toRead -= read;
                             offset += read;
+
+                            actualResponse = encoding.GetString(chunk, 0, offset);
                         }
-                        if (toRead != 0) throw new EndOfStreamException("EOF negotiating HTTP tunnel");
+                        if (toRead != 0 && !actualResponse.EndsWith("\r\n\r\n")) throw new EndOfStreamException("EOF negotiating HTTP tunnel");
                         // lazy
-                        var actualResponse = encoding.GetString(chunk, 0, offset);
-                        if (ExpectedResponse != actualResponse)
+                        if (ExpectedResponse1 != actualResponse && ExpectedResponse2 != actualResponse)
                         {
                             throw new InvalidOperationException("Unexpected response negotiating HTTP tunnel");
                         }

src/StackExchange.Redis/ConfigurationOptions.cs

@@ -912,19 +912,25 @@ private ConfigurationOptions DoParse(string configuration, bool ignoreUnknown)
                             {
                                 Tunnel = null;
                             }
-                            else if (value.StartsWith("http:"))
+                            else
                             {
-                                value = value.Substring(5);
-                                if (!Format.TryParseEndPoint(value, out var ep))
+                                // For backwards compatibility with `http:address_with_port`.
+                                if (value.StartsWith("http:") && !value.StartsWith("http://"))
+                                {
+                                    value = value.Insert(5, "//");
+                                }
+
+                                var uri = new Uri(value, UriKind.Absolute);
+                                if (uri.Scheme != "http")
+                                {
+                                    throw new ArgumentException("Tunnel cannot be parsed: " + value);
+                                }
+                                if (!Format.TryParseEndPoint($"{uri.Host}:{uri.Port}", out var ep))
                                 {
                                     throw new ArgumentException("HTTP tunnel cannot be parsed: " + value);
                                 }
                                 Tunnel = Tunnel.HttpProxy(ep);
                             }
-                            else
-                            {
-                                throw new ArgumentException("Tunnel cannot be parsed: " + value);
-                            }
                             break;
                         // Deprecated options we ignore...
                         case OptionKeys.HighPrioritySocketThreads:

tests/StackExchange.Redis.Tests/ConfigTests.cs

@@ -619,19 +619,21 @@ public async Task MutableOptions()
         Assert.Equal(newPass, conn.RawConfig.Password);
     }
 
-    [Fact]
-    public void HttpTunnelCanRoundtrip()
+    [Theory]
+    [InlineData("http://somewhere:22", "http:somewhere:22")]
+    [InlineData("http:somewhere:22", "http:somewhere:22")]
+    public void HttpTunnelCanRoundtrip(string input, string expected)
     {
-        var config = ConfigurationOptions.Parse("127.0.0.1:6380,tunnel=http:somewhere:22");
+        var config = ConfigurationOptions.Parse($"127.0.0.1:6380,tunnel={input}");
         var ip = Assert.IsType<IPEndPoint>(Assert.Single(config.EndPoints));
         Assert.Equal(6380, ip.Port);
         Assert.Equal("127.0.0.1", ip.Address.ToString());
 
         Assert.NotNull(config.Tunnel);
-        Assert.Equal("http:somewhere:22", config.Tunnel.ToString());
+        Assert.Equal(expected, config.Tunnel.ToString());
 
         var cs = config.ToString();
-        Assert.Equal("127.0.0.1:6380,tunnel=http:somewhere:22", cs);
+        Assert.Equal($"127.0.0.1:6380,tunnel={expected}", cs);
     }
 
     private class CustomTunnel : Tunnel { }