infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-04-27 23:31:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

8180483: sun.net.www.protocol.http.HttpClient::available may consume one character

Browse Source 
Reviewed-by: dfuchs
This commit is contained in:
Volkan Yazici 2026-04-24 10:12:04 +00:00
parent 9813fd4d62
commit 7fcf2bb3ef
3 changed files with 207 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
src/java.base/share/classes/sun/net/www/http/HttpClient.java
View File

@ -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}
* <p>
* 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();

7
src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java
View File

@ -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) {

222
test/jdk/sun/net/www/http/HttpClient/IsAvailable.java
View File

@ -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<HttpClient> AVAILABLE_ACCESSOR = findAvailableAccessor();
private static Predicate<HttpClient> 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);
}
}
}
}
}