adamfisk#149 ProxyToServerConnection now removes itself as handler fr…

…om the old context when falling back to another chained proxy

src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java

@@ -1250,6 +1250,9 @@ private void recordClientDisconnected() {
     }
 
     public InetSocketAddress getClientAddress() {
+        if (channel == null) {
+            return null;
+        }
         return (InetSocketAddress) channel.remoteAddress();
     }
 

src/main/java/org/littleshoot/proxy/impl/ProxyConnection.java

@@ -5,6 +5,7 @@
 import io.netty.buffer.Unpooled;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
+import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelOutboundHandlerAdapter;
@@ -121,7 +122,7 @@ protected void read(Object msg) {
         LOG.debug("Reading: {}", msg);
 
         lastReadTime = System.currentTimeMillis();
-        
+
         if (tunneling) {
             // In tunneling mode, this connection is simply shoveling bytes
             readRaw((ByteBuf) msg);
@@ -291,7 +292,7 @@ protected void disconnected() {
         become(DISCONNECTED);
         LOG.debug("Disconnected");
     }
-    
+
     /**
      * This method is called when the underlying {@link Channel} times out due
      * to an idle timeout.

src/main/java/org/littleshoot/proxy/impl/ProxyToServerConnection.java

@@ -647,11 +647,15 @@ protected boolean connectionFailed(Throwable cause)
         }
         this.chainedProxy = this.availableChainedProxies.poll();
         if (chainedProxy != null) {
+            // Remove ourselves as handler on the old context
+            this.ctx.pipeline().remove(this);
+            this.ctx.close();
+            this.ctx = null;
             this.setupConnectionParameters();
             this.connectAndWrite(initialRequest);
-            return true;
+            return true; // yes, we fell back
         } else {
-            return false;
+            return false; // nothing to fall back to
         }
     }
 

src/test/java/org/littleshoot/proxy/BadServerAuthenticationTCPChainedProxyTest.java

@@ -0,0 +1,52 @@
+package org.littleshoot.proxy;
+
+import static org.littleshoot.proxy.TransportProtocol.*;
+
+import javax.net.ssl.SSLEngine;
+
+import org.littleshoot.proxy.extras.SelfSignedSslEngineSource;
+
+/**
+ * Tests that servers are authenticated and that if they're missing certs, we
+ * get an error.
+ */
+public class BadServerAuthenticationTCPChainedProxyTest extends
+        BaseChainedProxyTest {
+    protected final SslEngineSource serverSslEngineSource = new SelfSignedSslEngineSource(
+            "chain_proxy_keystore_1.jks");
+
+    protected final SslEngineSource clientSslEngineSource = new SelfSignedSslEngineSource(
+            "chain_proxy_keystore_2.jks");
+
+    @Override
+    protected boolean expectBadGatewayForEverything() {
+        return true;
+    }
+
+    @Override
+    protected HttpProxyServerBootstrap upstreamProxy() {
+        return super.upstreamProxy()
+                .withTransportProtocol(TCP)
+                .withSslEngineSource(serverSslEngineSource);
+    }
+
+    @Override
+    protected ChainedProxy newChainedProxy() {
+        return new BaseChainedProxy() {
+            @Override
+            public TransportProtocol getTransportProtocol() {
+                return TransportProtocol.TCP;
+            }
+
+            @Override
+            public boolean requiresEncryption() {
+                return true;
+            }
+
+            @Override
+            public SSLEngine newSslEngine() {
+                return clientSslEngineSource.newSslEngine();
+            }
+        };
+    }
+}

src/test/java/org/littleshoot/proxy/BaseChainedProxyTest.java

@@ -96,23 +96,23 @@ protected void tearDown() throws Exception {
     @Override
     public void testSimplePostRequest() throws Exception {
         super.testSimplePostRequest();
-        if (!expectBadGatewayForEverything()) {
+        if (isChained() && !expectBadGatewayForEverything()) {
             assertThatUpstreamProxyReceivedSentRequests();
         }
     }
 
     @Override
     public void testSimpleGetRequest() throws Exception {
         super.testSimpleGetRequest();
-        if (!expectBadGatewayForEverything()) {
+        if (isChained() && !expectBadGatewayForEverything()) {
             assertThatUpstreamProxyReceivedSentRequests();
         }
     }
 
     @Override
     public void testProxyWithBadAddress() throws Exception {
         super.testProxyWithBadAddress();
-        if (!expectBadGatewayForEverything()) {
+        if (isChained() && !expectBadGatewayForEverything()) {
             assertThatUpstreamProxyReceivedSentRequests();
         }
     }

src/test/java/org/littleshoot/proxy/ChainedProxyWithFallbackDueToSSLTest.java → src/test/java/org/littleshoot/proxy/ChainedProxyWithFallbackToDirectDueToSSLTest.java

@@ -9,8 +9,15 @@
  * the downstream proxy is unavailable, the downstream proxy should just fall
  * back to a direct connection.
  */
-public class ChainedProxyWithFallbackDueToSSLTest extends
-        BadClientAuthenticationTCPChainedProxyTest {
+public class ChainedProxyWithFallbackToDirectDueToSSLTest extends
+        BadServerAuthenticationTCPChainedProxyTest {
+    @Override
+    protected boolean isChained() {
+        // Set this to false since we don't actually expect anything to go
+        // through the chained proxy
+        return false;
+    }
+
     @Override
     protected boolean expectBadGatewayForEverything() {
         return false;
@@ -21,6 +28,7 @@ protected ChainedProxyManager chainedProxyManager() {
             @Override
             public void lookupChainedProxies(HttpRequest httpRequest,
                     Queue<ChainedProxy> chainedProxies) {
+                // This first one has a bad cert
                 chainedProxies.add(newChainedProxy());
                 chainedProxies
                         .add(ChainedProxyAdapter.FALLBACK_TO_DIRECT_CONNECTION);

src/test/java/org/littleshoot/proxy/ChainedProxyWithFallbackToOtherChainedProxyDueToSSLTest.java

@@ -0,0 +1,48 @@
+package org.littleshoot.proxy;
+
+import io.netty.handler.codec.http.HttpRequest;
+
+import java.util.Queue;
+
+import javax.net.ssl.SSLEngine;
+
+/**
+ * Tests a proxy chained to a downstream proxy with an untrusted SSL cert. When
+ * the downstream proxy is unavailable, the downstream proxy should just fall
+ * back to a the next chained proxy.
+ */
+public class ChainedProxyWithFallbackToOtherChainedProxyDueToSSLTest extends
+        BadServerAuthenticationTCPChainedProxyTest {
+    @Override
+    protected boolean expectBadGatewayForEverything() {
+        return false;
+    }
+
+    protected ChainedProxyManager chainedProxyManager() {
+        return new ChainedProxyManager() {
+            @Override
+            public void lookupChainedProxies(HttpRequest httpRequest,
+                    Queue<ChainedProxy> chainedProxies) {
+                // This first one has a bad cert
+                chainedProxies.add(newChainedProxy());
+                // This 2nd one should work
+                chainedProxies.add(new BaseChainedProxy() {
+                    @Override
+                    public TransportProtocol getTransportProtocol() {
+                        return TransportProtocol.TCP;
+                    }
+
+                    @Override
+                    public boolean requiresEncryption() {
+                        return true;
+                    }
+
+                    @Override
+                    public SSLEngine newSslEngine() {
+                        return serverSslEngineSource.newSslEngine();
+                    }
+                });
+            }
+        };
+    }
+}