diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java index b14d76d8dbadb..a02506cff5c76 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java @@ -174,15 +174,20 @@ private static boolean hostnameVerificationDisabledValue() { public static final int SLICE_THRESHOLD = 32; /** - * Allocated buffer size. Must never be higher than 16K. But can be lower - * if smaller allocation units preferred. HTTP/2 mandates that all - * implementations support frame payloads of at least 16K. + * The capacity of ephemeral {@link ByteBuffer}s allocated to pass data to and from the client. + * It is ensured to have a value between 1 and 2^14 (16,384). */ - private static final int DEFAULT_BUFSIZE = 16 * 1024; - public static final int BUFSIZE = getIntegerNetProperty( - "jdk.httpclient.bufsize", DEFAULT_BUFSIZE - ); + "jdk.httpclient.bufsize", 1, + // We cap at 2^14 (16,384) for two main reasons: + // - The initial frame size is 2^14 (RFC 9113) + // - SSL record layer fragments data in chunks of 2^14 bytes or less (RFC 5246) + 1 << 14, + // We choose 2^14 (16,384) as the default, because: + // 1. It maximizes throughput within the limits described above + // 2. It is small enough to not create a GC bottleneck when it is partially filled + 1 << 14, + true); public static final BiPredicate ACCEPT_ALL = (x,y) -> true; diff --git a/src/java.net.http/share/classes/module-info.java b/src/java.net.http/share/classes/module-info.java index 392385136b084..48f23953ad088 100644 --- a/src/java.net.http/share/classes/module-info.java +++ b/src/java.net.http/share/classes/module-info.java @@ -48,7 +48,9 @@ * depending on the context. These restrictions cannot be overridden by this property. * *

  • {@systemProperty jdk.httpclient.bufsize} (default: 16384 bytes or 16 kB)
    - * The size to use for internal allocated buffers in bytes. + * The capacity of internal ephemeral buffers allocated to pass data to and from the + * client, in bytes. Valid values are in the range [1, 2^14 (16384)]. + * If an invalid value is provided, the default value is used. *

    • *

  • {@systemProperty jdk.httpclient.connectionPoolSize} (default: 0)
    * The maximum number of connections to keep in the HTTP/1.1 keep alive cache. A value of 0 diff --git a/test/jdk/java/net/httpclient/BufferSize1Test.java b/test/jdk/java/net/httpclient/BufferSize1Test.java new file mode 100644 index 0000000000000..842dc06a630ac --- /dev/null +++ b/test/jdk/java/net/httpclient/BufferSize1Test.java @@ -0,0 +1,158 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.httpclient.test.lib.common.HttpServerAdapters; +import jdk.internal.net.http.common.Utils; +import jdk.test.lib.net.SimpleSSLContext; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import javax.net.ssl.SSLContext; +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpClient.Version; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; + +import static java.net.http.HttpClient.Builder.NO_PROXY; +import static java.net.http.HttpClient.Version.HTTP_1_1; +import static java.net.http.HttpClient.Version.HTTP_2; +import static java.net.http.HttpClient.Version.HTTP_3; +import static java.net.http.HttpOption.H3_DISCOVERY; +import static java.net.http.HttpOption.Http3DiscoveryMode.HTTP_3_URI_ONLY; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/* + * @test id + * @bug 8367976 + * @summary Verifies that setting the `jdk.httpclient.bufsize` system property + * to its lowest possible value, 1, does not wedge the client + * @library /test/jdk/java/net/httpclient/lib + * /test/lib + * @run junit/othervm -Djdk.httpclient.bufsize=1 BufferSize1Test + */ + +class BufferSize1Test implements HttpServerAdapters { + + @BeforeAll + static void verifyBufferSize() { + assertEquals(1, Utils.BUFSIZE); + } + + static Object[][] testArgs() { + return new Object[][]{ + {HTTP_1_1, false}, + {HTTP_1_1, true}, + {HTTP_2, false}, + {HTTP_2, true}, + {HTTP_3, true} + }; + } + + @ParameterizedTest + @MethodSource("testArgs") + void test(Version version, boolean secure) throws Exception { + + // Create the server + var sslContext = secure || HTTP_3.equals(version) ? new SimpleSSLContext().get() : null; + try (var server = switch (version) { + case HTTP_1_1, HTTP_2 -> HttpTestServer.create(version, sslContext); + case HTTP_3 -> HttpTestServer.create(HTTP_3_URI_ONLY, sslContext); + }) { + + // Add the handler and start the server + var serverHandlerPath = "/" + BufferSize1Test.class.getSimpleName(); + server.addHandler(new HttpTestEchoHandler(), serverHandlerPath); + server.start(); + + // Create the client + try (var client = createClient(version, sslContext)) { + + // Create the request with body to ensure that `ByteBuffer`s + // will be used throughout the entire end-to-end interaction. + byte[] requestBodyBytes = "body".repeat(1000).getBytes(StandardCharsets.US_ASCII); + var request = createRequest(sslContext, server, serverHandlerPath, version, requestBodyBytes); + + // Execute and verify the request. + // Do it twice to cover code paths before and after a protocol upgrade. + requestAndVerify(client, request, requestBodyBytes); + requestAndVerify(client, request, requestBodyBytes); + + } + + } + + } + + private HttpClient createClient(Version version, SSLContext sslContext) { + var clientBuilder = newClientBuilderForH3() + .proxy(NO_PROXY) + .version(version); + if (sslContext != null) { + clientBuilder.sslContext(sslContext); + } + return clientBuilder.build(); + } + + private static HttpRequest createRequest( + SSLContext sslContext, + HttpTestServer server, + String serverHandlerPath, + Version version, + byte[] requestBodyBytes) { + var requestUri = URI.create(String.format( + "%s://%s%s/x", + sslContext == null ? "http" : "https", + server.serverAuthority(), + serverHandlerPath)); + var requestBuilder = HttpRequest + .newBuilder(requestUri) + .version(version) + .POST(HttpRequest.BodyPublishers.ofByteArray(requestBodyBytes)); + if (HTTP_3.equals(version)) { + requestBuilder.setOption(H3_DISCOVERY, HTTP_3_URI_ONLY); + } + return requestBuilder.build(); + } + + private static void requestAndVerify(HttpClient client, HttpRequest request, byte[] requestBodyBytes) + throws IOException, InterruptedException { + var response = client.send(request, HttpResponse.BodyHandlers.ofByteArray()); + if (response.statusCode() != 200) { + throw new AssertionError("Was expecting status code 200, found: " + response.statusCode()); + } + byte[] responseBodyBytes = response.body(); + int mismatchIndex = Arrays.mismatch(requestBodyBytes, responseBodyBytes); + assertTrue( + mismatchIndex < 0, + String.format( + "Response body (%s bytes) mismatches the request body (%s bytes) at index %s!", + responseBodyBytes.length, requestBodyBytes.length, mismatchIndex)); + } + +} diff --git a/test/jdk/java/net/httpclient/BufferSizePropertyClampTest.java b/test/jdk/java/net/httpclient/BufferSizePropertyClampTest.java new file mode 100644 index 0000000000000..caef0a58a6dc7 --- /dev/null +++ b/test/jdk/java/net/httpclient/BufferSizePropertyClampTest.java @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import jdk.internal.net.http.common.Utils; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.text.MessageFormat; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.logging.Handler; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/* + * @test + * @bug 8367976 + * @summary Verifies that the `jdk.httpclient.bufsize` system property is + * clamped correctly + * + * @library /test/lib + * + * @comment `-Djdk.httpclient.HttpClient.log=errors` is needed to enable + * logging and verify that invalid input gets logged + * @run junit/othervm + * -Djdk.httpclient.HttpClient.log=errors + * -Djdk.httpclient.bufsize=-1 + * BufferSizePropertyClampTest + * @run junit/othervm + * -Djdk.httpclient.HttpClient.log=errors + * -Djdk.httpclient.bufsize=0 + * BufferSizePropertyClampTest + * @run junit/othervm + * -Djdk.httpclient.HttpClient.log=errors + * -Djdk.httpclient.bufsize=16385 + * BufferSizePropertyClampTest + */ + +class BufferSizePropertyClampTest { + + /** Anchor to avoid the {@code Logger} instance get GC'ed */ + private static final Logger CLIENT_LOGGER = + Logger.getLogger("jdk.httpclient.HttpClient"); + + private static final List CLIENT_LOGGER_MESSAGES = + Collections.synchronizedList(new ArrayList<>()); + + @BeforeAll + static void registerLoggerHandler() { + CLIENT_LOGGER.addHandler(new Handler() { + + @Override + public void publish(LogRecord record) { + var message = MessageFormat.format(record.getMessage(), record.getParameters()); + CLIENT_LOGGER_MESSAGES.add(message); + } + + @Override + public void flush() { + // Do nothing + } + + @Override + public void close() { + // Do nothing + } + + }); + } + + @Test + void test() { + assertEquals(16384, Utils.BUFSIZE); + assertEquals( + 1, CLIENT_LOGGER_MESSAGES.size(), + "Unexpected number of logger messages: " + CLIENT_LOGGER_MESSAGES); + var expectedMessage = "ERROR: Property value for jdk.httpclient.bufsize=" + + System.getProperty("jdk.httpclient.bufsize") + + " not in [1..16384]: using default=16384"; + assertEquals(expectedMessage, CLIENT_LOGGER_MESSAGES.getFirst().replaceAll(",", "")); + } + +} diff --git a/test/jdk/java/net/httpclient/HttpRequestBodyPublishers/OfByteArrayTest.java b/test/jdk/java/net/httpclient/HttpRequestBodyPublishers/OfByteArrayTest.java index 9973272b43513..19f7369125dee 100644 --- a/test/jdk/java/net/httpclient/HttpRequestBodyPublishers/OfByteArrayTest.java +++ b/test/jdk/java/net/httpclient/HttpRequestBodyPublishers/OfByteArrayTest.java @@ -43,8 +43,6 @@ * @run junit OfByteArrayTest * * @comment Using `main/othervm` to initiate tests that depend on a custom-configured JVM - * @run main/othervm -Djdk.httpclient.bufsize=-1 OfByteArrayTest testInvalidBufferSize - * @run main/othervm -Djdk.httpclient.bufsize=0 OfByteArrayTest testInvalidBufferSize * @run main/othervm -Djdk.httpclient.bufsize=3 OfByteArrayTest testChunking "" 0 0 "" * @run main/othervm -Djdk.httpclient.bufsize=3 OfByteArrayTest testChunking a 0 0 "" * @run main/othervm -Djdk.httpclient.bufsize=3 OfByteArrayTest testChunking a 1 0 "" @@ -88,7 +86,6 @@ void testInvalidOffsetOrLength(String contentText, int offset, int length) { */ public static void main(String[] args) throws InterruptedException { switch (args[0]) { - case "testInvalidBufferSize" -> testInvalidBufferSize(); case "testChunking" -> testChunking( parseStringArg(args[1]), Integer.parseInt(args[2]), @@ -102,10 +99,6 @@ private static String parseStringArg(String arg) { return arg == null || arg.trim().equals("\"\"") ? "" : arg; } - private static void testInvalidBufferSize() { - assertThrows(IllegalArgumentException.class, () -> HttpRequest.BodyPublishers.ofByteArray(new byte[1])); - } - private static void testChunking( String contentText, int offset, int length, String expectedBuffersText) throws InterruptedException {