From 23f033ac08906b8db4dbb5695d6f0701ad14e4d3 Mon Sep 17 00:00:00 2001
From: Vladimir Kostyukov <vkostyukov@twitter.com>
Date: Mon, 24 Oct 2016 10:52:42 -0700
Subject: [PATCH] Read if needed on ProxyHandler's handshake. Fixes #5933.

Motivation:

When auto-read is disabled and no reads are issued by a user, ProxyHandler will stall the connection on the proxy handshake phase waiting for the first response from a server (that was never read).

Modifications:

Read if needed when very first handshake message is send by ProxyHandler.

Result:

Proxy handshake now succeeds no matter if auto-read disabled or enabled. Without the fix, the new test is failing on master.
---
 .../java/io/netty/handler/proxy/ProxyHandler.java    | 12 +++++++++---
 .../io/netty/handler/proxy/ProxyHandlerTest.java     | 12 ++++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java b/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java
index 8d5b80b0aa..756de090a0 100644
--- a/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java
+++ b/handler-proxy/src/main/java/io/netty/handler/proxy/ProxyHandler.java
@@ -208,6 +208,8 @@ public abstract class ProxyHandler extends ChannelDuplexHandler {
         if (initialMessage != null) {
             sendToProxyServer(initialMessage);
         }
+
+        readIfNeeded(ctx);
     }
 
     /**
@@ -384,9 +386,7 @@ public abstract class ProxyHandler extends ChannelDuplexHandler {
         if (suppressChannelReadComplete) {
             suppressChannelReadComplete = false;
 
-            if (!ctx.channel().config().isAutoRead()) {
-                ctx.read();
-            }
+            readIfNeeded(ctx);
         } else {
             ctx.fireChannelReadComplete();
         }
@@ -412,6 +412,12 @@ public abstract class ProxyHandler extends ChannelDuplexHandler {
         }
     }
 
+    private static void readIfNeeded(ChannelHandlerContext ctx) {
+        if (!ctx.channel().config().isAutoRead()) {
+            ctx.read();
+        }
+    }
+
     private void writePendingWrites() {
         if (pendingWrites != null) {
             pendingWrites.removeAndWriteAll();
diff --git a/handler-proxy/src/test/java/io/netty/handler/proxy/ProxyHandlerTest.java b/handler-proxy/src/test/java/io/netty/handler/proxy/ProxyHandlerTest.java
index 1c5cb5ec06..697b57bfe4 100644
--- a/handler-proxy/src/test/java/io/netty/handler/proxy/ProxyHandlerTest.java
+++ b/handler-proxy/src/test/java/io/netty/handler/proxy/ProxyHandlerTest.java
@@ -26,6 +26,7 @@ import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandler;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInitializer;
+import io.netty.channel.ChannelOption;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.EventLoopGroup;
 import io.netty.channel.SimpleChannelInboundHandler;
@@ -61,6 +62,7 @@ import java.util.List;
 import java.util.Queue;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.TimeUnit;
 
 import static org.hamcrest.CoreMatchers.*;
@@ -363,9 +365,16 @@ public class ProxyHandlerTest {
         final Queue<Throwable> exceptions = new LinkedBlockingQueue<Throwable>();
         volatile int eventCount;
 
+        private static void readIfNeeded(ChannelHandlerContext ctx) {
+            if (!ctx.channel().config().isAutoRead()) {
+                ctx.read();
+            }
+        }
+
         @Override
         public void channelActive(ChannelHandlerContext ctx) throws Exception {
             ctx.writeAndFlush(Unpooled.copiedBuffer("A\n", CharsetUtil.US_ASCII));
+            readIfNeeded(ctx);
         }
 
         @Override
@@ -378,6 +387,7 @@ public class ProxyHandlerTest {
                     // ProxyHandlers in the pipeline.  Therefore, we send the 'B' message only on the first event.
                     ctx.writeAndFlush(Unpooled.copiedBuffer("B\n", CharsetUtil.US_ASCII));
                 }
+                readIfNeeded(ctx);
             }
         }
 
@@ -388,6 +398,7 @@ public class ProxyHandlerTest {
             if ("2".equals(str)) {
                 ctx.writeAndFlush(Unpooled.copiedBuffer("C\n", CharsetUtil.US_ASCII));
             }
+            readIfNeeded(ctx);
         }
 
         @Override
@@ -523,6 +534,7 @@ public class ProxyHandlerTest {
             Bootstrap b = new Bootstrap();
             b.group(group);
             b.channel(NioSocketChannel.class);
+            b.option(ChannelOption.AUTO_READ, ThreadLocalRandom.current().nextBoolean());
             b.resolver(NoopAddressResolverGroup.INSTANCE);
             b.handler(new ChannelInitializer<SocketChannel>() {
                 @Override