Repository: netty/netty
Issue: HttpObjectEncoder can not encode chunked response

I'm tring HttpStaticFileServer with netty `5.0.0.Alpha2`, but not work, and I find that has two issues.
1. the `DefaultFullHttpResponse` implements `LastHttpContent`, after encode any of `DefaultFullHttpResponse` msg, `HttpObjectEncoder#encodeChunkedContent` will reset `state` to `ST_INIT`, then rest msg can not be encoded.
   
   ``` java
   private void encodeChunkedContent(ChannelHandlerContext ctx, Object msg, long contentLength, List<Object> out) {
       ...
       if (msg instanceof LastHttpContent) {
           ...
           state = ST_INIT; // Line 171
       } else {
           ...
       }
       ...
   }
   ```
2. the sample code not set `transfer-encoding` header to `chunked`. in HttpObjectEncoder will process as `chunked` response only the response msg has `chunked` header. 
   
   ``` java
   protected void encode(ChannelHandlerContext ctx, Object msg, List<Object> out) throws Exception {
       if (msg instanceof HttpMessage) {
           ...
           state = HttpHeaderUtil.isTransferEncodingChunked(m) ? ST_CONTENT_CHUNK : ST_CONTENT_NON_CHUNK; // Line 75
           ...
       }
   }
   ```


Issue comments:
> the DefaultFullHttpResponse implements LastHttpContent, after encode any of DefaultFullHttpResponse msg, HttpObjectEncoder#encodeChunkedContent will reset state to ST_INIT, then rest msg can not be encoded.

I think this is the expected behaviour since it's a completed http message.

in `HttpStaticFileServer` only the response header was encoded, but the `chunked` file content can not be encoded.
And I think this will also cause `Keep-Alive` not work.

> the sample code not set transfer-encoding header to chunked. in HttpObjectEncoder will process as chunked response only the response msg has chunked header.

Just sent #4683 to fix it.

> in HttpStaticFileServer only the response header was encoded, but the chunked file content can not be encoded.
> And I think this will also cause Keep-Alive not work.

HttpStaticFileServerHandler uses DefaultHttpResponse to send the file:
https://github.com/netty/netty/blob/b6a4f5de9d213cdbe32ff7772be4bcf6a1ba9094/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java#L177

`DefaultFullHttpResponse` is used to send the file list and doesn't use HttpChunkedInput.

HttpStaticFileServerHandler uses `DefaultHttpResponse` to send the `header`:
https://github.com/netty/netty/blob/b6a4f5de9d213cdbe32ff7772be4bcf6a1ba9094/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java#L186

and uses `HttpChunkedInput` to send the `content`:
https://github.com/netty/netty/blob/b6a4f5de9d213cdbe32ff7772be4bcf6a1ba9094/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java#L198

but only the `header` was encoded by `HttpObjectEncoder`, the `content` can not be encoded.

When I ran `HttpStaticFileServer` with ssl and downloaded a file, I did see `HttpObjectEncoder` processes `DefaultHttpResponse` and the following `DefaultHttpContent`s. Did you see any unexpected behaviour?

`IllegalStateException` will be throwed in `HttpObjectEncoder`  both http and https:
https://github.com/netty/netty/blob/netty-5.0.0.Alpha2/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java#L92

because after the `DefaultHttpResponse  header` was encoded, the `state` has been reset to `ST_INIT`:
https://github.com/netty/netty/blob/netty-5.0.0.Alpha2/codec-http/src/main/java/io/netty/handler/codec/http/HttpObjectEncoder.java#L171

Sorry, I was testing the branch 4.1. Yes, `HttpStaticFileServer` is broken in `5.0.0.Alpha2`. However, since 5.0 has been dropped (#4466), could you test branch 4.1 and see if it still doesn't work?

I tried `4.1` and got the same error.

@cpf624 can you please tell us the exact steps you did. Tried with 4.1 branch with and without ssl and it "just works".

Sorry, it's my fault, I write wrong `HttpResponse` as `FullHttpResponse`:

``` java
FullHttpResponse response = new DefaultFullHttpResponse(HTTP_1_1, OK);
```

it should be:
https://github.com/netty/netty/blob/4.1/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java#L177

Ok closing...


Pull request review comments:

Patch that resolved the issue:
diff --git a/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java b/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
index b2a4f33b3b0..f40962f720b 100644
--- a/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
+++ b/example/src/main/java/io/netty/example/http/file/HttpStaticFileServerHandler.java
@@ -182,18 +182,22 @@ public void channelRead0(ChannelHandlerContext ctx, FullHttpRequest request) thr
             response.headers().set(HttpHeaderNames.CONNECTION, HttpHeaderValues.KEEP_ALIVE);
         }
 
-        // Write the initial line and the header.
-        ctx.write(response);
-
         // Write the content.
         ChannelFuture sendFileFuture;
         ChannelFuture lastContentFuture;
         if (ctx.pipeline().get(SslHandler.class) == null) {
+            // Write the initial line and the header.
+            ctx.write(response);
+            // Write the content.
             sendFileFuture =
                     ctx.write(new DefaultFileRegion(raf.getChannel(), 0, fileLength), ctx.newProgressivePromise());
             // Write the end marker.
             lastContentFuture = ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT);
         } else {
+            HttpUtil.setTransferEncodingChunked(response, true);
+            // Write the initial line and the header.
+            ctx.write(response);
+            // Write the content.
             sendFileFuture =
                     ctx.writeAndFlush(new HttpChunkedInput(new ChunkedFile(raf, 0, fileLength, 8192)),
                             ctx.newProgressivePromise());


Test patch: