Skip to content

Commit 3ccb3c0

Browse files
jaikiranjaikiran
committed
8305906: HttpClient may use incorrect key when finding pooled HTTP/2 connection for IPv6 address
Reviewed-by: djelinski, dfuchs
1 parent a25b7b8 commit 3ccb3c0

File tree

3 files changed

+190
-11
lines changed

3 files changed

+190
-11
lines changed

‎src/java.net.http/share/classes/jdk/internal/net/http/Http2ClientImpl.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,7 @@ class Http2ClientImpl {
9898
*/
9999
CompletableFuture<Http2Connection> getConnectionFor(HttpRequestImpl req,
100100
Exchange<?> exchange) {
101-
URI uri = req.uri();
102-
InetSocketAddress proxy = req.proxy();
103-
String key = Http2Connection.keyFor(uri, proxy);
101+
String key = Http2Connection.keyFor(req);
104102

105103
lock.lock();
106104
try {

‎src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ private Http2Connection(HttpRequestImpl request,
454454
this(connection,
455455
h2client,
456456
1,
457-
keyFor(request.uri(), request.proxy()));
457+
keyFor(request));
458458

459459
Log.logTrace("Connection send window size {0} ", windowController.connectionWindowSize());
460460

@@ -595,15 +595,13 @@ static String keyFor(HttpConnection connection) {
595595
return keyString(isSecure, proxyAddr, addr.getHostString(), addr.getPort());
596596
}
597597

598-
static String keyFor(URI uri, InetSocketAddress proxy) {
599-
boolean isSecure = uri.getScheme().equalsIgnoreCase("https");
600-
601-
String host = uri.getHost();
602-
int port = uri.getPort();
603-
return keyString(isSecure, proxy, host, port);
598+
static String keyFor(final HttpRequestImpl request) {
599+
final InetSocketAddress targetAddr = request.getAddress();
600+
final InetSocketAddress proxy = request.proxy();
601+
final boolean secure = request.secure();
602+
return keyString(secure, proxy, targetAddr.getHostString(), targetAddr.getPort());
604603
}
605604

606-
607605
// Compute the key for an HttpConnection in the Http2ClientImpl pool:
608606
// The key string follows one of the three forms below:
609607
// {C,S}:H:host:port

‎test/jdk/java/net/httpclient/http2/ConnectionReuseTest.java‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
import java.io.IOException;
25+
import java.io.OutputStream;
26+
import java.net.InetSocketAddress;
27+
import java.net.URI;
28+
import java.net.http.HttpClient;
29+
import java.net.http.HttpRequest;
30+
import java.net.http.HttpResponse;
31+
import java.net.http.HttpResponse.BodyHandlers;
32+
import java.nio.charset.StandardCharsets;
33+
import java.util.ArrayList;
34+
import java.util.List;
35+
import java.util.stream.Stream;
36+
37+
import javax.net.ssl.SSLContext;
38+
39+
import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestExchange;
40+
import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestHandler;
41+
import jdk.httpclient.test.lib.common.HttpServerAdapters.HttpTestServer;
42+
import jdk.test.lib.net.IPSupport;
43+
import jdk.test.lib.net.SimpleSSLContext;
44+
import org.junit.jupiter.api.AfterAll;
45+
import org.junit.jupiter.api.Assumptions;
46+
import org.junit.jupiter.api.BeforeAll;
47+
import org.junit.jupiter.params.ParameterizedTest;
48+
import org.junit.jupiter.params.provider.Arguments;
49+
import org.junit.jupiter.params.provider.MethodSource;
50+
import static java.net.http.HttpClient.Builder.NO_PROXY;
51+
import static java.net.http.HttpClient.Version.HTTP_2;
52+
import static org.junit.jupiter.api.Assertions.assertEquals;
53+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
54+
import static org.junit.jupiter.api.Assertions.assertNotNull;
55+
56+
/*
57+
* @test
58+
* @bug 8305906
59+
* @summary verify that the HttpClient pools and reuses a connection for HTTP/2 requests
60+
* @library /test/lib /test/jdk/java/net/httpclient/lib
61+
* @build jdk.test.lib.net.SimpleSSLContext
62+
* jdk.test.lib.net.IPSupport
63+
* jdk.httpclient.test.lib.common.HttpServerAdapters
64+
*
65+
* @run junit ConnectionReuseTest
66+
* @run junit/othervm -Djava.net.preferIPv6Addresses=true
67+
* -Djdk.internal.httpclient.debug=true ConnectionReuseTest
68+
*/
69+
public class ConnectionReuseTest {
70+
71+
private static SSLContext sslContext;
72+
private static HttpTestServer http2_Server; // h2 server over HTTP
73+
private static HttpTestServer https2_Server; // h2 server over HTTPS
74+
75+
@BeforeAll
76+
public static void beforeAll() throws Exception {
77+
if (IPSupport.preferIPv6Addresses()) {
78+
IPSupport.printPlatformSupport(System.err); // for debug purposes
79+
// this test is run with -Djava.net.preferIPv6Addresses=true, so skip (all) tests
80+
// if IPv6 isn't supported on this host
81+
Assumptions.assumeTrue(IPSupport.hasIPv6(), "Skipping tests - IPv6 is not supported");
82+
}
83+
sslContext = new SimpleSSLContext().get();
84+
assertNotNull(sslContext, "Unexpected null sslContext");
85+
86+
http2_Server = HttpTestServer.create(HTTP_2);
87+
http2_Server.addHandler(new Handler(), "/");
88+
http2_Server.start();
89+
System.out.println("Started HTTP v2 server at " + http2_Server.serverAuthority());
90+
91+
https2_Server = HttpTestServer.create(HTTP_2, sslContext);
92+
https2_Server.addHandler(new Handler(), "/");
93+
https2_Server.start();
94+
System.out.println("Started HTTPS v2 server at " + https2_Server.serverAuthority());
95+
}
96+
97+
@AfterAll
98+
public static void afterAll() {
99+
if (https2_Server != null) {
100+
System.out.println("Stopping server " + https2_Server);
101+
https2_Server.stop();
102+
}
103+
if (http2_Server != null) {
104+
System.out.println("Stopping server " + http2_Server);
105+
http2_Server.stop();
106+
}
107+
}
108+
109+
private static Stream<Arguments> requestURIs() throws Exception {
110+
final List<Arguments> arguments = new ArrayList<>();
111+
// h2 over HTTPS
112+
arguments.add(Arguments.of(new URI("https://" + https2_Server.serverAuthority() + "/")));
113+
// h2 over HTTP
114+
arguments.add(Arguments.of(new URI("http://" + http2_Server.serverAuthority() + "/")));
115+
if (IPSupport.preferIPv6Addresses()) {
116+
if (https2_Server.getAddress().getAddress().isLoopbackAddress()) {
117+
// h2 over HTTPS, use the short form of the host, in the request URI
118+
arguments.add(Arguments.of(new URI("https://[::1]:" +
119+
https2_Server.getAddress().getPort() + "/")));
120+
}
121+
if (http2_Server.getAddress().getAddress().isLoopbackAddress()) {
122+
// h2 over HTTP, use the short form of the host, in the request URI
123+
arguments.add(Arguments.of(new URI("http://[::1]:" +
124+
http2_Server.getAddress().getPort() + "/")));
125+
}
126+
}
127+
return arguments.stream();
128+
}
129+
130+
/**
131+
* Uses a single instance of a HttpClient and issues multiple requests to {@code requestURI}
132+
* and expects that each of the request internally uses the same connection
133+
*/
134+
@ParameterizedTest
135+
@MethodSource("requestURIs")
136+
public void testConnReuse(final URI requestURI) throws Exception {
137+
final HttpClient.Builder builder = HttpClient.newBuilder()
138+
.proxy(NO_PROXY).sslContext(sslContext);
139+
final HttpRequest req = HttpRequest.newBuilder().uri(requestURI)
140+
.GET().version(HTTP_2).build();
141+
try (final HttpClient client = builder.build()) {
142+
String clientConnAddr = null;
143+
for (int i = 1; i <= 5; i++) {
144+
System.out.println("Issuing request(" + i + ") " + req);
145+
final HttpResponse<String> resp = client.send(req, BodyHandlers.ofString());
146+
assertEquals(200, resp.statusCode(), "unexpected response code");
147+
final String respBody = resp.body();
148+
System.out.println("Server side handler responded to a request from " + respBody);
149+
assertNotEquals(Handler.UNKNOWN_CLIENT_ADDR, respBody,
150+
"server handler couldn't determine client address in request");
151+
if (i == 1) {
152+
// for the first request we just keep track of the client connection address
153+
// that got used for this request
154+
clientConnAddr = respBody;
155+
} else {
156+
// verify that the client connection used to issue the request is the same
157+
// as the previous request's client connection
158+
assertEquals(clientConnAddr, respBody, "HttpClient unexpectedly used a" +
159+
" different connection for request(" + i + ")");
160+
}
161+
}
162+
}
163+
}
164+
165+
private static final class Handler implements HttpTestHandler {
166+
167+
private static final String UNKNOWN_CLIENT_ADDR = "unknown";
168+
169+
@Override
170+
public void handle(final HttpTestExchange t) throws IOException {
171+
final InetSocketAddress clientAddr = t.getRemoteAddress();
172+
System.out.println("Handling request " + t.getRequestURI() + " from " + clientAddr);
173+
// we write out the client address into the response body
174+
final byte[] responseBody = clientAddr == null
175+
? UNKNOWN_CLIENT_ADDR.getBytes(StandardCharsets.UTF_8)
176+
: clientAddr.toString().getBytes(StandardCharsets.UTF_8);
177+
t.sendResponseHeaders(200, responseBody.length);
178+
try (final OutputStream os = t.getResponseBody()) {
179+
os.write(responseBody);
180+
}
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)