From 661acd24c8281bb338ed8b66839b3b7e2d93bcc6 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 7 Jan 2010 09:05:38 +0000 Subject: [PATCH] Fixed issue: NETTY-272 HttpMessageEncoder should not prepend/append extra data around HttpChunk content if Transfer-Encoding is not chunked. * HttpMessageEncoder now does not add any extra data around HttpChunk content if Transfer-Encoding is not 'chunked' * Moved the utility code that checks the existance of 'Transfer-Encoding: chunked' to HttpCodecUtil so that both HttpMessageEncoder and DefaultHttpMessage can use it --- .../codec/http/DefaultHttpMessage.java | 14 +--- .../handler/codec/http/HttpCodecUtil.java | 15 +++++ .../codec/http/HttpMessageEncoder.java | 67 ++++++++++++------- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java index b5dca47ae6..c95361bf04 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/DefaultHttpMessage.java @@ -112,19 +112,9 @@ public class DefaultHttpMessage implements HttpMessage { public boolean isChunked() { if (chunked) { return true; + } else { + return HttpCodecUtil.isTransferEncodingChunked(this); } - - List chunked = getHeaders(HttpHeaders.Names.TRANSFER_ENCODING); - if (chunked.isEmpty()) { - return false; - } - - for (String v: chunked) { - if (v.equalsIgnoreCase(HttpHeaders.Values.CHUNKED)) { - return true; - } - } - return false; } public void setChunked(boolean chunked) { 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 d5dbfeba66..68be2e708a 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 @@ -16,6 +16,7 @@ package org.jboss.netty.handler.codec.http; import java.nio.charset.Charset; +import java.util.List; import org.jboss.netty.util.CharsetUtil; @@ -159,4 +160,18 @@ class HttpCodecUtil { "value must not end with '\\r' or '\\n':" + value); } } + + static boolean isTransferEncodingChunked(HttpMessage m) { + List chunked = m.getHeaders(HttpHeaders.Names.TRANSFER_ENCODING); + if (chunked.isEmpty()) { + return false; + } + + for (String v: chunked) { + if (v.equalsIgnoreCase(HttpHeaders.Values.CHUNKED)) { + return true; + } + } + return false; + } } diff --git a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageEncoder.java b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageEncoder.java index 01d24208f8..b9f693ca19 100644 --- a/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageEncoder.java +++ b/src/main/java/org/jboss/netty/handler/codec/http/HttpMessageEncoder.java @@ -50,12 +50,14 @@ import org.jboss.netty.util.CharsetUtil; * * @apiviz.landmark */ -@ChannelPipelineCoverage("all") +@ChannelPipelineCoverage("one") public abstract class HttpMessageEncoder extends OneToOneEncoder { private static final ChannelBuffer LAST_CHUNK = copiedBuffer("0\r\n\r\n", CharsetUtil.US_ASCII); + private volatile boolean chunked; + /** * Creates a new instance. */ @@ -66,16 +68,21 @@ public abstract class HttpMessageEncoder extends OneToOneEncoder { @Override protected Object encode(ChannelHandlerContext ctx, Channel channel, Object msg) throws Exception { if (msg instanceof HttpMessage) { - HttpMessage request = (HttpMessage) msg; + HttpMessage m = (HttpMessage) msg; + boolean chunked = this.chunked = HttpCodecUtil.isTransferEncodingChunked(m); ChannelBuffer header = ChannelBuffers.dynamicBuffer( channel.getConfig().getBufferFactory()); - encodeInitialLine(header, request); - encodeHeaders(header, request); + encodeInitialLine(header, m); + encodeHeaders(header, m); header.writeBytes(CRLF); - ChannelBuffer content = request.getContent(); + ChannelBuffer content = m.getContent(); if (!content.readable()) { return header; // no content + } else if (chunked) { + throw new IllegalArgumentException( + "HttpMessage.content must be empty " + + "if Transfer-Encoding is chunked."); } else { return wrappedBuffer(header, content); } @@ -83,28 +90,38 @@ public abstract class HttpMessageEncoder extends OneToOneEncoder { if (msg instanceof HttpChunk) { HttpChunk chunk = (HttpChunk) msg; - if (chunk == HttpChunk.LAST_CHUNK) { - return LAST_CHUNK.duplicate(); - } else if (chunk instanceof HttpChunkTrailer) { - ChannelBuffer trailer = ChannelBuffers.dynamicBuffer( - channel.getConfig().getBufferFactory()); - trailer.writeByte((byte) '0'); - trailer.writeBytes(CRLF); - encodeTrailingHeaders(trailer, (HttpChunkTrailer) chunk); - trailer.writeBytes(CRLF); - return trailer; - } else { - ChannelBuffer content = chunk.getContent(); - int contentLength = content.readableBytes(); + if (chunked) { + if (chunk == HttpChunk.LAST_CHUNK) { + chunked = false; + return LAST_CHUNK.duplicate(); + } else if (chunk instanceof HttpChunkTrailer) { + ChannelBuffer trailer = ChannelBuffers.dynamicBuffer( + channel.getConfig().getBufferFactory()); + trailer.writeByte((byte) '0'); + trailer.writeBytes(CRLF); + encodeTrailingHeaders(trailer, (HttpChunkTrailer) chunk); + trailer.writeBytes(CRLF); + return trailer; + } else { + ChannelBuffer content = chunk.getContent(); + int contentLength = content.readableBytes(); - return wrappedBuffer( - copiedBuffer( - Integer.toHexString(contentLength), - CharsetUtil.US_ASCII), - wrappedBuffer(CRLF), - content.slice(content.readerIndex(), contentLength), - wrappedBuffer(CRLF)); + return wrappedBuffer( + copiedBuffer( + Integer.toHexString(contentLength), + CharsetUtil.US_ASCII), + wrappedBuffer(CRLF), + content.slice(content.readerIndex(), contentLength), + wrappedBuffer(CRLF)); + } + } else { + if (chunk == HttpChunk.LAST_CHUNK) { + return null; + } else { + return chunk.getContent(); + } } + } // Unknown message type.