Set result for decoded request and add test for #8721 (#8721)

Motivation:
I want to fix bug in vert.x project (eclipse-vertx/vert.x#2562) caused by ComposedLastHttpContent result being null. I don't know if it is intentional that this last decoded chuck in the issue returns null, but if not - I am providing fix for that.

Modification:
* Added new constructor in ComposedLastHttpContent allowing to pass DecoderResult
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentEncoder
* set DecoderResult.SUCCESS for created ComposedLastHttpContent in HttpContentDecoder

Result:
Fixes eclipse-vertx/vert.x#2562
This commit is contained in:
Bartek Kowalczyk 2019-01-21 07:45:03 +01:00 committed by Norman Maurer
parent 7a613c2131
commit 8bcea35d2e
6 changed files with 58 additions and 2 deletions

View File

@ -28,6 +28,11 @@ final class ComposedLastHttpContent implements LastHttpContent {
this.trailingHeaders = trailingHeaders; this.trailingHeaders = trailingHeaders;
} }
ComposedLastHttpContent(HttpHeaders trailingHeaders, DecoderResult result) {
this(trailingHeaders);
this.result = result;
}
@Override @Override
public HttpHeaders trailingHeaders() { public HttpHeaders trailingHeaders() {
return trailingHeaders; return trailingHeaders;

View File

@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.CodecException; import io.netty.handler.codec.CodecException;
import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.MessageToMessageDecoder; import io.netty.handler.codec.MessageToMessageDecoder;
import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCountUtil;
@ -164,7 +165,7 @@ public abstract class HttpContentDecoder extends MessageToMessageDecoder<HttpObj
if (headers.isEmpty()) { if (headers.isEmpty()) {
out.add(LastHttpContent.EMPTY_LAST_CONTENT); out.add(LastHttpContent.EMPTY_LAST_CONTENT);
} else { } else {
out.add(new ComposedLastHttpContent(headers)); out.add(new ComposedLastHttpContent(headers, DecoderResult.SUCCESS));
} }
} }
} }

View File

@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufHolder; import io.netty.buffer.ByteBufHolder;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.MessageToMessageCodec; import io.netty.handler.codec.MessageToMessageCodec;
import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCountUtil;
@ -264,7 +265,7 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec<HttpReque
if (headers.isEmpty()) { if (headers.isEmpty()) {
out.add(LastHttpContent.EMPTY_LAST_CONTENT); out.add(LastHttpContent.EMPTY_LAST_CONTENT);
} else { } else {
out.add(new ComposedLastHttpContent(headers)); out.add(new ComposedLastHttpContent(headers, DecoderResult.SUCCESS));
} }
return true; return true;
} }

View File

@ -18,6 +18,7 @@ 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.DecoderResult;
import io.netty.handler.codec.EncoderException; import io.netty.handler.codec.EncoderException;
import io.netty.handler.codec.compression.ZlibWrapper; import io.netty.handler.codec.compression.ZlibWrapper;
import io.netty.util.CharsetUtil; import io.netty.util.CharsetUtil;
@ -188,6 +189,7 @@ public class HttpContentCompressorTest {
assertThat(chunk.content().isReadable(), is(false)); assertThat(chunk.content().isReadable(), is(false));
assertThat(chunk, is(instanceOf(LastHttpContent.class))); assertThat(chunk, is(instanceOf(LastHttpContent.class)));
assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test"))); assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test")));
assertEquals(DecoderResult.SUCCESS, chunk.decoderResult());
chunk.release(); chunk.release();
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
@ -331,6 +333,7 @@ public class HttpContentCompressorTest {
assertThat(res.content().readableBytes(), is(0)); assertThat(res.content().readableBytes(), is(0));
assertThat(res.content().toString(CharsetUtil.US_ASCII), is("")); assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
assertEquals("Netty", res.trailingHeaders().get(of("X-Test"))); assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
assertEquals(DecoderResult.SUCCESS, res.decoderResult());
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
} }
@ -370,6 +373,7 @@ public class HttpContentCompressorTest {
assertThat(res.content().readableBytes(), is(0)); assertThat(res.content().readableBytes(), is(0));
assertThat(res.content().toString(CharsetUtil.US_ASCII), is("")); assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
assertEquals("Netty", res.trailingHeaders().get(of("X-Test"))); assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
assertEquals(DecoderResult.SUCCESS, res.decoderResult());
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
} }

View File

@ -89,6 +89,48 @@ public class HttpContentDecoderTest {
assertFalse(channel.finish()); // assert that no messages are left in channel assertFalse(channel.finish()); // assert that no messages are left in channel
} }
@Test
public void testChunkedRequestDecompression() {
HttpResponseDecoder decoder = new HttpResponseDecoder();
HttpContentDecoder decompressor = new HttpContentDecompressor();
EmbeddedChannel channel = new EmbeddedChannel(decoder, decompressor, null);
String headers = "HTTP/1.1 200 OK\r\n"
+ "Transfer-Encoding: chunked\r\n"
+ "Trailer: My-Trailer\r\n"
+ "Content-Encoding: gzip\r\n\r\n";
channel.writeInbound(Unpooled.copiedBuffer(headers.getBytes(CharsetUtil.US_ASCII)));
String chunkLength = Integer.toHexString(GZ_HELLO_WORLD.length);
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(chunkLength + "\r\n", CharsetUtil.US_ASCII)));
assertTrue(channel.writeInbound(Unpooled.copiedBuffer(GZ_HELLO_WORLD)));
assertTrue(channel.writeInbound(Unpooled.copiedBuffer("\r\n".getBytes(CharsetUtil.US_ASCII))));
assertTrue(channel.writeInbound(Unpooled.copiedBuffer("0\r\n", CharsetUtil.US_ASCII)));
assertTrue(channel.writeInbound(Unpooled.copiedBuffer("My-Trailer: 42\r\n\r\n\r\n", CharsetUtil.US_ASCII)));
Object ob1 = channel.readInbound();
assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
Object ob2 = channel.readInbound();
assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
HttpContent content = (HttpContent) ob2;
assertEquals(HELLO_WORLD, content.content().toString(CharsetUtil.US_ASCII));
content.release();
Object ob3 = channel.readInbound();
assertThat(ob1, is(instanceOf(DefaultHttpResponse.class)));
LastHttpContent lastContent = (LastHttpContent) ob3;
assertNotNull(lastContent.decoderResult());
assertTrue(lastContent.decoderResult().isSuccess());
assertFalse(lastContent.trailingHeaders().isEmpty());
assertEquals("42", lastContent.trailingHeaders().get("My-Trailer"));
assertHasInboundMessages(channel, false);
assertHasOutboundMessages(channel, false);
assertFalse(channel.finish());
}
@Test @Test
public void testResponseDecompression() { public void testResponseDecompression() {
// baseline test: response decoder, content decompressor && request aggregator work as expected // baseline test: response decoder, content decompressor && request aggregator work as expected

View File

@ -22,6 +22,7 @@ import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.embedded.EmbeddedChannel; import io.netty.channel.embedded.EmbeddedChannel;
import io.netty.handler.codec.CodecException; import io.netty.handler.codec.CodecException;
import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.EncoderException; import io.netty.handler.codec.EncoderException;
import io.netty.handler.codec.MessageToByteEncoder; import io.netty.handler.codec.MessageToByteEncoder;
import io.netty.util.CharsetUtil; import io.netty.util.CharsetUtil;
@ -156,6 +157,7 @@ public class HttpContentEncoderTest {
assertThat(chunk.content().isReadable(), is(false)); assertThat(chunk.content().isReadable(), is(false));
assertThat(chunk, is(instanceOf(LastHttpContent.class))); assertThat(chunk, is(instanceOf(LastHttpContent.class)));
assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test"))); assertEquals("Netty", ((LastHttpContent) chunk).trailingHeaders().get(of("X-Test")));
assertEquals(DecoderResult.SUCCESS, res.decoderResult());
chunk.release(); chunk.release();
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
@ -285,6 +287,7 @@ public class HttpContentEncoderTest {
assertThat(res.content().readableBytes(), is(0)); assertThat(res.content().readableBytes(), is(0));
assertThat(res.content().toString(CharsetUtil.US_ASCII), is("")); assertThat(res.content().toString(CharsetUtil.US_ASCII), is(""));
assertEquals("Netty", res.trailingHeaders().get(of("X-Test"))); assertEquals("Netty", res.trailingHeaders().get(of("X-Test")));
assertEquals(DecoderResult.SUCCESS, res.decoderResult());
assertThat(ch.readOutbound(), is(nullValue())); assertThat(ch.readOutbound(), is(nullValue()));
} }