From a093b89bfef7efe8a1c8ad1d3ab07a7c17ac59c4 Mon Sep 17 00:00:00 2001 From: Nitesh Kant Date: Mon, 8 May 2017 13:45:13 -0700 Subject: [PATCH] Allow HTTP decoding post CONNECT in `HttpClientCode` __Motivation__ `HttpClientCodec` skips HTTP decoding on the connection after a successful HTTP CONNECT response is received. This behavior follows the spec for a client but pragmatically, if one creates a client to use a proxy transparently, the codec becomes useless after HTTP CONNECT. Ideally, one should be able to configure whether HTTP CONNECT should result in pass-through or not. This will enable client writers to continue using HTTP decoding even after HTTP CONNECT. __Modification__ Added overloaded constructors to accept `parseHttpPostConnect`. If this parameter is `true` then the codec continues decoding even after a successful HTTP CONNECT. Also fixed a bug in the codec that was incrementing request count post HTTP CONNECT but not decrementing it on response. Now, the request count is only incremented if the codec is not `done`. __Result__ Easier usage by HTTP client writers who wants to connect to a proxy but still decode HTTP for their users for subsequent requests. --- .../handler/codec/http/HttpClientCodec.java | 33 ++++- .../codec/http/HttpClientCodecTest.java | 117 +++++++++++++----- 2 files changed, 112 insertions(+), 38 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java index 5f90033a03..da4c440466 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpClientCodec.java @@ -47,6 +47,7 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler queue = new ArrayDeque(); + private final boolean parseHttpAfterConnectRequest; /** If true, decoding stops (i.e. pass-through) */ private boolean done; @@ -84,8 +85,18 @@ public final class HttpClientCodec extends CombinedChannelDuplexHandler\r\n"; @@ -60,28 +63,12 @@ public class HttpClientCodecTest { private static final String CHUNKED_RESPONSE = INCOMPLETE_CHUNKED_RESPONSE + "\r\n"; @Test - public void testFailsNotOnRequestResponse() { + public void testConnectWithResponseContent() { HttpClientCodec codec = new HttpClientCodec(4096, 8192, 8192, true); EmbeddedChannel ch = new EmbeddedChannel(codec); - ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/")); - ch.writeInbound(Unpooled.copiedBuffer(RESPONSE, CharsetUtil.ISO_8859_1)); + sendRequestAndReadResponse(ch, HttpMethod.CONNECT, RESPONSE); ch.finish(); - - for (;;) { - Object msg = ch.readOutbound(); - if (msg == null) { - break; - } - release(msg); - } - for (;;) { - Object msg = ch.readInbound(); - if (msg == null) { - break; - } - release(msg); - } } @Test @@ -89,23 +76,8 @@ public class HttpClientCodecTest { HttpClientCodec codec = new HttpClientCodec(4096, 8192, 8192, true); EmbeddedChannel ch = new EmbeddedChannel(codec); - ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "http://localhost/")); - ch.writeInbound(Unpooled.copiedBuffer(CHUNKED_RESPONSE, CharsetUtil.ISO_8859_1)); + sendRequestAndReadResponse(ch, HttpMethod.GET, CHUNKED_RESPONSE); ch.finish(); - for (;;) { - Object msg = ch.readOutbound(); - if (msg == null) { - break; - } - release(msg); - } - for (;;) { - Object msg = ch.readInbound(); - if (msg == null) { - break; - } - release(msg); - } } @Test @@ -233,4 +205,81 @@ public class HttpClientCodecTest { cb.config().group().shutdownGracefully(); } } + + @Test + public void testContinueParsingAfterConnect() throws Exception { + testAfterConnect(true); + } + + @Test + public void testPassThroughAfterConnect() throws Exception { + testAfterConnect(false); + } + + private static void testAfterConnect(final boolean parseAfterConnect) throws Exception { + EmbeddedChannel ch = new EmbeddedChannel(new HttpClientCodec(4096, 8192, 8192, true, true, parseAfterConnect)); + + Consumer connectResponseConsumer = new Consumer(); + sendRequestAndReadResponse(ch, HttpMethod.CONNECT, EMPTY_RESPONSE, connectResponseConsumer); + assertTrue("No connect response messages received.", connectResponseConsumer.getReceivedCount() > 0); + Consumer responseConsumer = new Consumer() { + @Override + void accept(Object object) { + if (parseAfterConnect) { + assertThat("Unexpected response message type.", object, instanceOf(HttpObject.class)); + } else { + assertThat("Unexpected response message type.", object, not(instanceOf(HttpObject.class))); + } + } + }; + sendRequestAndReadResponse(ch, HttpMethod.GET, RESPONSE, responseConsumer); + assertTrue("No response messages received.", responseConsumer.getReceivedCount() > 0); + assertFalse("Channel finish failed.", ch.finish()); + } + + private static void sendRequestAndReadResponse(EmbeddedChannel ch, HttpMethod httpMethod, String response) { + sendRequestAndReadResponse(ch, httpMethod, response, new Consumer()); + } + + private static void sendRequestAndReadResponse(EmbeddedChannel ch, HttpMethod httpMethod, String response, + Consumer responseConsumer) { + assertTrue("Channel outbound write failed.", + ch.writeOutbound(new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, httpMethod, "http://localhost/"))); + assertTrue("Channel inbound write failed.", + ch.writeInbound(Unpooled.copiedBuffer(response, CharsetUtil.ISO_8859_1))); + + for (;;) { + Object msg = ch.readOutbound(); + if (msg == null) { + break; + } + release(msg); + } + for (;;) { + Object msg = ch.readInbound(); + if (msg == null) { + break; + } + responseConsumer.onResponse(msg); + release(msg); + } + } + + private static class Consumer { + + private int receivedCount; + + final void onResponse(Object object) { + receivedCount++; + accept(object); + } + + void accept(Object object) { + // Default noop. + } + + int getReceivedCount() { + return receivedCount; + } + } }