From 50c6735bca827a49aa1b2e552d5e9f45903c2d4b Mon Sep 17 00:00:00 2001
From: Bjørn Christian Seime <bjorncs@vespa.ai>
Date: Fri, 17 Nov 2023 10:48:11 +0100
Subject: Fail immediately when `Content-Length` is present

---
 .../http/server/jetty/ServletRequestReader.java     | 21 ++++++++++++++-------
 .../jdisc/http/server/jetty/HttpServerTest.java     | 14 ++++++++++++++
 2 files changed, 28 insertions(+), 7 deletions(-)

(limited to 'container-core')

diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
index a022d208d05..9fee54dd1d4 100644
--- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
+++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
@@ -1,7 +1,6 @@
 // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http.server.jetty;
 
-import com.yahoo.jdisc.Response;
 import com.yahoo.jdisc.handler.CompletionHandler;
 import com.yahoo.jdisc.handler.ContentChannel;
 import com.yahoo.jdisc.http.ConnectorConfig;
@@ -19,6 +18,8 @@ import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static com.yahoo.jdisc.Response.Status.REQUEST_TOO_LONG;
+
 /**
  * Finished when either
  * 1) There was an error
@@ -111,7 +112,8 @@ class ServletRequestReader {
         long maxContentSize = resolveMaxContentSize(cfg);
         var msgTemplate = resolveMaxContentSizeErrorMessage(cfg);
         this.requestContentChannel = maxContentSize >= 0
-                ? new ByteLimitedContentChannel(Objects.requireNonNull(requestContentChannel), maxContentSize, msgTemplate)
+                ? new ByteLimitedContentChannel(
+                        Objects.requireNonNull(requestContentChannel), maxContentSize, msgTemplate, req.getContentLengthLong())
                 : Objects.requireNonNull(requestContentChannel);
         this.janitor = Objects.requireNonNull(janitor);
         this.metricReporter = Objects.requireNonNull(metricReporter);
@@ -285,24 +287,29 @@ class ServletRequestReader {
     private static class ByteLimitedContentChannel implements ContentChannel {
         private final long maxContentSize;
         private final String messageTemplate;
+        private final long contentLengthHeader;
         private final AtomicLong bytesWritten = new AtomicLong();
         private final ContentChannel delegate;
 
-        ByteLimitedContentChannel(ContentChannel delegate, long maxContentSize, String messageTemplate) {
+        ByteLimitedContentChannel(ContentChannel delegate, long maxContentSize, String messageTemplate, long contentLengthHeader) {
             this.delegate = delegate;
             this.maxContentSize = maxContentSize;
             this.messageTemplate = messageTemplate;
+            this.contentLengthHeader = contentLengthHeader;
         }
 
         @Override
         public void write(ByteBuffer buf, CompletionHandler handler) {
             long written = bytesWritten.addAndGet(buf.remaining());
-            if (written > maxContentSize) {
+            if (contentLengthHeader != -1 && contentLengthHeader > maxContentSize) {
                 handler.failed(new RequestException(
-                        Response.Status.REQUEST_TOO_LONG, messageTemplate.formatted(written, maxContentSize)));
-                return;
+                        REQUEST_TOO_LONG, messageTemplate.formatted(contentLengthHeader, maxContentSize)));
+            } else if (written > maxContentSize) {
+                handler.failed(new RequestException(
+                        REQUEST_TOO_LONG, messageTemplate.formatted(written, maxContentSize)));
+            } else {
+                delegate.write(buf, handler);
             }
-            delegate.write(buf, handler);
         }
 
         @Override public void close(CompletionHandler h) { delegate.close(h); }
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
index 6e218a6ab66..adb35db8ebf 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -159,6 +159,20 @@ public class HttpServerTest {
         assertTrue(driver.close());
     }
 
+    @Test
+    void requireThatTooLargePayloadFailsWith413() throws Exception {
+        final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance(
+                new EchoRequestHandler(),
+                new ServerConfig.Builder(),
+                new ConnectorConfig.Builder()
+                        .maxContentSize(100));
+        driver.client().newPost("/status.html")
+                .setBinaryContent(new byte[200])
+                .execute()
+                .expectStatusCode(is(REQUEST_TOO_LONG));
+        assertTrue(driver.close());
+    }
+
     @Test
     void requireThatMultipleHostHeadersReturns400() throws Exception {
         var metricConsumer = new MetricConsumerMock();
-- 
cgit v1.2.3