Skip to content

Commit 4b4cfe7

Browse files
GoeLinGoeLin
committed
8286171: HttpClient/2 : Expect:100-Continue blocks indefinitely when response is not 100
Reviewed-by: rschmelter Backport-of: 267144311c96109421b897b359c155a963661d31
1 parent c159a0c commit 4b4cfe7

File tree

6 files changed

+326
-3
lines changed

6 files changed

+326
-3
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,8 +464,8 @@ private CompletableFuture<Response> expectContinue(ExchangeImpl<T> ex) {
464464
"Unable to handle 101 while waiting for 100");
465465
return MinimalFuture.failedFuture(failed);
466466
}
467-
return exchImpl.readBodyAsync(this::ignoreBody, false, parentExecutor)
468-
.thenApply(v -> r1);
467+
exchImpl.expectContinueFailed(rcode);
468+
return MinimalFuture.completedFuture(r1);
469469
}
470470
});
471471
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,8 @@ HttpBodySubscriberWrapper<T> createResponseSubscriber(HttpResponse.BodyHandler<T
260260
// Needed to handle cancellation during the upgrade from
261261
// Http1Exchange to Stream
262262
void upgraded() { }
263+
264+
// Called when server returns non 100 response to
265+
// an Expect-Continue
266+
void expectContinueFailed(int rcode) { }
263267
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,4 +863,14 @@ final HttpClientImpl client() {
863863
String dbgString() {
864864
return "Http1Exchange";
865865
}
866+
867+
@Override
868+
void expectContinueFailed(int rcode) {
869+
var response = this.response;
870+
if (response != null) {
871+
// Sets a flag which closes the connection locally when
872+
// onFinished() is called
873+
response.closeWhenFinished();
874+
}
875+
}
866876
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ class Http1Response<T> {
6666
private final Http1AsyncReceiver asyncReceiver;
6767
private volatile EOFException eof;
6868
private volatile BodyParser bodyParser;
69+
private volatile boolean closeWhenFinished;
6970
// max number of bytes of (fixed length) body to ignore on redirect
7071
private static final int MAX_IGNORE = 1024;
7172

@@ -404,7 +405,11 @@ public <U> CompletableFuture<U> readBody(HttpResponse.BodySubscriber<U> p,
404405

405406
private void onFinished() {
406407
asyncReceiver.clear();
407-
if (return2Cache) {
408+
if (closeWhenFinished) {
409+
if (debug.on())
410+
debug.log("Closing Connection when finished");
411+
connection.close();
412+
} else if (return2Cache) {
408413
Log.logTrace("Attempting to return connection to the pool: {0}", connection);
409414
// TODO: need to do something here?
410415
// connection.setAsyncCallbacks(null, null, null);
@@ -416,6 +421,10 @@ private void onFinished() {
416421
}
417422
}
418423

424+
void closeWhenFinished() {
425+
closeWhenFinished = true;
426+
}
427+
419428
HttpHeaders responseHeaders() {
420429
return headers;
421430
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,13 @@ private boolean consumed(DataFrame df) {
310310
return endStream;
311311
}
312312

313+
@Override
314+
void expectContinueFailed(int rcode) {
315+
// Have to mark request as sent, due to no request body being sent in the
316+
// event of a 417 Expectation Failed or some other non 100 response code
317+
requestSent();
318+
}
319+
313320
// This method is called by Http2Connection::decrementStreamCount in order
314321
// to make sure that the stream count is decremented only once for
315322
// a given stream.

‎test/jdk/java/net/httpclient/ExpectContinueTest.java

Lines changed: 293 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,293 @@
1+
/*
2+
* Copyright (c) 2022, 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. Oracle designates this
8+
* particular file as subject to the "Classpath" exception as provided
9+
* by Oracle in the LICENSE file that accompanied this code.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
26+
/*
27+
* @test
28+
* @summary Tests that when the httpclient sends a 100 Expect Continue header and receives
29+
* a response code of 417 Expectation Failed, that the client does not hang
30+
* indefinitely and closes the connection.
31+
* @bug 8286171
32+
* @library /test/lib /test/jdk/java/net/httpclient/lib
33+
* @build jdk.httpclient.test.lib.common.HttpServerAdapters
34+
* @run testng/othervm ExpectContinueTest
35+
*/
36+
37+
38+
import com.sun.net.httpserver.HttpServer;
39+
40+
import org.testng.annotations.AfterTest;
41+
import org.testng.annotations.BeforeTest;
42+
import org.testng.annotations.DataProvider;
43+
import org.testng.annotations.Test;
44+
45+
import javax.net.ServerSocketFactory;
46+
import java.io.BufferedReader;
47+
import java.io.Closeable;
48+
import java.io.IOException;
49+
import java.io.InputStream;
50+
import java.io.InputStreamReader;
51+
import java.io.OutputStream;
52+
import java.io.OutputStreamWriter;
53+
import java.io.PrintWriter;
54+
import java.io.Writer;
55+
import java.net.InetAddress;
56+
import java.net.InetSocketAddress;
57+
import java.net.ServerSocket;
58+
import java.net.Socket;
59+
import java.net.URI;
60+
import java.net.http.HttpClient;
61+
import java.net.http.HttpRequest;
62+
import java.net.http.HttpResponse;
63+
import java.util.StringTokenizer;
64+
import java.util.concurrent.CompletableFuture;
65+
import jdk.httpclient.test.lib.common.HttpServerAdapters;
66+
import jdk.httpclient.test.lib.http2.Http2TestServer;
67+
68+
import static java.net.http.HttpClient.Version.HTTP_1_1;
69+
import static java.net.http.HttpClient.Version.HTTP_2;
70+
import static java.nio.charset.StandardCharsets.UTF_8;
71+
import static org.testng.Assert.assertEquals;
72+
73+
public class ExpectContinueTest implements HttpServerAdapters {
74+
75+
HttpTestServer http1TestServer; // HTTP/1.1
76+
Http1HangServer http1HangServer;
77+
HttpTestServer http2TestServer; // HTTP/2
78+
79+
URI getUri;
80+
URI postUri;
81+
URI hangUri;
82+
URI h2getUri;
83+
URI h2postUri;
84+
URI h2hangUri;
85+
86+
static final String EXPECTATION_FAILED_417 = "417 Expectation Failed";
87+
88+
@BeforeTest
89+
public void setup() throws Exception {
90+
InetSocketAddress saHang = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
91+
92+
http1TestServer = HttpTestServer.create(HTTP_1_1);
93+
http1TestServer.addHandler(new GetHandler(), "/http1/get");
94+
http1TestServer.addHandler(new PostHandler(), "/http1/post");
95+
getUri = URI.create("http://" + http1TestServer.serverAuthority() + "/http1/get");
96+
postUri = URI.create("http://" + http1TestServer.serverAuthority() + "/http1/post");
97+
98+
// Due to limitations of the above Http1 Server, a manual approach is taken to test the hanging with the
99+
// httpclient using Http1 so that the correct response header can be returned for the test case
100+
http1HangServer = new Http1HangServer(saHang);
101+
hangUri = URI.create("http://" + http1HangServer.ia.getCanonicalHostName() + ":" + http1HangServer.port + "/http1/hang");
102+
103+
104+
http2TestServer = HttpTestServer.create(HTTP_2);
105+
http2TestServer.addHandler(new GetHandler(), "/http2/get");
106+
http2TestServer.addHandler(new PostHandler(), "/http2/post");
107+
http2TestServer.addHandler(new PostHandlerCantContinue(), "/http2/hang");
108+
h2getUri = URI.create("http://" + http2TestServer.serverAuthority() + "/http2/get");
109+
h2postUri = URI.create("http://" + http2TestServer.serverAuthority() + "/http2/post");
110+
h2hangUri = URI.create("http://" + http2TestServer.serverAuthority() + "/http2/hang");
111+
112+
http1TestServer.start();
113+
http1HangServer.start();
114+
http2TestServer.start();
115+
}
116+
117+
@AfterTest
118+
public void teardown() throws IOException {
119+
http1TestServer.stop();
120+
http1HangServer.close();
121+
http2TestServer.stop();
122+
}
123+
124+
static class GetHandler implements HttpTestHandler {
125+
126+
@Override
127+
public void handle(HttpTestExchange exchange) throws IOException {
128+
try (InputStream is = exchange.getRequestBody();
129+
OutputStream os = exchange.getResponseBody()) {
130+
is.readAllBytes();
131+
byte[] bytes = "RESPONSE_BODY".getBytes(UTF_8);
132+
exchange.sendResponseHeaders(200, bytes.length);
133+
os.write(bytes);
134+
}
135+
}
136+
}
137+
138+
static class PostHandler implements HttpTestHandler {
139+
140+
@Override
141+
public void handle(HttpTestExchange exchange) throws IOException {
142+
// Http1 server has already sent 100 response at this point but not Http2 server
143+
if (exchange.getExchangeVersion().equals(HttpClient.Version.HTTP_2)) {
144+
// Send 100 Headers, tell client that we're ready for body
145+
exchange.sendResponseHeaders(100, 0);
146+
}
147+
148+
// Read body from client and acknowledge with 200
149+
try (InputStream is = exchange.getRequestBody();
150+
OutputStream os = exchange.getResponseBody()) {
151+
is.readAllBytes();
152+
exchange.sendResponseHeaders(200, 0);
153+
}
154+
}
155+
}
156+
157+
static class PostHandlerCantContinue implements HttpTestHandler {
158+
159+
@Override
160+
public void handle(HttpTestExchange exchange) throws IOException {
161+
//Send 417 Headers, tell client to not send body
162+
try (InputStream is = exchange.getRequestBody();
163+
OutputStream os = exchange.getResponseBody()) {
164+
byte[] bytes = EXPECTATION_FAILED_417.getBytes();
165+
exchange.sendResponseHeaders(417, bytes.length);
166+
os.write(bytes);
167+
}
168+
}
169+
}
170+
171+
static class Http1HangServer extends Thread implements Closeable {
172+
173+
final ServerSocket ss;
174+
final InetAddress ia;
175+
final int port;
176+
volatile boolean closed = false;
177+
volatile Socket client;
178+
179+
Http1HangServer(InetSocketAddress sa) throws IOException {
180+
ss = ServerSocketFactory.getDefault()
181+
.createServerSocket(sa.getPort(), -1, sa.getAddress());
182+
ia = ss.getInetAddress();
183+
port = ss.getLocalPort();
184+
}
185+
186+
@Override
187+
public void run() {
188+
byte[] bytes = EXPECTATION_FAILED_417.getBytes();
189+
190+
while (!closed) {
191+
try {
192+
// Not using try with resources here as we expect the client to close resources when
193+
// 417 is received
194+
client = ss.accept();
195+
InputStream is = client.getInputStream();
196+
OutputStream os = client.getOutputStream();
197+
198+
BufferedReader reader = new BufferedReader(new InputStreamReader(is));
199+
Writer w = new OutputStreamWriter(os, UTF_8);
200+
PrintWriter pw = new PrintWriter(w);
201+
202+
StringBuilder response = new StringBuilder();
203+
String line = null;
204+
StringBuilder reqBuilder = new StringBuilder();
205+
while (!(line = reader.readLine()).isEmpty()) {
206+
reqBuilder.append(line + "\r\n");
207+
}
208+
String req = reqBuilder.toString();
209+
System.err.println("Http1HangServer received: " + req);
210+
StringTokenizer tokenizer = new StringTokenizer(req);
211+
String method = tokenizer.nextToken();
212+
String path = tokenizer.nextToken();
213+
String version = tokenizer.nextToken();
214+
215+
boolean validRequest = method.equals("POST") && path.equals("/http1/hang")
216+
&& version.equals("HTTP/1.1");
217+
// If correct request, send 417 reply. Otherwise, wait for correct one
218+
if (validRequest) {
219+
closed = true;
220+
response.append("HTTP/1.1 417 Expectation Failed\r\n")
221+
.append("Content-Length: ")
222+
.append(0)
223+
.append("\r\n\r\n");
224+
pw.print(response);
225+
pw.flush();
226+
227+
os.write(bytes);
228+
os.flush();
229+
} else {
230+
client.close();
231+
}
232+
} catch (IOException e) {
233+
closed = true;
234+
e.printStackTrace();
235+
}
236+
}
237+
}
238+
239+
@Override
240+
public void close() throws IOException {
241+
if (client != null) client.close();
242+
if (ss != null) ss.close();
243+
}
244+
}
245+
246+
@DataProvider(name = "uris")
247+
public Object[][] urisData() {
248+
return new Object[][]{
249+
{ getUri, postUri, hangUri, HTTP_1_1 },
250+
{ h2getUri, h2postUri, h2hangUri, HttpClient.Version.HTTP_2 }
251+
};
252+
}
253+
254+
@Test(dataProvider = "uris")
255+
public void test(URI getUri, URI postUri, URI hangUri, HttpClient.Version version) throws IOException, InterruptedException {
256+
HttpClient client = HttpClient.newBuilder()
257+
.version(version)
258+
.build();
259+
260+
HttpRequest getRequest = HttpRequest.newBuilder(getUri)
261+
.GET()
262+
.build();
263+
264+
HttpRequest postRequest = HttpRequest.newBuilder(postUri)
265+
.POST(HttpRequest.BodyPublishers.ofString("Sample Post"))
266+
.expectContinue(true)
267+
.build();
268+
269+
HttpRequest hangRequest = HttpRequest.newBuilder(hangUri)
270+
.POST(HttpRequest.BodyPublishers.ofString("Sample Post"))
271+
.expectContinue(true)
272+
.build();
273+
274+
CompletableFuture<HttpResponse<String>> cf = client.sendAsync(getRequest, HttpResponse.BodyHandlers.ofString());
275+
HttpResponse<String> resp = cf.join();
276+
System.err.println("Response Headers: " + resp.headers());
277+
System.err.println("Response Status Code: " + resp.statusCode());
278+
assertEquals(resp.statusCode(), 200);
279+
280+
cf = client.sendAsync(postRequest, HttpResponse.BodyHandlers.ofString());
281+
resp = cf.join();
282+
System.err.println("Response Headers: " + resp.headers());
283+
System.err.println("Response Status Code: " + resp.statusCode());
284+
assertEquals(resp.statusCode(), 200);
285+
286+
cf = client.sendAsync(hangRequest, HttpResponse.BodyHandlers.ofString());
287+
resp = cf.join();
288+
System.err.println("Response Headers: " + resp.headers());
289+
System.err.println("Response Status Code: " + resp.statusCode());
290+
assertEquals(resp.statusCode(), 417);
291+
}
292+
293+
}

0 commit comments

Comments
 (0)