HttpObjectDecoder ignores HTTP trailer header when empty line is rece… (#8799)

* HttpObjectDecoder ignores HTTP trailer header when empty line is received in seperate ByteBuf

Motivation:

When the empty line that termines the trailers was sent in a seperate ByteBuf we did ignore the previous parsed trailers and just returned none.

Modifications:

- Correct respect previous parsed trailers.
- Add unit test.

Result:

Fixes https://github.com/netty/netty/issues/8736
This commit is contained in:
Norman Maurer 2019-01-31 20:27:47 +01:00
parent ab0c33adff
commit 9725cae033
2 changed files with 71 additions and 41 deletions

View File

@ -633,13 +633,18 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
if (line == null) { if (line == null) {
return null; return null;
} }
CharSequence lastHeader = null;
if (line.length() > 0) {
LastHttpContent trailer = this.trailer; LastHttpContent trailer = this.trailer;
if (line.length() == 0 && trailer == null) {
// We have received the empty line which signals the trailer is complete and did not parse any trailers
// before. Just return an empty last content to reduce allocations.
return LastHttpContent.EMPTY_LAST_CONTENT;
}
CharSequence lastHeader = null;
if (trailer == null) { if (trailer == null) {
trailer = this.trailer = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, validateHeaders); trailer = this.trailer = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, validateHeaders);
} }
do { while (line.length() > 0) {
char firstChar = line.charAt(0); char firstChar = line.charAt(0);
if (lastHeader != null && (firstChar == ' ' || firstChar == '\t')) { if (lastHeader != null && (firstChar == ' ' || firstChar == '\t')) {
List<String> current = trailer.trailingHeaders().getAll(lastHeader); List<String> current = trailer.trailingHeaders().getAll(lastHeader);
@ -664,20 +669,16 @@ public abstract class HttpObjectDecoder extends ByteToMessageDecoder {
name = null; name = null;
value = null; value = null;
} }
line = headerParser.parse(buffer); line = headerParser.parse(buffer);
if (line == null) { if (line == null) {
return null; return null;
} }
} while (line.length() > 0); }
this.trailer = null; this.trailer = null;
return trailer; return trailer;
} }
return LastHttpContent.EMPTY_LAST_CONTENT;
}
protected abstract boolean isDecodingRequest(); protected abstract boolean isDecodingRequest();
protected abstract HttpMessage createMessage(String[] initialLine) throws Exception; protected abstract HttpMessage createMessage(String[] initialLine) throws Exception;
protected abstract HttpMessage createInvalidMessage(); protected abstract HttpMessage createInvalidMessage();

View File

@ -634,4 +634,33 @@ public class HttpResponseDecoderTest {
assertThat(message.decoderResult().cause(), instanceOf(PrematureChannelClosureException.class)); assertThat(message.decoderResult().cause(), instanceOf(PrematureChannelClosureException.class));
assertNull(channel.readInbound()); assertNull(channel.readInbound());
} }
@Test
public void testTrailerWithEmptyLineInSeparateBuffer() {
HttpResponseDecoder decoder = new HttpResponseDecoder();
EmbeddedChannel channel = new EmbeddedChannel(decoder);
String headers = "HTTP/1.1 200 OK\r\n"
+ "Transfer-Encoding: chunked\r\n"
+ "Trailer: My-Trailer\r\n";
assertFalse(channel.writeInbound(Unpooled.copiedBuffer(headers.getBytes(CharsetUtil.US_ASCII))));
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", CharsetUtil.US_ASCII)));
assertTrue(channel.writeInbound(Unpooled.copiedBuffer("\r\n", CharsetUtil.US_ASCII)));
HttpResponse response = channel.readInbound();
assertEquals(2, response.headers().size());
assertEquals("chunked", response.headers().get(HttpHeaderNames.TRANSFER_ENCODING));
assertEquals("My-Trailer", response.headers().get(HttpHeaderNames.TRAILER));
LastHttpContent lastContent = channel.readInbound();
assertEquals(1, lastContent.trailingHeaders().size());
assertEquals("42", lastContent.trailingHeaders().get("My-Trailer"));
assertEquals(0, lastContent.content().readableBytes());
lastContent.release();
assertFalse(channel.finish());
}
} }