From 150c1bef6da8bdd7697227153e45cd4288e9445c Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:33:26 -0700 Subject: [PATCH 01/13] Allow headers size extend to maxRequestHeadersSize in http client --- .../org/eclipse/jetty/client/HttpClient.java | 20 +- .../client/http/HttpSenderOverHTTPTest.java | 188 ++++++++++++++++++ 2 files changed, 195 insertions(+), 13 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 5592dd13bccd..6b86af055fab 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -104,7 +104,7 @@ * }</pre> */ @ManagedObject("The HTTP client") -public class HttpClient extends ContainerLifeCycle implements AutoCloseable +public class HttpClient extends ContainerLifeCycle { public static final String USER_AGENT = "Jetty/" + Jetty.VERSION; private static final Logger LOG = LoggerFactory.getLogger(HttpClient.class); @@ -135,6 +135,7 @@ public class HttpClient extends ContainerLifeCycle implements AutoCloseable private String defaultRequestContentType = "application/octet-stream"; private boolean useInputDirectByteBuffers = true; private boolean useOutputDirectByteBuffers = true; + private int maxRequestHeadersSize = 32 * 1024; private int maxResponseHeadersSize = -1; private Sweeper destinationSweeper; @@ -218,7 +219,7 @@ protected void doStart() throws Exception if (cookieStore == null) cookieStore = new HttpCookieStore.Default(); - getContainedBeans(Aware.class).forEach(bean -> bean.setHttpClient(this)); + transport.setHttpClient(this); super.doStart(); @@ -1142,18 +1143,11 @@ public ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory.C return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory); } - @Override - public void close() throws Exception - { - stop(); + public int getMaxRequestHeadersSize() { + return maxRequestHeadersSize; } - /** - * <p>Descendant beans of {@code HttpClient} that implement this interface - * are made aware of the {@code HttpClient} instance while it is starting.</p> - */ - public interface Aware - { - void setHttpClient(HttpClient httpClient); + public void setMaxRequestHeadersSize(int maxRequestHeadersSize) { + this.maxRequestHeadersSize = maxRequestHeadersSize; } } diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java index 313fd841de41..c7101f466a59 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java @@ -16,7 +16,10 @@ import java.net.URI; import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; +import java.util.HashMap; import java.util.Locale; +import java.util.Map; +import java.util.Random; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -29,6 +32,7 @@ import org.eclipse.jetty.client.Result; import org.eclipse.jetty.client.transport.HttpDestination; import org.eclipse.jetty.client.transport.internal.HttpConnectionOverHTTP; +import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.io.ByteArrayEndPoint; import org.eclipse.jetty.util.Promise; import org.hamcrest.Matchers; @@ -39,6 +43,7 @@ import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.wildfly.common.Assert.assertFalse; public class HttpSenderOverHTTPTest { @@ -302,4 +307,187 @@ public void onSuccess(Request request) assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); assertTrue(successLatch.await(5, TimeUnit.SECONDS)); } + + private static Random rnd = new Random(); + private static final String CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890"; + + public static final int CHARS_LENGTH = CHARS.length(); + + protected static String getRandomString(int size) { + StringBuilder sb = new StringBuilder(size); + while (sb.length() < size) { // length of the random string. + int index = rnd.nextInt(CHARS_LENGTH); + sb.append(CHARS.charAt(index)); + } + return sb.toString(); + } + + @Test + public void testSmallHeadersSize() throws Exception + { + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + request.agent(getRandomString(888)); //More than the request buffer size, but less than the default max request headers size + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch successLatch = new CountDownLatch(1); + final CountDownLatch failureLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onSuccess(Request request) + { + successLatch.countDown(); + } + + @Override + public void onFailure(Request request, Throwable failure) { + failureLatch.countDown(); + } + }); + connection.send(request, null); + + String requestString = endPoint.takeOutputString(); + assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(successLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testMaxRequestHeadersSize() throws Exception + { + byte[] buffer = new byte[32 * 1024]; + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + //More than the request buffer size, but less than the default max request headers size + + int desiredHeadersSize = 20 * 1024; + int currentHeadersSize = 0; + int i = 0; + while(currentHeadersSize < desiredHeadersSize) { + final int index = i ++; + final String headerValue = getRandomString(800); + final int headerSize = headerValue.length(); + currentHeadersSize += headerSize; + request.cookie(new HttpCookie() { + @Override + public String getName() { + return "large" + index; + } + + @Override + public String getValue() { + return headerValue; + } + + @Override + public int getVersion() { + return 0; + } + + @Override + public Map<String, String> getAttributes() { + return new HashMap<>(); + } + }); + } + + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch successLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onSuccess(Request request) + { + successLatch.countDown(); + } + }); + connection.send(request, null); + + String requestString = endPoint.takeOutputString(); + assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(successLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testMaxRequestHeadersSizeOverflow() throws Exception + { + byte[] buffer = new byte[32 * 1024]; + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + //More than the request buffer size, but less than the default max request headers size + + int desiredHeadersSize = 35 * 1024; + int currentHeadersSize = 0; + int i = 0; + while(currentHeadersSize < desiredHeadersSize) { + final int index = i ++; + final String headerValue = getRandomString(800); + final int headerSize = headerValue.length(); + currentHeadersSize += headerSize; + request.cookie(new HttpCookie() { + @Override + public String getName() { + return "large" + index; + } + + @Override + public String getValue() { + return headerValue; + } + + @Override + public int getVersion() { + return 0; + } + + @Override + public Map<String, String> getAttributes() { + return new HashMap<>(); + } + }); + } + + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch failureLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onFailure(Request request, Throwable failure) + { + failureLatch.countDown(); + } + }); + connection.send(request, null); + + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(failureLatch.await(5, TimeUnit.SECONDS)); + } } From a996d533198b5940ce5999b18b75718df3b654df Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:36:23 -0700 Subject: [PATCH 02/13] Allow headers size extend to maxRequestHeadersSize in http client --- .../internal/HttpSenderOverHTTP.java | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 1f6bc4a3f756..e14bd774b4ea 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -155,7 +155,7 @@ private HeadersCallback() protected Action process() throws Exception { HttpClient httpClient = getHttpChannel().getHttpDestination().getHttpClient(); - HttpExchange exchange = getHttpExchange(); + HttpExchange exchange = getHttpChannel().getHttpExchange(); ByteBufferPool bufferPool = httpClient.getByteBufferPool(); boolean useDirectByteBuffers = httpClient.isUseOutputDirectByteBuffers(); while (true) @@ -178,9 +178,21 @@ protected Action process() throws Exception } case HEADER_OVERFLOW: { - headerBuffer.release(); - headerBuffer = null; - throw new IllegalArgumentException("Request header too large"); + int maxRequestHeadersSize = httpClient.getMaxRequestHeadersSize(); + if (headerBuffer.capacity() < maxRequestHeadersSize) { + RetainableByteBuffer newHeaderBuffer = bufferPool.acquire(maxRequestHeadersSize, useDirectByteBuffers); + headerBuffer.getByteBuffer().flip(); + newHeaderBuffer.getByteBuffer().put(headerBuffer.getByteBuffer()); + RetainableByteBuffer toRelease = headerBuffer; + headerBuffer = newHeaderBuffer; + toRelease.release(); + break; + } + else { + headerBuffer.release(); + headerBuffer = null; + throw new IllegalArgumentException("Request header too large"); + } } case NEED_CHUNK: { @@ -235,9 +247,17 @@ protected Action process() throws Exception } @Override - protected void onSuccess() + public void succeeded() { release(); + super.succeeded(); + } + + @Override + public void failed(Throwable x) + { + release(); + super.failed(x); } @Override @@ -251,7 +271,6 @@ protected void onCompleteSuccess() protected void onCompleteFailure(Throwable cause) { super.onCompleteFailure(cause); - release(); callback.failed(cause); } From de470bf61e5e21299c459be22e8bdb8ec0a9f0b6 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:39:08 -0700 Subject: [PATCH 03/13] Fix merging error --- .../org/eclipse/jetty/client/HttpClient.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 6b86af055fab..b4ce960c04c4 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -104,7 +104,7 @@ * }</pre> */ @ManagedObject("The HTTP client") -public class HttpClient extends ContainerLifeCycle +public class HttpClient extends ContainerLifeCycle implements AutoCloseable { public static final String USER_AGENT = "Jetty/" + Jetty.VERSION; private static final Logger LOG = LoggerFactory.getLogger(HttpClient.class); @@ -219,7 +219,7 @@ protected void doStart() throws Exception if (cookieStore == null) cookieStore = new HttpCookieStore.Default(); - transport.setHttpClient(this); + getContainedBeans(Aware.class).forEach(bean -> bean.setHttpClient(this)); super.doStart(); @@ -1143,6 +1143,19 @@ public ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory.C return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory); } + @Override + public void close() throws Exception { + stop(); + } + + /** + * <p>Descendant beans of {@code HttpClient} that implement this interface + * are made aware of the {@code HttpClient} instance while it is starting.</p> + */ + public interface Aware { + void setHttpClient(HttpClient httpClient); + } + public int getMaxRequestHeadersSize() { return maxRequestHeadersSize; } From 8d5a224423bf6cfb292fff97bd17acafee69bac6 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:40:15 -0700 Subject: [PATCH 04/13] Fix syntax difference --- .../java/org/eclipse/jetty/client/HttpClient.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index b4ce960c04c4..8dd6212a2ef8 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -1144,7 +1144,8 @@ public ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory.C } @Override - public void close() throws Exception { + public void close() throws Exception + { stop(); } @@ -1152,15 +1153,18 @@ public void close() throws Exception { * <p>Descendant beans of {@code HttpClient} that implement this interface * are made aware of the {@code HttpClient} instance while it is starting.</p> */ - public interface Aware { + public interface Aware + { void setHttpClient(HttpClient httpClient); } - public int getMaxRequestHeadersSize() { + public int getMaxRequestHeadersSize() + { return maxRequestHeadersSize; } - public void setMaxRequestHeadersSize(int maxRequestHeadersSize) { + public void setMaxRequestHeadersSize(int maxRequestHeadersSize) + { this.maxRequestHeadersSize = maxRequestHeadersSize; } } From 4f4cd064ceefcfe798f8f0035bbf8dc52a1087ca Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:41:34 -0700 Subject: [PATCH 05/13] Avoid unnecessary change --- .../jetty/client/transport/internal/HttpSenderOverHTTP.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index e14bd774b4ea..6aa621200878 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -155,7 +155,7 @@ private HeadersCallback() protected Action process() throws Exception { HttpClient httpClient = getHttpChannel().getHttpDestination().getHttpClient(); - HttpExchange exchange = getHttpChannel().getHttpExchange(); + HttpExchange exchange = getHttpExchange(); ByteBufferPool bufferPool = httpClient.getByteBufferPool(); boolean useDirectByteBuffers = httpClient.isUseOutputDirectByteBuffers(); while (true) From 1af9f816501bed8d7db49d60d934070af0d9bed2 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:43:46 -0700 Subject: [PATCH 06/13] Avoid unnecessary change --- .../client/transport/internal/HttpSenderOverHTTP.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 6aa621200878..3e3901f12483 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -247,24 +247,17 @@ protected Action process() throws Exception } @Override - public void succeeded() + protected void onSuccess() { release(); super.succeeded(); } - @Override - public void failed(Throwable x) - { - release(); - super.failed(x); - } - @Override protected void onCompleteSuccess() { super.onCompleteSuccess(); - callback.succeeded(); + release(); } @Override From 23daa2c4731cc79e73d6fdf4c278f0b93ebb5127 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:44:39 -0700 Subject: [PATCH 07/13] Avoid unnecessary change --- .../jetty/client/transport/internal/HttpSenderOverHTTP.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 3e3901f12483..9626efeb582a 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -257,14 +257,14 @@ protected void onSuccess() protected void onCompleteSuccess() { super.onCompleteSuccess(); - release(); + callback.succeeded(); } @Override protected void onCompleteFailure(Throwable cause) { super.onCompleteFailure(cause); - callback.failed(cause); + release(); } private void release() From 5de3261b5c5c009645a227043e79933a58ec534b Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:45:19 -0700 Subject: [PATCH 08/13] Avoid unnecessary change --- .../jetty/client/transport/internal/HttpSenderOverHTTP.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 9626efeb582a..485d9f142389 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -250,7 +250,6 @@ protected Action process() throws Exception protected void onSuccess() { release(); - super.succeeded(); } @Override @@ -265,6 +264,7 @@ protected void onCompleteFailure(Throwable cause) { super.onCompleteFailure(cause); release(); + callback.failed(cause); } private void release() From 89bf780d09ab561d4f037afce0e7f376b131813c Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:46:52 -0700 Subject: [PATCH 09/13] Avoid unnecessary change --- .../org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java index c7101f466a59..964c13653173 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java @@ -43,7 +43,6 @@ import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.wildfly.common.Assert.assertFalse; public class HttpSenderOverHTTPTest { From 0dc9e658f89ed66b739102eb54bf22d140d6fad0 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 13:56:40 -0700 Subject: [PATCH 10/13] Fix syntax checking issues --- .../internal/HttpSenderOverHTTP.java | 6 ++- .../client/http/HttpSenderOverHTTPTest.java | 39 ++++++++++++------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index 485d9f142389..dfe4eb8ecb1e 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -179,7 +179,8 @@ protected Action process() throws Exception case HEADER_OVERFLOW: { int maxRequestHeadersSize = httpClient.getMaxRequestHeadersSize(); - if (headerBuffer.capacity() < maxRequestHeadersSize) { + if (headerBuffer.capacity() < maxRequestHeadersSize) + { RetainableByteBuffer newHeaderBuffer = bufferPool.acquire(maxRequestHeadersSize, useDirectByteBuffers); headerBuffer.getByteBuffer().flip(); newHeaderBuffer.getByteBuffer().put(headerBuffer.getByteBuffer()); @@ -188,7 +189,8 @@ protected Action process() throws Exception toRelease.release(); break; } - else { + else + { headerBuffer.release(); headerBuffer = null; throw new IllegalArgumentException("Request header too large"); diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java index 964c13653173..aaf19ab98c9a 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java @@ -348,7 +348,8 @@ public void onSuccess(Request request) } @Override - public void onFailure(Request request, Throwable failure) { + public void onFailure(Request request, Throwable failure) + { failureLatch.countDown(); } }); @@ -374,29 +375,35 @@ public void testMaxRequestHeadersSize() throws Exception int desiredHeadersSize = 20 * 1024; int currentHeadersSize = 0; int i = 0; - while(currentHeadersSize < desiredHeadersSize) { + while(currentHeadersSize < desiredHeadersSize) + { final int index = i ++; final String headerValue = getRandomString(800); final int headerSize = headerValue.length(); currentHeadersSize += headerSize; - request.cookie(new HttpCookie() { + request.cookie(new HttpCookie() + { @Override - public String getName() { + public String getName() + { return "large" + index; } @Override - public String getValue() { + public String getValue() + { return headerValue; } @Override - public int getVersion() { + public int getVersion() + { return 0; } @Override - public Map<String, String> getAttributes() { + public Map<String, String> getAttributes() + { return new HashMap<>(); } }); @@ -440,29 +447,35 @@ public void testMaxRequestHeadersSizeOverflow() throws Exception int desiredHeadersSize = 35 * 1024; int currentHeadersSize = 0; int i = 0; - while(currentHeadersSize < desiredHeadersSize) { + while(currentHeadersSize < desiredHeadersSize) + { final int index = i ++; final String headerValue = getRandomString(800); final int headerSize = headerValue.length(); currentHeadersSize += headerSize; - request.cookie(new HttpCookie() { + request.cookie(new HttpCookie() + { @Override - public String getName() { + public String getName() + { return "large" + index; } @Override - public String getValue() { + public String getValue() + { return headerValue; } @Override - public int getVersion() { + public int getVersion() + { return 0; } @Override - public Map<String, String> getAttributes() { + public Map<String, String> getAttributes() + { return new HashMap<>(); } }); From 06f827e607504289c6db7f61c08732d368dd7d25 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Tue, 29 Oct 2024 14:02:32 -0700 Subject: [PATCH 11/13] Fix syntax checking issues --- .../jetty/client/http/HttpSenderOverHTTPTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java index aaf19ab98c9a..fc6eeb685ccb 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java @@ -312,9 +312,11 @@ public void onSuccess(Request request) public static final int CHARS_LENGTH = CHARS.length(); - protected static String getRandomString(int size) { + protected static String getRandomString(int size) + { StringBuilder sb = new StringBuilder(size); - while (sb.length() < size) { // length of the random string. + while (sb.length() < size) + { // length of the random string. int index = rnd.nextInt(CHARS_LENGTH); sb.append(CHARS.charAt(index)); } @@ -375,9 +377,9 @@ public void testMaxRequestHeadersSize() throws Exception int desiredHeadersSize = 20 * 1024; int currentHeadersSize = 0; int i = 0; - while(currentHeadersSize < desiredHeadersSize) + while (currentHeadersSize < desiredHeadersSize) { - final int index = i ++; + final int index = i++; final String headerValue = getRandomString(800); final int headerSize = headerValue.length(); currentHeadersSize += headerSize; @@ -447,9 +449,9 @@ public void testMaxRequestHeadersSizeOverflow() throws Exception int desiredHeadersSize = 35 * 1024; int currentHeadersSize = 0; int i = 0; - while(currentHeadersSize < desiredHeadersSize) + while (currentHeadersSize < desiredHeadersSize) { - final int index = i ++; + final int index = i++; final String headerValue = getRandomString(800); final int headerSize = headerValue.length(); currentHeadersSize += headerSize; From 8e16919a55fd0fb5e85ed2aa0f85a07a30a87400 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Thu, 7 Nov 2024 09:25:04 -0800 Subject: [PATCH 12/13] Fix based on the recommendations --- .../java/org/eclipse/jetty/client/HttpClient.java | 12 +----------- .../transport/HttpClientTransportOverHTTP.java | 14 ++++++++++++++ .../transport/internal/HttpSenderOverHTTP.java | 10 +++++++++- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java index 8dd6212a2ef8..3f02d9823495 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/HttpClient.java @@ -135,7 +135,7 @@ public class HttpClient extends ContainerLifeCycle implements AutoCloseable private String defaultRequestContentType = "application/octet-stream"; private boolean useInputDirectByteBuffers = true; private boolean useOutputDirectByteBuffers = true; - private int maxRequestHeadersSize = 32 * 1024; + private int maxResponseHeadersSize = -1; private Sweeper destinationSweeper; @@ -1157,14 +1157,4 @@ public interface Aware { void setHttpClient(HttpClient httpClient); } - - public int getMaxRequestHeadersSize() - { - return maxRequestHeadersSize; - } - - public void setMaxRequestHeadersSize(int maxRequestHeadersSize) - { - this.maxRequestHeadersSize = maxRequestHeadersSize; - } } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpClientTransportOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpClientTransportOverHTTP.java index 61add8e5791b..925c9920491c 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpClientTransportOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/HttpClientTransportOverHTTP.java @@ -39,6 +39,8 @@ public class HttpClientTransportOverHTTP extends AbstractConnectorHttpClientTran private int headerCacheSize = 1024; private boolean headerCacheCaseSensitive; + private int maxRequestHeadersSize = 32 * 1024; + public HttpClientTransportOverHTTP() { this(1); @@ -127,4 +129,16 @@ public void setInitializeConnections(boolean initialize) { factory.setInitializeConnections(initialize); } + + /** + * @return The maximum allowed size in bytes for the HTTP request headers + */ + @ManagedAttribute("The maximum allowed size in bytes for the HTTP request headers") + public int getMaxRequestHeadersSize() { + return maxRequestHeadersSize; + } + + public void setMaxRequestHeadersSize(int maxRequestHeadersSize) { + this.maxRequestHeadersSize = maxRequestHeadersSize; + } } diff --git a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java index dfe4eb8ecb1e..03fb78d3d94d 100644 --- a/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java +++ b/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpSenderOverHTTP.java @@ -16,7 +16,9 @@ import java.nio.ByteBuffer; import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.client.HttpClientTransport; import org.eclipse.jetty.client.HttpRequestException; +import org.eclipse.jetty.client.transport.HttpClientTransportOverHTTP; import org.eclipse.jetty.client.transport.HttpExchange; import org.eclipse.jetty.client.transport.HttpRequest; import org.eclipse.jetty.client.transport.HttpSender; @@ -178,7 +180,13 @@ protected Action process() throws Exception } case HEADER_OVERFLOW: { - int maxRequestHeadersSize = httpClient.getMaxRequestHeadersSize(); + int maxRequestHeadersSize = -1; + //For HTTP1.1 only + HttpClientTransport transport = httpClient.getTransport(); + if (transport instanceof HttpClientTransportOverHTTP httpTransport) + { + maxRequestHeadersSize = httpTransport.getMaxRequestHeadersSize(); + } if (headerBuffer.capacity() < maxRequestHeadersSize) { RetainableByteBuffer newHeaderBuffer = bufferPool.acquire(maxRequestHeadersSize, useDirectByteBuffers); From 5035c260031d61eda20355e9a882c48d770ef501 Mon Sep 17 00:00:00 2001 From: Sheldon <shaoxt@gmail.com> Date: Thu, 7 Nov 2024 09:31:04 -0800 Subject: [PATCH 13/13] Move the test case --- .../eclipse/jetty/client/HttpClientTest.java | 207 +++++++++++++++++- .../client/http/HttpSenderOverHTTPTest.java | 198 ----------------- 2 files changed, 202 insertions(+), 203 deletions(-) diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java index 7d229d1d0315..1df46cbf8157 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientTest.java @@ -28,11 +28,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; -import java.util.Arrays; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Random; +import java.util.*; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Exchanger; @@ -56,6 +52,7 @@ import org.eclipse.jetty.http.HttpScheme; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpVersion; +import org.eclipse.jetty.io.ByteArrayEndPoint; import org.eclipse.jetty.io.ClientConnector; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.EndPoint; @@ -76,6 +73,7 @@ import org.eclipse.jetty.util.component.LifeCycle; import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ArgumentsSource; @@ -2012,4 +2010,203 @@ public void perform() .send(this); } } + + + private static Random rnd = new Random(); + private static final String CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890"; + + public static final int CHARS_LENGTH = CHARS.length(); + + protected static String getRandomString(int size) + { + StringBuilder sb = new StringBuilder(size); + while (sb.length() < size) + { // length of the random string. + int index = rnd.nextInt(CHARS_LENGTH); + sb.append(CHARS.charAt(index)); + } + return sb.toString(); + } + + @Test + public void testSmallHeadersSize() throws Exception + { + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + request.agent(getRandomString(888)); //More than the request buffer size, but less than the default max request headers size + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch successLatch = new CountDownLatch(1); + final CountDownLatch failureLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onSuccess(Request request) + { + successLatch.countDown(); + } + + @Override + public void onFailure(Request request, Throwable failure) + { + failureLatch.countDown(); + } + }); + connection.send(request, null); + + String requestString = endPoint.takeOutputString(); + assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(successLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testMaxRequestHeadersSize() throws Exception + { + byte[] buffer = new byte[32 * 1024]; + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + //More than the request buffer size, but less than the default max request headers size + + int desiredHeadersSize = 20 * 1024; + int currentHeadersSize = 0; + int i = 0; + while (currentHeadersSize < desiredHeadersSize) + { + final int index = i++; + final String headerValue = getRandomString(800); + final int headerSize = headerValue.length(); + currentHeadersSize += headerSize; + request.cookie(new HttpCookie() + { + @Override + public String getName() + { + return "large" + index; + } + + @Override + public String getValue() + { + return headerValue; + } + + @Override + public int getVersion() + { + return 0; + } + + @Override + public Map<String, String> getAttributes() + { + return new HashMap<>(); + } + }); + } + + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch successLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onSuccess(Request request) + { + successLatch.countDown(); + } + }); + connection.send(request, null); + + String requestString = endPoint.takeOutputString(); + assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(successLatch.await(5, TimeUnit.SECONDS)); + } + + @Test + public void testMaxRequestHeadersSizeOverflow() throws Exception + { + byte[] buffer = new byte[32 * 1024]; + ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); + HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); + destination.start(); + HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); + Request request = client.newRequest(URI.create("http://localhost/")); + //More than the request buffer size, but less than the default max request headers size + + int desiredHeadersSize = 35 * 1024; + int currentHeadersSize = 0; + int i = 0; + while (currentHeadersSize < desiredHeadersSize) + { + final int index = i++; + final String headerValue = getRandomString(800); + final int headerSize = headerValue.length(); + currentHeadersSize += headerSize; + request.cookie(new HttpCookie() + { + @Override + public String getName() + { + return "large" + index; + } + + @Override + public String getValue() + { + return headerValue; + } + + @Override + public int getVersion() + { + return 0; + } + + @Override + public Map<String, String> getAttributes() + { + return new HashMap<>(); + } + }); + } + + final CountDownLatch headersLatch = new CountDownLatch(1); + final CountDownLatch failureLatch = new CountDownLatch(1); + request.listener(new Request.Listener() + { + @Override + public void onHeaders(Request request) + { + headersLatch.countDown(); + } + + @Override + public void onFailure(Request request, Throwable failure) + { + failureLatch.countDown(); + } + }); + connection.send(request, null); + + assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); + assertTrue(failureLatch.await(5, TimeUnit.SECONDS)); + } } diff --git a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java index fc6eeb685ccb..6e1ddc1e0bae 100644 --- a/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java +++ b/jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpSenderOverHTTPTest.java @@ -306,202 +306,4 @@ public void onSuccess(Request request) assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); assertTrue(successLatch.await(5, TimeUnit.SECONDS)); } - - private static Random rnd = new Random(); - private static final String CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890"; - - public static final int CHARS_LENGTH = CHARS.length(); - - protected static String getRandomString(int size) - { - StringBuilder sb = new StringBuilder(size); - while (sb.length() < size) - { // length of the random string. - int index = rnd.nextInt(CHARS_LENGTH); - sb.append(CHARS.charAt(index)); - } - return sb.toString(); - } - - @Test - public void testSmallHeadersSize() throws Exception - { - ByteArrayEndPoint endPoint = new ByteArrayEndPoint(); - HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); - destination.start(); - HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); - Request request = client.newRequest(URI.create("http://localhost/")); - request.agent(getRandomString(888)); //More than the request buffer size, but less than the default max request headers size - final CountDownLatch headersLatch = new CountDownLatch(1); - final CountDownLatch successLatch = new CountDownLatch(1); - final CountDownLatch failureLatch = new CountDownLatch(1); - request.listener(new Request.Listener() - { - @Override - public void onHeaders(Request request) - { - headersLatch.countDown(); - } - - @Override - public void onSuccess(Request request) - { - successLatch.countDown(); - } - - @Override - public void onFailure(Request request, Throwable failure) - { - failureLatch.countDown(); - } - }); - connection.send(request, null); - - String requestString = endPoint.takeOutputString(); - assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); - assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); - assertTrue(successLatch.await(5, TimeUnit.SECONDS)); - } - - @Test - public void testMaxRequestHeadersSize() throws Exception - { - byte[] buffer = new byte[32 * 1024]; - ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); - HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); - destination.start(); - HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); - Request request = client.newRequest(URI.create("http://localhost/")); - //More than the request buffer size, but less than the default max request headers size - - int desiredHeadersSize = 20 * 1024; - int currentHeadersSize = 0; - int i = 0; - while (currentHeadersSize < desiredHeadersSize) - { - final int index = i++; - final String headerValue = getRandomString(800); - final int headerSize = headerValue.length(); - currentHeadersSize += headerSize; - request.cookie(new HttpCookie() - { - @Override - public String getName() - { - return "large" + index; - } - - @Override - public String getValue() - { - return headerValue; - } - - @Override - public int getVersion() - { - return 0; - } - - @Override - public Map<String, String> getAttributes() - { - return new HashMap<>(); - } - }); - } - - final CountDownLatch headersLatch = new CountDownLatch(1); - final CountDownLatch successLatch = new CountDownLatch(1); - request.listener(new Request.Listener() - { - @Override - public void onHeaders(Request request) - { - headersLatch.countDown(); - } - - @Override - public void onSuccess(Request request) - { - successLatch.countDown(); - } - }); - connection.send(request, null); - - String requestString = endPoint.takeOutputString(); - assertTrue(requestString.startsWith("GET / HTTP/1.1\r\nAccept-Encoding: gzip\r\n")); - assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); - assertTrue(successLatch.await(5, TimeUnit.SECONDS)); - } - - @Test - public void testMaxRequestHeadersSizeOverflow() throws Exception - { - byte[] buffer = new byte[32 * 1024]; - ByteArrayEndPoint endPoint = new ByteArrayEndPoint(buffer, buffer.length); - HttpDestination destination = new HttpDestination(client, new Origin("http", "localhost", 8080)); - destination.start(); - HttpConnectionOverHTTP connection = new HttpConnectionOverHTTP(endPoint, destination, new Promise.Adapter<Connection>()); - Request request = client.newRequest(URI.create("http://localhost/")); - //More than the request buffer size, but less than the default max request headers size - - int desiredHeadersSize = 35 * 1024; - int currentHeadersSize = 0; - int i = 0; - while (currentHeadersSize < desiredHeadersSize) - { - final int index = i++; - final String headerValue = getRandomString(800); - final int headerSize = headerValue.length(); - currentHeadersSize += headerSize; - request.cookie(new HttpCookie() - { - @Override - public String getName() - { - return "large" + index; - } - - @Override - public String getValue() - { - return headerValue; - } - - @Override - public int getVersion() - { - return 0; - } - - @Override - public Map<String, String> getAttributes() - { - return new HashMap<>(); - } - }); - } - - final CountDownLatch headersLatch = new CountDownLatch(1); - final CountDownLatch failureLatch = new CountDownLatch(1); - request.listener(new Request.Listener() - { - @Override - public void onHeaders(Request request) - { - headersLatch.countDown(); - } - - @Override - public void onFailure(Request request, Throwable failure) - { - failureLatch.countDown(); - } - }); - connection.send(request, null); - - assertTrue(headersLatch.await(5, TimeUnit.SECONDS)); - assertTrue(failureLatch.await(5, TimeUnit.SECONDS)); - } }