Fix a bug where HttpObjectDecoder produces LastHttpContent after entering BAD_MESSAGE state

trustin · trustin · commit 0244e35fa688 · 2014-01-10T16:16:02.000+09:00
- Fixes netty#2103 - Added LastHttpContent.EMPTY_LAST_CONTENT.toString() for athestic reasons.
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectDecoder.java
@@ -438,7 +438,10 @@ private HttpMessage invalidMessage(Exception cause) {
             message = createInvalidMessage();
             message.setDecoderResult(DecoderResult.failure(cause));
         }
-        return message;
+
+        HttpMessage ret = message;
+        message = null;
+        return ret;
     }
 
     private HttpContent invalidChunk(Exception cause) {
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/LastHttpContent.java b/codec-http/src/main/java/io/netty/handler/codec/http/LastHttpContent.java
@@ -83,6 +83,11 @@ public boolean release() {
         public boolean release(int decrement) {
             return false;
         }
+
+        @Override
+        public String toString() {
+            return "EmptyLastHttpContent";
+        }
     };
 
     HttpHeaders trailingHeaders();
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpResponseDecoderTest.java
@@ -478,4 +478,36 @@ public void testWebSocketResponse() {
 
         assertThat(ch.readInbound(), is(nullValue()));
     }
+
+    @Test
+    public void testGarbageHeaders() {
+        // A response without headers - from https://github.com/netty/netty/issues/2103
+        byte[] data = ("<html>\r\n" +
+                "<head><title>400 Bad Request</title></head>\r\n" +
+                "<body bgcolor=\"white\">\r\n" +
+                "<center><h1>400 Bad Request</h1></center>\r\n" +
+                "<hr><center>nginx/1.1.19</center>\r\n" +
+                "</body>\r\n" +
+                "</html>\r\n").getBytes();
+
+        EmbeddedChannel ch = new EmbeddedChannel(new HttpResponseDecoder());
+
+        ch.writeInbound(Unpooled.wrappedBuffer(data));
+
+        // Garbage input should generate the 999 Unknown response.
+        HttpResponse res = ch.readInbound();
+        assertThat(res.getProtocolVersion(), sameInstance(HttpVersion.HTTP_1_0));
+        assertThat(res.getStatus().code(), is(999));
+        assertThat(res.getDecoderResult().isFailure(), is(true));
+        assertThat(res.getDecoderResult().isFinished(), is(true));
+        assertThat(ch.readInbound(), is(nullValue()));
+
+        // More garbage should not generate anything (i.e. the decoder discards anything beyond this point.)
+        ch.writeInbound(Unpooled.wrappedBuffer(data));
+        assertThat(ch.readInbound(), is(nullValue()));
+
+        // Closing the connection should not generate anything since the protocol has been violated.
+        ch.finish();
+        assertThat(ch.readInbound(), is(nullValue()));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -438,7 +438,10 @@ private HttpMessage invalidMessage(Exception cause) {`
`438`	`438`	`message = createInvalidMessage();`
`439`	`439`	`message.setDecoderResult(DecoderResult.failure(cause));`
`440`	`440`	`}`
`441`		`- return message;`
	`441`	`+`
	`442`	`+ HttpMessage ret = message;`
	`443`	`+ message = null;`
	`444`	`+ return ret;`
`442`	`445`	`}`
`443`	`446`
`444`	`447`	`private HttpContent invalidChunk(Exception cause) {`