Clean up Assets don't work on HTTPS with Netty #621

Author: jknack

jooby-netty/src/main/java/org/jooby/internal/netty/NettyResponse.java

@@ -20,7 +20,6 @@
 
 import static io.netty.channel.ChannelFutureListener.CLOSE;
 
-import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
@@ -51,7 +50,6 @@
 import io.netty.handler.codec.http.LastHttpContent;
 import io.netty.handler.codec.http2.HttpConversionUtil;
 import io.netty.handler.ssl.SslHandler;
-import io.netty.handler.stream.ChunkedFile;
 import io.netty.handler.stream.ChunkedNioFile;
 import io.netty.handler.stream.ChunkedStream;
 import io.netty.handler.stream.ChunkedWriteHandler;
@@ -147,10 +145,7 @@ public void send(final InputStream stream) throws Exception {
       ctx.channel().attr(NettyRequest.NEED_FLUSH).set(false);
 
       // add chunker
-      ChannelPipeline pipeline = ctx.pipeline();
-      if (pipeline.get("chunker") == null) {
-        pipeline.addAfter("codec", "chunker", new ChunkedWriteHandler());
-      }
+      chunkHandler(ctx.pipeline());
 
       // group all write
       ctx.channel().eventLoop().execute(() -> {
@@ -190,28 +185,32 @@ public void send(final FileChannel channel, final long offset, final long count)
     ChannelHandlerContext ctx = this.ctx;
     ctx.channel().attr(NettyRequest.NEED_FLUSH).set(false);
 
-    // add chunker
     ChannelPipeline pipeline = ctx.pipeline();
-    if (pipeline.get("chunker") == null) {
-      pipeline.addAfter("codec", "chunker", new ChunkedWriteHandler());
-    }
+    boolean ssl = pipeline.get(SslHandler.class) != null;
 
-    // Create the cunked input here already, to properly handle the IOException
-    final HttpChunkedInput chunkedInput = new HttpChunkedInput(new ChunkedNioFile(channel, offset, count, 8192));
+    if (ssl) {
+      // add chunker
+      chunkHandler(pipeline);
 
-    ctx.channel().eventLoop().execute(() -> {
-      // send headers
-      ctx.write(rsp);
+      // Create the chunked input here already, to properly handle the IOException
+      HttpChunkedInput chunkedInput = new HttpChunkedInput(
+          new ChunkedNioFile(channel, offset, count, bufferSize));
 
-      // For SSL, we cannot send a file region
-      if (ctx.pipeline().get(SslHandler.class) == null)
-      {
+      ctx.channel().eventLoop().execute(() -> {
+        // send headers
+        ctx.write(rsp);
+        // chunked file
+        keepAlive(ctx.writeAndFlush(chunkedInput));
+      });
+    } else {
+      ctx.channel().eventLoop().execute(() -> {
+        // send headers
+        ctx.write(rsp);
+        // file region
         ctx.write(new DefaultFileRegion(channel, offset, count));
         keepAlive(ctx.writeAndFlush(LastHttpContent.EMPTY_LAST_CONTENT));
-      } else {
-        keepAlive(ctx.writeAndFlush(chunkedInput));
-      }
-    });
+      });
+    }
 
     committed = true;
 
@@ -298,4 +297,10 @@ public void reset() {
     status = HttpResponseStatus.OK;
   }
 
+  private void chunkHandler(final ChannelPipeline pipeline) {
+    if (pipeline.get("chunker") == null) {
+      pipeline.addAfter("codec", "chunker", new ChunkedWriteHandler());
+    }
+  }
+
 }

jooby-netty/src/test/java/org/jooby/internal/netty/NettyResponseTest.java

@@ -20,7 +20,6 @@
 import java.util.List;
 import java.util.Optional;
 
-import org.apache.http.impl.bootstrap.SSLServerSetupHandler;
 import org.jooby.test.MockUnit;
 import org.jooby.test.MockUnit.Block;
 import org.junit.Test;
@@ -603,14 +602,10 @@ public void sendFileChannel() throws Exception {
         })
         .expect(unit -> {
           ChannelPipeline pipeline = unit.mock(ChannelPipeline.class);
-          expect(pipeline.get("chunker")).andReturn(null);
           expect(pipeline.get(SslHandler.class)).andReturn(null);
-          expect(pipeline.addAfter(eq("codec"), eq("chunker"), isA(ChunkedWriteHandler.class)))
-                  .andReturn(pipeline);
 
           ChannelHandlerContext ctx = unit.get(ChannelHandlerContext.class);
           expect(ctx.pipeline()).andReturn(pipeline);
-          expect(ctx.pipeline()).andReturn(pipeline);
         })
         .expect(noKeepAliveNoLen)
         .run(unit -> {
@@ -688,7 +683,6 @@ public void sendFileChannelSSL() throws Exception {
 
               ChannelHandlerContext ctx = unit.get(ChannelHandlerContext.class);
               expect(ctx.pipeline()).andReturn(pipeline);
-              expect(ctx.pipeline()).andReturn(pipeline);
             })
             .expect(noKeepAliveNoLen)
             .run(unit -> {
@@ -751,14 +745,10 @@ public void sendFileChannelNoLen() throws Exception {
         })
         .expect(unit -> {
           ChannelPipeline pipeline = unit.mock(ChannelPipeline.class);
-          expect(pipeline.get("chunker")).andReturn(null);
           expect(pipeline.get(SslHandler.class)).andReturn(null);
-          expect(pipeline.addAfter(eq("codec"), eq("chunker"), isA(ChunkedWriteHandler.class)))
-              .andReturn(pipeline);
 
           ChannelHandlerContext ctx = unit.get(ChannelHandlerContext.class);
           expect(ctx.pipeline()).andReturn(pipeline);
-          expect(ctx.pipeline()).andReturn(pipeline);
         })
         .expect(setNeedFlush)
         .expect(noKeepAliveNoLen)