[#4079] Fix IllegalStateException when HttpContentEncoder is used and 100 Continue response is used.
Motivation:
Whe a 100 Continue response was written an IllegalStateException was produced as soon as the user wrote the following response. This regression was introduced by 41b0080fcc
.
Modifications:
- Special handle 100 Continue responses
- Added unit tests
Result:
Fixed regression.
This commit is contained in:
parent
9a445206ca
commit
16d136dc55
@ -60,6 +60,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
|
|||||||
|
|
||||||
private static final CharSequence ZERO_LENGTH_HEAD = "HEAD";
|
private static final CharSequence ZERO_LENGTH_HEAD = "HEAD";
|
||||||
private static final CharSequence ZERO_LENGTH_CONNECT = "CONNECT";
|
private static final CharSequence ZERO_LENGTH_CONNECT = "CONNECT";
|
||||||
|
private static final int CONTINUE_CODE = HttpResponseStatus.CONTINUE.code();
|
||||||
|
|
||||||
private final Queue<CharSequence> acceptEncodingQueue = new ArrayDeque<CharSequence>();
|
private final Queue<CharSequence> acceptEncodingQueue = new ArrayDeque<CharSequence>();
|
||||||
private CharSequence acceptEncoding;
|
private CharSequence acceptEncoding;
|
||||||
@ -99,12 +100,18 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
|
|||||||
assert encoder == null;
|
assert encoder == null;
|
||||||
|
|
||||||
final HttpResponse res = (HttpResponse) msg;
|
final HttpResponse res = (HttpResponse) msg;
|
||||||
|
final int code = res.getStatus().code();
|
||||||
|
if (code == CONTINUE_CODE) {
|
||||||
|
// We need to not poll the encoding when response with CONTINUE as another response will follow
|
||||||
|
// for the issued request. See https://github.com/netty/netty/issues/4079
|
||||||
|
acceptEncoding = null;
|
||||||
|
} else {
|
||||||
// Get the list of encodings accepted by the peer.
|
// Get the list of encodings accepted by the peer.
|
||||||
acceptEncoding = acceptEncodingQueue.poll();
|
acceptEncoding = acceptEncodingQueue.poll();
|
||||||
if (acceptEncoding == null) {
|
if (acceptEncoding == null) {
|
||||||
throw new IllegalStateException("cannot send more responses than requests");
|
throw new IllegalStateException("cannot send more responses than requests");
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* per rfc2616 4.3 Message Body
|
* per rfc2616 4.3 Message Body
|
||||||
@ -117,7 +124,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
|
|||||||
*
|
*
|
||||||
* This code is now inline with HttpClientDecoder.Decoder
|
* This code is now inline with HttpClientDecoder.Decoder
|
||||||
*/
|
*/
|
||||||
if (isPassthru(res, acceptEncoding)) {
|
if (isPassthru(code, acceptEncoding)) {
|
||||||
if (isFull) {
|
if (isFull) {
|
||||||
out.add(ReferenceCountUtil.retain(res));
|
out.add(ReferenceCountUtil.retain(res));
|
||||||
} else {
|
} else {
|
||||||
@ -198,10 +205,9 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private static boolean isPassthru(HttpResponse res, CharSequence httpMethod) {
|
private static boolean isPassthru(int code, CharSequence httpMethod) {
|
||||||
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);
|
(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) {
|
||||||
|
@ -18,10 +18,12 @@ package io.netty.handler.codec.http;
|
|||||||
import io.netty.buffer.ByteBufUtil;
|
import io.netty.buffer.ByteBufUtil;
|
||||||
import io.netty.buffer.Unpooled;
|
import io.netty.buffer.Unpooled;
|
||||||
import io.netty.channel.embedded.EmbeddedChannel;
|
import io.netty.channel.embedded.EmbeddedChannel;
|
||||||
|
import io.netty.handler.codec.EncoderException;
|
||||||
import io.netty.handler.codec.compression.ZlibWrapper;
|
import io.netty.handler.codec.compression.ZlibWrapper;
|
||||||
import io.netty.handler.codec.http.HttpHeaders.Names;
|
import io.netty.handler.codec.http.HttpHeaders.Names;
|
||||||
import io.netty.handler.codec.http.HttpHeaders.Values;
|
import io.netty.handler.codec.http.HttpHeaders.Values;
|
||||||
import io.netty.util.CharsetUtil;
|
import io.netty.util.CharsetUtil;
|
||||||
|
import io.netty.util.ReferenceCountUtil;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
|
|
||||||
import static org.hamcrest.CoreMatchers.*;
|
import static org.hamcrest.CoreMatchers.*;
|
||||||
@ -292,6 +294,78 @@ public class HttpContentCompressorTest {
|
|||||||
assertThat(ch.readOutbound(), is(nullValue()));
|
assertThat(ch.readOutbound(), is(nullValue()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void test100Continue() throws Exception {
|
||||||
|
FullHttpRequest request = newRequest();
|
||||||
|
HttpHeaders.set100ContinueExpected(request);
|
||||||
|
|
||||||
|
EmbeddedChannel ch = new EmbeddedChannel(new HttpContentCompressor());
|
||||||
|
ch.writeInbound(request);
|
||||||
|
|
||||||
|
FullHttpResponse continueResponse = new DefaultFullHttpResponse(
|
||||||
|
HttpVersion.HTTP_1_1, HttpResponseStatus.CONTINUE, Unpooled.EMPTY_BUFFER);
|
||||||
|
|
||||||
|
ch.writeOutbound(continueResponse);
|
||||||
|
|
||||||
|
FullHttpResponse res = new DefaultFullHttpResponse(
|
||||||
|
HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.EMPTY_BUFFER);
|
||||||
|
res.trailingHeaders().set("X-Test", "Netty");
|
||||||
|
ch.writeOutbound(res);
|
||||||
|
|
||||||
|
Object o = ch.readOutbound();
|
||||||
|
assertThat(o, is(instanceOf(FullHttpResponse.class)));
|
||||||
|
|
||||||
|
res = (FullHttpResponse) o;
|
||||||
|
assertSame(continueResponse, res);
|
||||||
|
res.release();
|
||||||
|
|
||||||
|
o = ch.readOutbound();
|
||||||
|
assertThat(o, is(instanceOf(FullHttpResponse.class)));
|
||||||
|
|
||||||
|
res = (FullHttpResponse) o;
|
||||||
|
assertThat(res.headers().get(Names.TRANSFER_ENCODING), is(nullValue()));
|
||||||
|
|
||||||
|
// Content encoding shouldn't be modified.
|
||||||
|
assertThat(res.headers().get(Names.CONTENT_ENCODING), is(nullValue()));
|
||||||
|
assertThat(res.content().readableBytes(), is(0));
|
||||||
|
assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
|
||||||
|
assertEquals("Netty", res.trailingHeaders().get("X-Test"));
|
||||||
|
assertThat(ch.readOutbound(), is(nullValue()));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testTooManyResponses() throws Exception {
|
||||||
|
FullHttpRequest request = newRequest();
|
||||||
|
EmbeddedChannel ch = new EmbeddedChannel(new HttpContentCompressor());
|
||||||
|
ch.writeInbound(request);
|
||||||
|
|
||||||
|
ch.writeOutbound(new DefaultFullHttpResponse(
|
||||||
|
HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.EMPTY_BUFFER));
|
||||||
|
|
||||||
|
try {
|
||||||
|
ch.writeOutbound(new DefaultFullHttpResponse(
|
||||||
|
HttpVersion.HTTP_1_1, HttpResponseStatus.OK, Unpooled.EMPTY_BUFFER));
|
||||||
|
fail();
|
||||||
|
} catch (EncoderException e) {
|
||||||
|
assertTrue(e.getCause() instanceof IllegalStateException);
|
||||||
|
}
|
||||||
|
assertTrue(ch.finish());
|
||||||
|
for (;;) {
|
||||||
|
Object message = ch.readOutbound();
|
||||||
|
if (message == null) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
ReferenceCountUtil.release(message);
|
||||||
|
}
|
||||||
|
for (;;) {
|
||||||
|
Object message = ch.readInbound();
|
||||||
|
if (message == null) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
ReferenceCountUtil.release(message);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static FullHttpRequest newRequest() {
|
private static FullHttpRequest newRequest() {
|
||||||
FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
|
FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/");
|
||||||
req.headers().set(Names.ACCEPT_ENCODING, "gzip");
|
req.headers().set(Names.ACCEPT_ENCODING, "gzip");
|
||||||
|
Loading…
Reference in New Issue
Block a user