diff --git a/src/java.base/share/classes/sun/net/www/http/HttpClient.java b/src/java.base/share/classes/sun/net/www/http/HttpClient.java index 82ab4c199a5..ffab60e714e 100644 --- a/src/java.base/share/classes/sun/net/www/http/HttpClient.java +++ b/src/java.base/share/classes/sun/net/www/http/HttpClient.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1994, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1994, 2026, 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 @@ -438,8 +438,17 @@ public class HttpClient extends NetworkClient { } } + /** + * {@return {@code true}, if the connection to the server is still + * established and there is no stale data to be read; {@code false}, + * otherwise} + *

+ * A {@code true} return value indicates that the connection is reusable for + * an HTTP request. A {@code false} return value indicates that the + * connection is either lost or dirty, and it should be closed. + */ protected boolean available() { - boolean available = true; + boolean available = false; int old = -1; lock(); @@ -447,24 +456,24 @@ public class HttpClient extends NetworkClient { try { old = serverSocket.getSoTimeout(); serverSocket.setSoTimeout(1); - BufferedInputStream tmpbuf = - new BufferedInputStream(serverSocket.getInputStream()); - int r = tmpbuf.read(); + int r = serverSocket.getInputStream().read(); if (r == -1) { logFinest("HttpClient.available(): " + "read returned -1: not available"); - available = false; } } catch (SocketTimeoutException e) { logFinest("HttpClient.available(): " + "SocketTimeout: its available"); + available = true; } finally { if (old != -1) serverSocket.setSoTimeout(old); } } catch (IOException e) { - logFinest("HttpClient.available(): " + - "SocketException: not available"); + logFinest("HttpClient.available(): IOException: not available"); + // `SocketTimeoutException` might have set the return value to + // `true`, but consequently `serverSocket::setSoTimeout` might have + // failed. Hence, reset the return value, always. available = false; } finally { unlock(); diff --git a/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java b/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java index f5804cd83bd..8b63d52a989 100644 --- a/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java +++ b/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java @@ -23,7 +23,6 @@ * questions. */ - package sun.net.www.protocol.https; import java.io.IOException; @@ -36,7 +35,6 @@ import java.net.URL; import java.net.UnknownHostException; import java.net.InetSocketAddress; import java.net.Proxy; -import java.security.Principal; import java.security.cert.*; import java.util.ArrayList; import java.util.List; @@ -242,8 +240,11 @@ final class HttpsClient extends HttpClient if (ret != null && httpuc != null && httpuc.streaming() && "POST".equals(httpuc.getRequestMethod())) { - if (!ret.available()) + if (!ret.available()) { + ret.inCache = false; + ret.closeServer(); ret = null; + } } if (ret != null) { diff --git a/test/jdk/sun/net/www/http/HttpClient/IsAvailable.java b/test/jdk/sun/net/www/http/HttpClient/IsAvailable.java index 9e903003f51..65752737c62 100644 --- a/test/jdk/sun/net/www/http/HttpClient/IsAvailable.java +++ b/test/jdk/sun/net/www/http/HttpClient/IsAvailable.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2026, 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 @@ -23,60 +23,210 @@ /* * @test - * @bug 8009650 - * @summary HttpClient available() check throws SocketException when connection - * has been closed + * @bug 8009650 8180483 + * @summary Verifies the `HttpClient::available` behavior against several socket + * states. + * + * A "streaming POST" is controlled by `HttpURLConnection::streaming`, + * which becomes `true` when the caller uses either + * `setFixedLengthStreamingMode()` or `setChunkedStreamingMode()`, and + * *then writes the request body*. Non-streaming mode uses + * `PosterOutputStream.java`, which buffers the body and makes replay + * possible. Whereas, streaming mode allows user to directly write to + * the socket, so the body is not safely replayable. For many ordinary + * (i.e., non-streaming) requests, if a connection reuse fails, the + * stack can often recover by opening a new connection and retrying; + * see the retry logic in `HttpClient`. OTOH, for a "streaming POST", + * retry is dangerous or impossible because the body may already be in + * flight and is not buffered for replay. Hence, + * `HttpClient::available` tries to check whether the cached socket is + * still usable by doing a 1ms `read()`. Though that probe is + * destructive: it can consume and strand one byte from the socket + * input stream. Hence, probe failures should result in removal of the + * connection. + * * @modules java.base/sun.net * java.base/sun.net.www.http:+open * @library /test/lib + * + * @comment `othervm` is required since the `HttpURLConnection` logger state is tainted. + * @run junit/othervm ${test.main.class} */ +import java.io.Closeable; +import java.io.IOException; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.net.InetAddress; -import java.net.InetSocketAddress; +import java.net.Socket; import java.net.URL; import java.net.ServerSocket; + +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import sun.net.www.http.HttpClient; -import java.security.*; -import java.lang.reflect.Method; + +import java.util.function.Predicate; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.Logger; + import jdk.test.lib.net.URIBuilder; -public class IsAvailable { +import static java.nio.charset.StandardCharsets.US_ASCII; +import static jdk.test.lib.Utils.adjustTimeout; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; - public static void main(String[] args) throws Exception { - int readTimeout = 20; - ServerSocket ss = new ServerSocket(); - InetAddress loopback = InetAddress.getLoopbackAddress(); - ss.bind(new InetSocketAddress(loopback, 0)); +class IsAvailable { - try (ServerSocket toclose = ss) { + private static final Logger LOGGER = + Logger.getLogger(IsAvailable.class.getCanonicalName()); - URL url1 = URIBuilder.newBuilder() - .scheme("http") - .loopback() - .port(ss.getLocalPort()) - .toURL(); + // Class-level anchor to avoid the `HttpURLConnection` logger getting GC'ed + private static final Logger HUC_LOGGER = + Logger.getLogger("sun.net.www.protocol.http.HttpURLConnection"); - HttpClient c1 = HttpClient.New(url1); + @BeforeAll + static void init() { + increaseLoggerVerbosity(HUC_LOGGER); + } - Method available = HttpClient.class. - getDeclaredMethod("available", null); - available.setAccessible(true); + private static void increaseLoggerVerbosity(Logger logger) { + logger.setLevel(Level.FINEST); + var handler = new ConsoleHandler(); + handler.setLevel(Level.FINEST); + logger.addHandler(handler); + } - c1.setReadTimeout(readTimeout); - boolean a = (boolean) available.invoke(c1); - if (!a) { - throw new RuntimeException("connection should be available"); - } - if (c1.getReadTimeout() != readTimeout) { - throw new RuntimeException("read timeout has been altered"); - } + @Test + void testClosedSocket() throws Exception { + try (var infra = new Infra()) { - c1.closeServer(); + // Obtain the initial read timeout + int readTimeout = infra.readTimeout(); + + // Verify that the just established connection is available + LOGGER.info("Checking the connection (#1)..."); + assertTrue(infra.available(), "Freshly established connection should be available"); + assertEquals(readTimeout, infra.httpClient.getReadTimeout(), "Read-timeout should be restored"); + + // Verify that closing the socket removes the availability + LOGGER.info("Closing the socket..."); + infra.clientSocket.close(); + LOGGER.info("Checking the connection (#2)..."); + assertFalse(infra.available(), "Connection over closed socket should not be available"); + assertEquals(readTimeout, infra.httpClient.getReadTimeout(), "Read-timeout should be restored"); - a = (boolean) available.invoke(c1); - if (a) { - throw new RuntimeException("connection shouldn't be available"); - } } } + + @Test + void testSocketWithUnconsumedData() throws Exception { + try (var infra = new Infra()) { + + // Obtain the initial read timeout + int readTimeout = infra.readTimeout(); + + // Verify that the just established connection is available + LOGGER.info("Checking the connection (#1)..."); + assertTrue(infra.available(), "Freshly established connection should be available"); + assertEquals(readTimeout, infra.httpClient.getReadTimeout(), "Read-timeout should be restored"); + + // Write (unexpected) data to the socket + LOGGER.info("Writing data to the socket..."); + try (var clientSocketOutputStream = infra.clientSocket.getOutputStream()) { + clientSocketOutputStream.write("unexpected data".getBytes(US_ASCII)); + } + + // Writing to the socket on the server side may not make the data + // immediately visible to the client side. Make sure we wait long + // enough for the data to get delivered. + Thread.sleep(adjustTimeout(500)); + + // Verify that the presence of stale data on the socket removes the availability + LOGGER.info("Checking the connection (#2)..."); + assertFalse(infra.available(), "available should be false if it managed to read some data from the socket"); + assertEquals(readTimeout, infra.httpClient.getReadTimeout(), "Read-timeout should be restored"); + + } + } + + private static final class Infra implements Closeable { + + private static final InetAddress LOOPBACK_ADDRESS = InetAddress.getLoopbackAddress(); + + private static final Predicate AVAILABLE_ACCESSOR = findAvailableAccessor(); + + private static Predicate findAvailableAccessor() { + final MethodHandle availableMH; + try { + availableMH = MethodHandles + .privateLookupIn(HttpClient.class, MethodHandles.lookup()) + .findVirtual(HttpClient.class, "available", MethodType.methodType(boolean.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + throw new RuntimeException(e); + } + return httpClient -> { + try { + return (boolean) availableMH.invoke(httpClient); + } catch (Throwable e) { + throw new RuntimeException(e); + } + }; + } + + private final ServerSocket serverSocket; + + private final HttpClient httpClient; + + private final Socket clientSocket; + + private Infra() throws Exception { + this.serverSocket = new ServerSocket(0, 0, LOOPBACK_ADDRESS); + URL url = URIBuilder.newBuilder() + .scheme("http") + .loopback() + .port(serverSocket.getLocalPort()) + .toURL(); + this.httpClient = HttpClient.New(url); + this.clientSocket = serverSocket.accept(); + } + + private int readTimeout() { + int readTimeout = httpClient.getReadTimeout(); + assertNotEquals( + 1, readTimeout, + "When the read timeout is 1, " + + "we cannot validate whether \"available()\" has restored that value or not, " + + "since \"available()\" temporarily sets it to 1 as well"); + return readTimeout; + } + + private boolean available() { + return AVAILABLE_ACCESSOR.test(httpClient); + } + + @Override + public void close() { + closeQuietly("client socket", clientSocket); + closeQuietly("HttpClient", httpClient::closeServer); + closeQuietly("server socket", serverSocket); + } + + private static void closeQuietly(String name, Closeable closeable) { + if (closeable != null) { + try { + closeable.close(); + } catch (IOException e) { + LOGGER.warning("Failed closing " + name + ": " + e); + } + } + } + + } + }