andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Support empty http responses when using compression

Browse Source 
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.
This commit is contained in:
Roelof Naude 2015-05-07 08:53:34 +02:00 committed by Norman Maurer
parent a80f0d69a7
commit 41b0080fcc
2 changed files with 121 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java
View File

@ -58,8 +58,11 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
AWAIT_CONTENT AWAIT_CONTENT
} }
private final Queue<String> acceptEncodingQueue = new ArrayDeque<String>(); private static final CharSequence ZERO_LENGTH_HEAD = "HEAD";
private String acceptEncoding; private static final CharSequence ZERO_LENGTH_CONNECT = "CONNECT";
private final Queue<CharSequence> acceptEncodingQueue = new ArrayDeque<CharSequence>();
private CharSequence acceptEncoding;
private EmbeddedChannel encoder; private EmbeddedChannel encoder;
private State state = State.AWAIT_HEADERS; private State state = State.AWAIT_HEADERS;
@ -71,10 +74,18 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
@Override @Override
protected void decode(ChannelHandlerContext ctx, HttpRequest msg, List<Object> out) protected void decode(ChannelHandlerContext ctx, HttpRequest msg, List<Object> out)
throws Exception { throws Exception {
String acceptedEncoding = msg.headers().get(HttpHeaders.Names.ACCEPT_ENCODING); CharSequence acceptedEncoding = msg.headers().get(HttpHeaders.Names.ACCEPT_ENCODING);
if (acceptedEncoding == null) { if (acceptedEncoding == null) {
acceptedEncoding = HttpHeaders.Values.IDENTITY; acceptedEncoding = HttpHeaders.Values.IDENTITY;
} }
HttpMethod meth = msg.getMethod();
if (meth == HttpMethod.HEAD) {
acceptedEncoding = ZERO_LENGTH_HEAD;
} else if (meth == HttpMethod.CONNECT) {
acceptedEncoding = ZERO_LENGTH_CONNECT;
}
acceptEncodingQueue.add(acceptedEncoding); acceptEncodingQueue.add(acceptedEncoding);
out.add(ReferenceCountUtil.retain(msg)); out.add(ReferenceCountUtil.retain(msg));
} }
@ -89,12 +100,24 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
final HttpResponse res = (HttpResponse) msg; final HttpResponse res = (HttpResponse) msg;
// Get the list of encodings accepted by the peer.
acceptEncoding = acceptEncodingQueue.poll();
if (acceptEncoding == null) {
throw new IllegalStateException("cannot send more responses than requests");
}
/* /*
* per rfc2616 4.3 Message Body * per rfc2616 4.3 Message Body
* All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a * All 1xx (informational), 204 (no content), and 304 (not modified) responses MUST NOT include a
* message-body. All other responses do include a message-body, although it MAY be of zero length. * message-body. All other responses do include a message-body, although it MAY be of zero length.
*
* 9.4 HEAD
* The HEAD method is identical to GET except that the server MUST NOT return a message-body
* in the response.
*
* This code is now inline with HttpClientDecoder.Decoder
*/ */
if (isPassthru(res)) { if (isPassthru(res, acceptEncoding)) {
if (isFull) { if (isFull) {
out.add(ReferenceCountUtil.retain(res)); out.add(ReferenceCountUtil.retain(res));
} else { } else {
@ -105,12 +128,6 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
break; break;
} }
// Get the list of encodings accepted by the peer.
acceptEncoding = acceptEncodingQueue.poll();
if (acceptEncoding == null) {
throw new IllegalStateException("cannot send more responses than requests");
}
if (isFull) { if (isFull) {
// Pass through the full response with empty content and continue waiting for the the next resp. // Pass through the full response with empty content and continue waiting for the the next resp.
if (!((ByteBufHolder) res).content().isReadable()) { if (!((ByteBufHolder) res).content().isReadable()) {
@ -120,7 +137,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
} }
// Prepare to encode the content. // Prepare to encode the content.
final Result result = beginEncode(res, acceptEncoding); final Result result = beginEncode(res, acceptEncoding.toString());
// If unable to encode, pass through. // If unable to encode, pass through.
if (result == null) { if (result == null) {
@ -181,9 +198,10 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
} }
} }
private static boolean isPassthru(HttpResponse res) { private static boolean isPassthru(HttpResponse res, CharSequence httpMethod) {
final int code = res.getStatus().code(); final int code = res.getStatus().code();
return code < 200 || code == 204 || code == 304; boolean expectEmptyBody = httpMethod == ZERO_LENGTH_HEAD || (httpMethod == ZERO_LENGTH_CONNECT && code == 200);
return code < 200 || code == 204 || code == 304 || expectEmptyBody;
} }
private static void ensureHeaders(HttpObject msg) { private static void ensureHeaders(HttpObject msg) {

90
codec-http/src/test/java/io/netty/handler/codec/http/HttpContentEncoderTest.java
View File

@ -254,6 +254,96 @@ public class HttpContentEncoderTest {
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
} }
@Test
public void testEmptyHeadResponse() throws Exception {
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.HEAD, "/");
ch.writeInbound(req);
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
ch.writeOutbound(res);
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
assertEmptyResponse(ch);
}
@Test
public void testHttp304Response() throws Exception {
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
req.headers().set(Names.ACCEPT_ENCODING, Values.GZIP);
ch.writeInbound(req);
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.NOT_MODIFIED);
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
ch.writeOutbound(res);
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
assertEmptyResponse(ch);
}
@Test
public void testConnect200Response() throws Exception {
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.CONNECT, "google.com:80");
ch.writeInbound(req);
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
ch.writeOutbound(res);
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
assertEmptyResponse(ch);
}
@Test
public void testConnectFailureResponse() throws Exception {
String content = "Not allowed by configuration";
EmbeddedChannel ch = new EmbeddedChannel(new TestEncoder());
HttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.CONNECT, "google.com:80");
ch.writeInbound(req);
HttpResponse res = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.METHOD_NOT_ALLOWED);
res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED);
ch.writeOutbound(res);
ch.writeOutbound(new DefaultHttpContent(Unpooled.wrappedBuffer(content.getBytes(CharsetUtil.UTF_8))));
ch.writeOutbound(LastHttpContent.EMPTY_LAST_CONTENT);
assertEncodedResponse(ch);
Object o = ch.readOutbound();
assertThat(o, is(instanceOf(HttpContent.class)));
HttpContent chunk = (HttpContent) o;
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("28"));
chunk.release();
chunk = (HttpContent) ch.readOutbound();
assertThat(chunk.content().isReadable(), is(true));
assertThat(chunk.content().toString(CharsetUtil.US_ASCII), is("0"));
chunk.release();
chunk = (HttpContent) ch.readOutbound();
assertThat(chunk, is(instanceOf(LastHttpContent.class)));
chunk.release();
assertThat(ch.readOutbound(), is(nullValue()));
}
private static void assertEmptyResponse(EmbeddedChannel ch) {
Object o = ch.readOutbound();
assertThat(o, is(instanceOf(HttpResponse.class)));
HttpResponse res = (HttpResponse) o;
assertThat(res, is(not(instanceOf(HttpContent.class))));
assertThat(res.headers().get(Names.TRANSFER_ENCODING), is("chunked"));
assertThat(res.headers().get(Names.CONTENT_LENGTH), is(nullValue()));
HttpContent chunk = (HttpContent) ch.readOutbound();
assertThat(chunk, is(instanceOf(LastHttpContent.class)));
chunk.release();
assertThat(ch.readOutbound(), is(nullValue()));
}
private static void assertEncodedResponse(EmbeddedChannel ch) { private static void assertEncodedResponse(EmbeddedChannel ch) {
Object o = ch.readOutbound(); Object o = ch.readOutbound();
assertThat(o, is(instanceOf(HttpResponse.class))); assertThat(o, is(instanceOf(HttpResponse.class)));