diff options
author | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2023-06-26 14:37:48 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2023-06-26 14:38:16 +0200 |
commit | a94809b6aab7e7da93cdff359dfce0743177447f (patch) | |
tree | 34b823395ae97aab3f749bf41895354afae69cf5 | |
parent | 74f39c2bed1f08bf9caf56d989408c96c0140a41 (diff) |
Add configurable limit for max request content size
4 files changed, 52 insertions, 2 deletions
diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json index 236586a4132..572d18b02f3 100644 --- a/container-core/abi-spec.json +++ b/container-core/abi-spec.json @@ -1044,6 +1044,7 @@ "public com.yahoo.jdisc.http.ConnectorConfig$Builder requestHeaderSize(int)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder responseHeaderSize(int)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder acceptQueueSize(int)", + "public com.yahoo.jdisc.http.ConnectorConfig$Builder maxContentSize(long)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder reuseAddress(boolean)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder idleTimeout(double)", "public com.yahoo.jdisc.http.ConnectorConfig$Builder tcpKeepAliveEnabled(boolean)", @@ -1413,6 +1414,7 @@ "public int requestHeaderSize()", "public int responseHeaderSize()", "public int acceptQueueSize()", + "public long maxContentSize()", "public boolean reuseAddress()", "public double idleTimeout()", "public boolean tcpKeepAliveEnabled()", 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 2f2c48e0b48..75ef655c60c 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,16 +1,19 @@ // Copyright Yahoo. 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 jakarta.servlet.ReadListener; import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServletRequest; +import org.eclipse.jetty.server.Request; import java.io.IOException; import java.nio.ByteBuffer; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Level; import java.util.logging.Logger; @@ -33,6 +36,7 @@ import java.util.logging.Logger; */ class ServletRequestReader { + private enum State { NOT_STARTED, READING, ALL_DATA_READ, REQUEST_CONTENT_CLOSED } @@ -96,12 +100,15 @@ class ServletRequestReader { private final CompletableFuture<Void> finishedFuture = new CompletableFuture<>(); ServletRequestReader( - HttpServletRequest req, + Request req, ContentChannel requestContentChannel, Janitor janitor, RequestMetricReporter metricReporter) { this.req = Objects.requireNonNull(req); - this.requestContentChannel = Objects.requireNonNull(requestContentChannel); + long maxContentSize = RequestUtils.getConnector(req).connectorConfig().maxContentSize(); + this.requestContentChannel = maxContentSize >= 0 + ? new ByteLimitedContentChannel(Objects.requireNonNull(requestContentChannel), maxContentSize) + : Objects.requireNonNull(requestContentChannel); this.janitor = Objects.requireNonNull(janitor); this.metricReporter = Objects.requireNonNull(metricReporter); } @@ -259,4 +266,30 @@ class ServletRequestReader { } } + private static class ByteLimitedContentChannel implements ContentChannel { + private final long maxContentSize; + private final AtomicLong bytesWritten = new AtomicLong(); + private final ContentChannel delegate; + + ByteLimitedContentChannel(ContentChannel delegate, long maxContentSize) { + this.delegate = delegate; + this.maxContentSize = maxContentSize; + } + + @Override + public void write(ByteBuffer buf, CompletionHandler handler) { + long written = bytesWritten.addAndGet(buf.remaining()); + if (written > maxContentSize) { + handler.failed(new RequestException( + Response.Status.REQUEST_TOO_LONG, + "Request content length %d exceeds limit of %d bytes".formatted(written, maxContentSize))); + return; + } + delegate.write(buf, handler); + } + + @Override public void close(CompletionHandler h) { delegate.close(h); } + @Override public void onError(Throwable t) { delegate.onError(t); } + } + } diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def index bdcc3f9e40a..3c01012fd9e 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.connector.def @@ -22,6 +22,9 @@ responseHeaderSize int default=65536 # The accept queue size (also known as accept backlog). acceptQueueSize int default=0 +# Max content size allowed for requests. Set to -1 to disable. +maxContentSize long default=-1 + # Whether the server socket reuses addresses. reuseAddress bool default=true 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 4bcea426910..6f9c854be64 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 @@ -70,6 +70,7 @@ import static com.yahoo.jdisc.Response.Status.GATEWAY_TIMEOUT; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; import static com.yahoo.jdisc.Response.Status.OK; +import static com.yahoo.jdisc.Response.Status.REQUEST_TOO_LONG; import static com.yahoo.jdisc.Response.Status.REQUEST_URI_TOO_LONG; import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED; import static com.yahoo.jdisc.Response.Status.UNSUPPORTED_MEDIA_TYPE; @@ -795,6 +796,17 @@ public class HttpServerTest { assertTrue(driver.close()); } + @Test + void exceedingMaxContentSizeReturns413() throws IOException { + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + new EchoRequestHandler(), + new ServerConfig.Builder(), + new ConnectorConfig.Builder().maxContentSize(4)); + driver.client().newPost("/").setBinaryContent(new byte[4]).execute().expectStatusCode(is(OK)); + driver.client().newPost("/").setBinaryContent(new byte[5]).execute().expectStatusCode(is(REQUEST_TOO_LONG)); + assertTrue(driver.close()); + } + private static JettyTestDriver createSslWithTlsClientAuthenticationEnforcer(Path certificateFile, Path privateKeyFile) { ConnectorConfig.Builder connectorConfig = new ConnectorConfig.Builder() .tlsClientAuthEnforcer( |