From 70eee55a48daad4222f8ff37d47f7182f05de473 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Wed, 17 Apr 2013 12:51:22 +0900 Subject: [PATCH] Revamp HttpContentEncoder - Use state machine to simplify the code - Always produce a chunked response for simplicity - Change the signature of beginEncode() - HttpContent was simply unnecessary. - Add more test cases - Fixes #1280 --- .../codec/http/HttpContentCompressor.java | 4 +- .../codec/http/HttpContentEncoder.java | 247 ++++++++---------- .../codec/http/HttpContentEncoderTest.java | 91 ++++--- 3 files changed, 169 insertions(+), 173 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentCompressor.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentCompressor.java index 82cca50f3c..f319f8f581 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentCompressor.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentCompressor.java @@ -93,8 +93,8 @@ public class HttpContentCompressor extends HttpContentEncoder { } @Override - protected Result beginEncode(HttpMessage header, HttpContent msg, String acceptEncoding) throws Exception { - String contentEncoding = header.headers().get(HttpHeaders.Names.CONTENT_ENCODING); + protected Result beginEncode(HttpResponse headers, String acceptEncoding) throws Exception { + String contentEncoding = headers.headers().get(HttpHeaders.Names.CONTENT_ENCODING); if (contentEncoding != null && !HttpHeaders.Values.IDENTITY.equalsIgnoreCase(contentEncoding)) { return null; diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java index d69e9fde8e..ec9dd0bcaf 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpContentEncoder.java @@ -33,18 +33,18 @@ import java.util.Queue; /** * Encodes the content of the outbound {@link HttpResponse} and {@link HttpContent}. * The original content is replaced with the new content encoded by the - * {@link EmbeddedByteChannel}, which is created by {@link #beginEncode(HttpMessage, HttpContent, String)}. + * {@link EmbeddedByteChannel}, which is created by {@link #beginEncode(HttpResponse, String)}. * Once encoding is finished, the value of the 'Content-Encoding' header * is set to the target content encoding, as returned by - * {@link #beginEncode(HttpMessage, HttpContent, String)}. + * {@link #beginEncode(HttpResponse, String)}. * Also, the 'Content-Length' header is updated to the length of the * encoded content. If there is no supported or allowed encoding in the * corresponding {@link HttpRequest}'s {@code "Accept-Encoding"} header, - * {@link #beginEncode(HttpMessage, HttpContent, String)} should return {@code null} so that + * {@link #beginEncode(HttpResponse, String)} should return {@code null} so that * no encoding occurs (i.e. pass-through). *

* Please note that this is an abstract class. You have to extend this class - * and implement {@link #beginEncode(HttpMessage, HttpContent, String)} properly to make + * and implement {@link #beginEncode(HttpResponse, String)} properly to make * this class functional. For example, refer to the source code of * {@link HttpContentCompressor}. *

@@ -52,16 +52,26 @@ import java.util.Queue; * so that this handler can intercept HTTP responses before {@link HttpObjectEncoder} * converts them into {@link ByteBuf}s. */ -public abstract class HttpContentEncoder extends MessageToMessageCodec { +public abstract class HttpContentEncoder extends MessageToMessageCodec { + + private enum State { + PASS_THROUGH, + AWAIT_HEADERS, + AWAIT_CONTENT + } private final Queue acceptEncodingQueue = new ArrayDeque(); + private String acceptEncoding; private EmbeddedByteChannel encoder; - private HttpMessage message; - private boolean encodeStarted; - private boolean continueResponse; + private State state = State.AWAIT_HEADERS; @Override - protected void decode(ChannelHandlerContext ctx, HttpMessage msg, MessageBuf out) + public boolean acceptOutboundMessage(Object msg) throws Exception { + return msg instanceof HttpContent || msg instanceof HttpResponse; + } + + @Override + protected void decode(ChannelHandlerContext ctx, HttpRequest msg, MessageBuf out) throws Exception { String acceptedEncoding = msg.headers().get(HttpHeaders.Names.ACCEPT_ENCODING); if (acceptedEncoding == null) { @@ -72,136 +82,115 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec out) - throws Exception { - if (msg instanceof HttpResponse && ((HttpResponse) msg).getStatus().code() == 100) { + protected void encode(ChannelHandlerContext ctx, HttpObject msg, MessageBuf out) throws Exception { + final boolean isFull = msg instanceof HttpResponse && msg instanceof LastHttpContent; + switch (state) { + case AWAIT_HEADERS: { + ensureHeaders(msg); + assert encoder == null; - if (!(msg instanceof LastHttpContent)) { - continueResponse = true; - } - // 100-continue response must be passed through. - out.add(BufUtil.retain(msg)); - return; - } + final HttpResponse res = (HttpResponse) msg; - if (continueResponse) { - if (msg instanceof LastHttpContent) { - continueResponse = false; - } - // 100-continue response must be passed through. - out.add(BufUtil.retain(msg)); - return; - } - - // handle the case of single complete message without content - if (msg instanceof FullHttpMessage && !((FullHttpMessage) msg).data().isReadable()) { - - // Remove content encoding - String acceptEncoding = acceptEncodingQueue.poll(); - if (acceptEncoding == null) { - throw new IllegalStateException("cannot send more responses than requests"); - } - - out.add(BufUtil.retain(msg)); - return; - } - - if (msg instanceof HttpMessage) { - assert message == null; - - // check if this message is also of type HttpContent is such case just make a safe copy of the headers - // as the content will get handled later and this simplify the handling - if (msg instanceof HttpContent) { - if (msg instanceof HttpRequest) { - HttpRequest req = (HttpRequest) msg; - message = new DefaultHttpRequest(req.getProtocolVersion(), req.getMethod(), req.getUri()); - message.headers().set(req.headers()); - } else if (msg instanceof HttpResponse) { - HttpResponse res = (HttpResponse) msg; - message = new DefaultHttpResponse(res.getProtocolVersion(), res.getStatus()); - message.headers().set(res.headers()); - } else { - out.add(msg); - return; + if (res.getStatus().code() == 100) { + if (isFull) { + out.add(BufUtil.retain(res)); + } else { + out.add(res); + // Pass through all following contents. + state = State.PASS_THROUGH; + } + break; } - } else { - message = (HttpMessage) msg; - } - cleanup(); - } - - if (msg instanceof HttpContent) { - HttpContent c = (HttpContent) msg; - - if (!encodeStarted) { - encodeStarted = true; - HttpMessage message = this.message; - HttpHeaders headers = message.headers(); - this.message = null; - - // Determine the content encoding. - String acceptEncoding = acceptEncodingQueue.poll(); + // Get the list of encodings accepted by the peer. + acceptEncoding = acceptEncodingQueue.poll(); if (acceptEncoding == null) { throw new IllegalStateException("cannot send more responses than requests"); } - Result result = beginEncode(message, c, acceptEncoding); - - if (result == null) { - if (c instanceof LastHttpContent) { - encodeStarted = false; - out.add(message); - out.add(new DefaultLastHttpContent(c.data().retain())); - return; - } else { - out.add(message); - out.add(new DefaultHttpContent(c.data().retain())); - return; + if (isFull) { + // Pass through the full response with empty content and continue waiting for the the next resp. + if (!((ByteBufHolder) res).data().isReadable()) { + out.add(BufUtil.retain(res)); + break; } } + // Prepare to encode the content. + Result result = beginEncode(res, acceptEncoding); + + // If unable to encode, pass through. + if (result == null) { + if (isFull) { + out.add(BufUtil.retain(res)); + } else { + out.add(res); + state = State.PASS_THROUGH; + } + break; + } + encoder = result.contentEncoder(); // Encode the content and remove or replace the existing headers // so that the message looks like a decoded message. - headers.set( - HttpHeaders.Names.CONTENT_ENCODING, - result.targetContentEncoding()); + res.headers().set(Names.CONTENT_ENCODING, result.targetContentEncoding()); - HttpObject[] encoded = encodeContent(message, c); + // Make the response chunked to simplify content transformation. + res.headers().remove(Names.CONTENT_LENGTH); + res.headers().set(Names.TRANSFER_ENCODING, Values.CHUNKED); - if (encoded[0] instanceof HttpMessage && encoded[encoded.length - 1] instanceof LastHttpContent) { - // Set 'Content-Length' if the length of the content is known. - long contentLength = 0; - for (int i = 1; i < encoded.length; i ++) { - contentLength += ((ByteBufHolder) encoded[i]).data().readableBytes(); - } - headers.set(Names.CONTENT_LENGTH, contentLength); - headers.remove(Names.TRANSFER_ENCODING); + // Output the rewritten response. + if (isFull) { + // Convert full message into unfull one. + HttpResponse newRes = new DefaultHttpResponse(res.getProtocolVersion(), res.getStatus()); + newRes.headers().set(res.headers()); + out.add(newRes); + // Fall through to encode the content of the full response. } else { - headers.remove(Names.CONTENT_LENGTH); - headers.set(Names.TRANSFER_ENCODING, Values.CHUNKED); + out.add(res); + state = State.AWAIT_CONTENT; + break; } - + } + case AWAIT_CONTENT: { + ensureContent(msg); + HttpContent[] encoded = encodeContent((HttpContent) msg); Collections.addAll(out, encoded); - return; + if (encoded[encoded.length - 1] instanceof LastHttpContent) { + state = State.AWAIT_HEADERS; + } + break; } - - if (encoder != null) { - Collections.addAll(out, encodeContent(null, c)); - return; + case PASS_THROUGH: { + ensureContent(msg); + out.add(BufUtil.retain(msg)); + // Passed through all following contents of the current response. + if (msg instanceof LastHttpContent) { + state = State.AWAIT_HEADERS; + } + break; } - - if (c instanceof LastHttpContent) { - encodeStarted = false; - } - - out.add(c.retain()); } } - private HttpObject[] encodeContent(HttpMessage msg, HttpContent c) { + private static void ensureHeaders(HttpObject msg) { + if (!(msg instanceof HttpResponse)) { + throw new IllegalStateException( + "unexpected message type: " + + msg.getClass().getName() + " (expected: " + HttpResponse.class.getSimpleName() + ')'); + } + } + + private static void ensureContent(HttpObject msg) { + if (!(msg instanceof HttpContent)) { + throw new IllegalStateException( + "unexpected message type: " + + msg.getClass().getName() + " (expected: " + HttpContent.class.getSimpleName() + ')'); + } + } + + private HttpContent[] encodeContent(HttpContent c) { ByteBuf newContent = Unpooled.buffer(); ByteBuf content = c.data(); encode(content, newContent); @@ -213,35 +202,20 @@ public abstract class HttpContentEncoder extends MessageToMessageCodec