Skip to content

Commit

Permalink
Fix a bug where HttpObjectDecoder generates two LastHttpContent conse…
Browse files Browse the repository at this point in the history
…cutively

Motivation:
When an HttpResponseDecoder decodes an invalid chunk, a LastHttpContent instance is produced and the decoder enters the 'BAD_MESSAGE' state, which is not supposed to produce a message any further.  However, because HttpObjectDecoder.invalidChunk() did not clear this.message out to null, decodeLast() will produce another LastHttpContent message on a certain situation.

Modification:
Do not forget to null out HttpObjectDecoder.message in invalidChunk(), and add a test case for it.

Result:
No more consecutive LastHttpContent messages produced by HttpObjectDecoder.
  • Loading branch information
trustin committed Feb 26, 2014
1 parent 3f53ba2 commit c425705
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ private HttpContent invalidChunk(Exception cause) {
checkpoint(State.BAD_MESSAGE);
HttpContent chunk = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER);
chunk.setDecoderResult(DecoderResult.failure(cause));
message = null;
return chunk;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,4 +536,32 @@ public void testGarbageHeaders() {
ch.finish();
assertThat(ch.readInbound(), is(nullValue()));
}


/**
* Tests if the decoder produces one and only {@link LastHttpContent} when an invalid chunk is received and
* the connection is closed.
*/
@Test
public void testGarbageChunk() {
EmbeddedChannel channel = new EmbeddedChannel(new HttpResponseDecoder());
String responseWithIllegalChunk =
"HTTP/1.1 200 OK\r\n" +
"Transfer-Encoding: chunked\r\n\r\n" +
"NOT_A_CHUNK_LENGTH\r\n";

channel.writeInbound(Unpooled.copiedBuffer(responseWithIllegalChunk, CharsetUtil.US_ASCII));
assertThat(channel.readInbound(), is(instanceOf(HttpResponse.class)));

// Ensure that the decoder generates the last chunk with correct decoder result.
LastHttpContent invalidChunk = channel.readInbound();
assertThat(invalidChunk.getDecoderResult().isFailure(), is(true));
invalidChunk.release();

// And no more messages should be produced by the decoder.
assertThat(channel.readInbound(), is(nullValue()));

// .. even after the connection is closed.
assertThat(channel.finish(), is(false));
}
}

0 comments on commit c425705

Please sign in to comment.