Skip to content

Commit 41b0080

Browse files
naude-rnormanmaurer
naude-r
authored and
normanmaurer
committed
Support empty http responses when using compression
Motivation: Found a bug in that netty would generate a 20 byte body when returing a response to an HTTP HEAD. the 20 bytes seems to be related to the compression footer. RFC2616, section 9.4 states that responses to an HTTP HEAD MUST not return a message body in the response. Netty's own client implementation expected an empty response. The extra bytes lead to a 2nd response with an error decoder result: java.lang.IllegalArgumentException: invalid version format: 14 Modifications: Track the HTTP request method. When processing the response we determine if the response is passthru unnchanged. This decision now takes into account the request method and passthru responses related to HTTP HEAD requests. Result: Netty's http client works and better RFC conformance.
1 parent a80f0d6 commit 41b0080

2 files changed

Lines changed: 121 additions & 13 deletions

File tree

‎codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,11 @@ private enum State {
5858
AWAIT_CONTENT
5959
}
6060

61-
private final Queue<String> acceptEncodingQueue = new ArrayDeque<String>();
62-
private String acceptEncoding;
61+
private static final CharSequence ZERO_LENGTH_HEAD = "HEAD";
62+
private static final CharSequence ZERO_LENGTH_CONNECT = "CONNECT";
63+
64+
private final Queue<CharSequence> acceptEncodingQueue = new ArrayDeque<CharSequence>();
65+
private CharSequence acceptEncoding;
6366
private EmbeddedChannel encoder;
6467
private State state = State.AWAIT_HEADERS;
6568

@@ -71,10 +74,18 @@ public boolean acceptOutboundMessage(Object msg) throws Exception {
7174
@Override
7275
protected void decode(ChannelHandlerContext ctx, HttpRequest msg, List<Object> out)
7376
throws Exception {
74-
String acceptedEncoding = msg.headers().get(HttpHeaders.Names.ACCEPT_ENCODING);
77+
CharSequence acceptedEncoding = msg.headers().get(HttpHeaders.Names.ACCEPT_ENCODING);
7578
if (acceptedEncoding == null) {
7679
acceptedEncoding = HttpHeaders.Values.IDENTITY;
7780
}
81+
82+
HttpMethod meth = msg.getMethod();
83+
if (meth == HttpMethod.HEAD) {
84+
acceptedEncoding = ZERO_LENGTH_HEAD;
85+
} else if (meth == HttpMethod.CONNECT) {
86+
acceptedEncoding = ZERO_LENGTH_CONNECT;
87+
}
88+
7889
acceptEncodingQueue.add(acceptedEncoding);
7990
out.add(ReferenceCountUtil.retain(msg));
8091
}
@@ -89,12 +100,24 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
89100

90101
final HttpResponse res = (HttpResponse) msg;
91102

103+
// Get the list of encodings accepted by the peer.
104+
acceptEncoding = acceptEncodingQueue.poll();
105+
if (acceptEncoding == null) {
106+
throw new IllegalStateException("cannot send more responses than requests");
107+
}
108+
92109
/*
93110
* per rfc2616 4.3 Message Body
94111
* All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a
95112
* message-body. All other responses do include a message-body, although it MAY be of zero length.
113+
*
114+
* 9.4 HEAD
115+
* The HEAD method is identical to GET except that the server MUST NOT return a message-body
116+
* in the response.
117+
*
118+
* This code is now inline with HttpClientDecoder.Decoder
96119
*/
97-
if (isPassthru(res)) {
120+
if (isPassthru(res, acceptEncoding)) {
98121
if (isFull) {
99122
out.add(ReferenceCountUtil.retain(res));
100123
} else {
@@ -105,12 +128,6 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
105128
break;
106129
}
107130

108-
// Get the list of encodings accepted by the peer.
109-
acceptEncoding = acceptEncodingQueue.poll();
110-
if (acceptEncoding == null) {
111-
throw new IllegalStateException("cannot send more responses than requests");
112-
}
113-
114131
if (isFull) {
115132
// Pass through the full response with empty content and continue waiting for the the next resp.
116133
if (!((ByteBufHolder) res).content().isReadable()) {
@@ -120,7 +137,7 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
120137
}
121138

122139
// Prepare to encode the content.
123-
final Result result = beginEncode(res, acceptEncoding);
140+
final Result result = beginEncode(res, acceptEncoding.toString());
124141

125142
// If unable to encode, pass through.
126143
if (result == null) {
@@ -181,9 +198,10 @@ protected void encode(ChannelHandlerContext ctx, HttpObject msg, List<Object> ou
181198
}
182199
}
183200

184-
private static boolean isPassthru(HttpResponse res) {
201+
private static boolean isPassthru(HttpResponse res, CharSequence httpMethod) {
185202
final int code = res.getStatus().code();
186-
return code < 200 || code == 204 || code == 304;
203+
boolean expectEmptyBody = httpMethod == ZERO_LENGTH_HEAD || (httpMethod == ZERO_LENGTH_CONNECT && code == 200);
204+
return code < 200 || code == 204 || code == 304 || expectEmptyBody;
187205
}
188206

189207
private static void ensureHeaders(HttpObject msg) {

‎codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,96 @@ public void testEmptyFullContentWithTrailer() throws Exception {
254254
assertThat(ch.readOutbound(), is(nullValue()));
255255
}
256256

257+
@Test
258+
public void testEmptyHeadResponse() throws Exception {
259+
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
260+
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.HEAD, "/");
261+
ch.writeInbound(req);
262+
263+
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
264+
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
265+
ch.writeOutbound(res);
266+
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
267+
268+
assertEmptyResponse(ch);
269+
}
270+
271+
@Test
272+
public void testHttp304Response() throws Exception {
273+
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
274+
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
275+
req.headers().set(Names.ACCEPT_ENCODING, Values.GZIP);
276+
ch.writeInbound(req);
277+
278+
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.NOT_MODIFIED);
279+
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
280+
ch.writeOutbound(res);
281+
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
282+
283+
assertEmptyResponse(ch);
284+
}
285+
286+
@Test
287+
public void testConnect200Response() throws Exception {
288+
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
289+
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.CONNECT, "google.com:80");
290+
ch.writeInbound(req);
291+
292+
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
293+
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
294+
ch.writeOutbound(res);
295+
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
296+
297+
assertEmptyResponse(ch);
298+
}
299+
300+
@Test
301+
public void testConnectFailureResponse() throws Exception {
302+
String content = "Not allowed by configuration";
303+
304+
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
305+
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.CONNECT, "google.com:80");
306+
ch.writeInbound(req);
307+
308+
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.METHOD_NOT_ALLOWED);
309+
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
310+
ch.writeOutbound(res);
311+
ch.writeOutbound(new DefaultHttpContent(Unpooled.wrappedBuffer(content.getBytes(CharsetUtil.UTF_8))));
312+
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
313+
314+
assertEncodedResponse(ch);
315+
Object o = ch.readOutbound();
316+
assertThat(o, is(instanceOf(HttpContent.class)));
317+
HttpContent chunk = (HttpContent) o;
318+
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("28"));
319+
chunk.release();
320+
321+
chunk = (HttpContent) ch.readOutbound();
322+
assertThat(chunk.content().isReadable(), is(true));
323+
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("0"));
324+
chunk.release();
325+
326+
chunk = (HttpContent) ch.readOutbound();
327+
assertThat(chunk, is(instanceOf(LastHttpContent.class)));
328+
chunk.release();
329+
assertThat(ch.readOutbound(), is(nullValue()));
330+
}
331+
332+
private static void assertEmptyResponse(EmbeddedChannel ch) {
333+
Object o = ch.readOutbound();
334+
assertThat(o, is(instanceOf(HttpResponse.class)));
335+
336+
HttpResponse res = (HttpResponse) o;
337+
assertThat(res, is(not(instanceOf(HttpContent.class))));
338+
assertThat(res.headers().get(Names.TRANSFER_ENCODING), is("chunked"));
339+
assertThat(res.headers().get(Names.CONTENT_LENGTH), is(nullValue()));
340+
341+
HttpContent chunk = (HttpContent) ch.readOutbound();
342+
assertThat(chunk, is(instanceOf(LastHttpContent.class)));
343+
chunk.release();
344+
assertThat(ch.readOutbound(), is(nullValue()));
345+
}
346+
257347
private static void assertEncodedResponse(EmbeddedChannel ch) {
258348
Object o = ch.readOutbound();
259349
assertThat(o, is(instanceOf(HttpResponse.class)));

0 commit comments

Comments
 (0)