netty: exceed file size are not detected #3790

- fix #3790

Author: jknack

modules/jooby-netty/src/main/java/io/jooby/internal/netty/NettyHandler.java

@@ -35,7 +35,6 @@ public class NettyHandler extends ChannelInboundHandlerAdapter {
   private final boolean defaultHeaders;
   private final long maxRequestSize;
   private final int maxFormFields;
-  private long contentLength;
   private long chunkSize;
   private final boolean http2;
   private NettyContext context;
@@ -78,7 +77,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
         router.match(context).execute(context);
       } else {
         // possibly body:
-        contentLength = contentLength(req);
+        long contentLength = contentLength(req);
         if (contentLength > 0 || isTransferEncodingChunked(req)) {
           context.httpDataFactory = new DefaultHttpDataFactory(bufferSize);
           context.httpDataFactory.setBaseDir(app.getTmpdir().toString());
@@ -88,20 +87,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
           router.match(context).execute(context);
         }
       }
-    } else if (isLastHttpContent(msg)) {
-      var chunk = (HttpContent) msg;
-      try {
-        // when decoder == null, chunk is always a LastHttpContent.EMPTY, ignore it
-        if (context.decoder != null) {
-          if (offer(context, chunk)) {
-            Router.Match route = router.match(context);
-            resetDecoderState(context, !route.matches());
-            route.execute(context);
-          }
-        }
-      } finally {
-        release(chunk);
-      }
     } else if (isHttpContent(msg)) {
       var chunk = (HttpContent) msg;
       try {
@@ -113,10 +98,13 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
             router.match(context).execute(context, Route.REQUEST_ENTITY_TOO_LARGE);
             return;
           }
-          offer(context, chunk);
+          if (offer(context, chunk) && isLastHttpContent(msg)) {
+            Router.Match route = router.match(context);
+            resetDecoderState(context, !route.matches());
+            route.execute(context);
+          }
         }
       } finally {
-        // must be released
         release(chunk);
       }
     } else if (isWebSocketFrame(msg)) {
@@ -191,7 +179,6 @@ private boolean offer(NettyContext context, HttpContent chunk) {
 
   private void resetDecoderState(NettyContext context, boolean destroy) {
     chunkSize = 0;
-    contentLength = -1;
     if (destroy && context.decoder != null) {
       var decoder = context.decoder;
       var httpDataFactory = context.httpDataFactory;

tests/src/test/java/io/jooby/i2806/Issue2806.java

@@ -5,9 +5,9 @@
  */
 package io.jooby.i2806;
 
+import static io.jooby.test.TestUtil._19kb;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-import java.util.Arrays;
 import java.util.Map;
 
 import com.google.common.collect.ImmutableMap;
@@ -22,9 +22,6 @@ public class Issue2806 {
 
   @ServerTest
   public void renderShouldWorkFromErrorHandlerWhenLargeRequestAreSent(ServerTestRunner runner) {
-    char[] chars = new char[19 * 1024];
-    Arrays.fill(chars, 'S');
-    String _19kb = new String(chars);
     runner
         .define(
             app -> {
@@ -43,20 +40,32 @@ public void renderShouldWorkFromErrorHandlerWhenLargeRequestAreSent(ServerTestRu
                     ctx.render(map);
                   });
 
-              app.post("/2806", ctx -> ctx.body().value(""));
+              app.post(
+                  "/2806",
+                  ctx -> {
+                    return ctx.body().value("");
+                  });
 
               app.get("/2806", ctx -> ctx.body().value(""));
             })
         .ready(
             client -> {
               // Exceeds
-              client.post(
-                  "/2806",
-                  RequestBody.create(_19kb, MediaType.get("text/plain")),
-                  rsp -> {
-                    assertEquals(413, rsp.code());
-                    assertEquals("{\"router\":true,\"route\":true}", rsp.body().string());
-                  });
+              client
+                  .post("/2806", RequestBody.create(_19kb, MediaType.get("text/plain")))
+                  .execute(
+                      rsp -> {
+                        assertEquals(413, rsp.code());
+                        assertEquals("{\"router\":true,\"route\":true}", rsp.body().string());
+                      });
+
+              client
+                  .get("/2806")
+                  .execute(
+                      rsp -> {
+                        assertEquals(200, rsp.code());
+                        assertEquals("", rsp.body().string());
+                      });
             });
   }
 }

tests/src/test/java/io/jooby/test/WebClient.java

@@ -14,10 +14,7 @@
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.BlockingQueue;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.TimeUnit;
+import java.util.concurrent.*;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
@@ -142,17 +139,46 @@ public void close() {
 
   public class Request {
     private final okhttp3.Request.Builder req;
+    private SneakyThrows.Consumer<okhttp3.Request.Builder> configurer;
 
     public Request(okhttp3.Request.Builder req) {
       this.req = req;
     }
 
     public Request prepare(SneakyThrows.Consumer<okhttp3.Request.Builder> configurer) {
-      configurer.accept(req);
+      this.configurer = configurer;
       return this;
     }
 
     public void execute(SneakyThrows.Consumer<Response> callback) {
+      execute(1, callback);
+    }
+
+    public void execute(int concurrency, SneakyThrows.Consumer<Response> callback) {
+      if (configurer != null) {
+        configurer.accept(req);
+      }
+      if (concurrency > 1) {
+        var futures = new ArrayList<CompletableFuture<String>>();
+        for (var i = 0; i < concurrency; i++) {
+          futures.add(
+              CompletableFuture.supplyAsync(
+                  () -> {
+                    executeCall(callback);
+                    return "success";
+                  }));
+          try {
+            CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])).join();
+          } catch (CompletionException x) {
+            throw SneakyThrows.propagate(x.getCause());
+          }
+        }
+      } else {
+        executeCall(callback);
+      }
+    }
+
+    private void executeCall(SneakyThrows.Consumer<Response> callback) {
       okhttp3.Request r = req.build();
       try (Response rsp = client.newCall(r).execute()) {
         callback.accept(rsp);