From a00f9f0b708664140cbeb54dd7c12a3edc9d66a0 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 30 Apr 2013 20:54:07 +0200 Subject: [PATCH] [#1322] Correctly handle the removal of Transfer-Encoding: chunked in HttpChunkAggregator --- .../codec/http/HttpChunkAggregator.java | 6 +---- .../handler/codec/http/HttpCodecUtil.java | 18 +++++++++++-- .../codec/http/HttpChunkAggregatorTest.java | 27 +++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpChunkAggregator.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpChunkAggregator.java index 66c7a45012..f3ae6eea75 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpChunkAggregator.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpChunkAggregator.java @@ -135,11 +135,7 @@ public class HttpChunkAggregator extends SimpleChannelUpstreamHandler implements if (m.isChunked()) { // A chunked message - remove 'Transfer-Encoding' header, // initialize the cumulative buffer, and wait for incoming chunks. - List encodings = m.getHeaders(HttpHeaders.Names.TRANSFER_ENCODING); - encodings.remove(HttpHeaders.Values.CHUNKED); - if (encodings.isEmpty()) { - m.removeHeader(HttpHeaders.Names.TRANSFER_ENCODING); - } + HttpCodecUtil.removeTransferEncodingChunked(m); m.setChunked(false); this.currentMessage = m; } else { diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpCodecUtil.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpCodecUtil.java index c3e6c3fbfc..109413aeff 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpCodecUtil.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpCodecUtil.java @@ -15,6 +15,7 @@ */ package org.jboss.netty.handler.codec.http; +import java.util.Iterator; import java.util.List; final class HttpCodecUtil { @@ -120,8 +121,21 @@ final class HttpCodecUtil { static void removeTransferEncodingChunked(HttpMessage m) { List values = m.getHeaders(HttpHeaders.Names.TRANSFER_ENCODING); - values.remove(HttpHeaders.Values.CHUNKED); - m.setHeader(HttpHeaders.Names.TRANSFER_ENCODING, values); + if (values.isEmpty()) { + return; + } + Iterator valuesIt = values.iterator(); + while (valuesIt.hasNext()) { + String value = valuesIt.next(); + if (value.equalsIgnoreCase(HttpHeaders.Values.CHUNKED)) { + valuesIt.remove(); + } + } + if (values.isEmpty()) { + m.removeHeader(HttpHeaders.Names.TRANSFER_ENCODING); + } else { + m.setHeader(HttpHeaders.Names.TRANSFER_ENCODING, values); + } } static boolean isContentLengthSet(HttpMessage m) { diff --git a/src/test/java/org/jboss/netty/handler/codec/http/HttpChunkAggregatorTest.java b/src/test/java/org/jboss/netty/handler/codec/http/HttpChunkAggregatorTest.java index 1697b49263..8560262784 100644 --- a/src/test/java/org/jboss/netty/handler/codec/http/HttpChunkAggregatorTest.java +++ b/src/test/java/org/jboss/netty/handler/codec/http/HttpChunkAggregatorTest.java @@ -139,4 +139,31 @@ public class HttpChunkAggregatorTest { aggr.beforeAdd(ctx); aggr.setMaxCumulationBufferComponents(10); } + + @Test + public void testAggregateTransferEncodingChunked() { + HttpChunkAggregator aggr = new HttpChunkAggregator(1024 * 1024); + DecoderEmbedder embedder = new DecoderEmbedder(aggr); + HttpMessage message = new DefaultHttpMessage(HttpVersion.HTTP_1_1); + HttpHeaders.setHeader(message, "X-Test", true); + HttpHeaders.setHeader(message, "Transfer-Encoding", "Chunked"); + message.setChunked(true); + HttpChunk chunk1 = new DefaultHttpChunk(ChannelBuffers.copiedBuffer("test", CharsetUtil.US_ASCII)); + HttpChunk chunk2 = new DefaultHttpChunk(ChannelBuffers.copiedBuffer("test2", CharsetUtil.US_ASCII)); + HttpChunk chunk3 = new DefaultHttpChunk(ChannelBuffers.EMPTY_BUFFER); + assertFalse(embedder.offer(message)); + assertFalse(embedder.offer(chunk1)); + assertFalse(embedder.offer(chunk2)); + + // this should trigger a messageReceived event so return true + assertTrue(embedder.offer(chunk3)); + assertTrue(embedder.finish()); + HttpMessage aggratedMessage = embedder.poll(); + assertNotNull(aggratedMessage); + + assertEquals(chunk1.getContent().readableBytes() + chunk2.getContent().readableBytes(), HttpHeaders.getContentLength(aggratedMessage)); + assertEquals(aggratedMessage.getHeader("X-Test"), Boolean.TRUE.toString()); + checkContentBuffer(aggratedMessage); + assertNull(embedder.poll()); + } }