8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0

Reviewed-by: chegar, dfuchs
This commit is contained in:
Michael McMahon 2018-10-19 14:23:43 +01:00
parent 4abb6861ec
commit b9279ef850
5 changed files with 396 additions and 4 deletions

View File

@ -26,21 +26,26 @@
package jdk.internal.net.http;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.net.ConnectException;
import java.net.http.HttpConnectTimeoutException;
import java.util.Iterator;
import java.util.LinkedList;
import java.security.AccessControlContext;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.Flow;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.net.http.HttpClient;
import java.net.http.HttpHeaders;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodySubscriber;
import java.net.http.HttpResponse.PushPromiseHandler;
import java.net.http.HttpTimeoutException;
import jdk.internal.net.http.common.Log;
@ -200,11 +205,60 @@ class MultiExchange<T> {
return cf;
}
// return true if the response is a type where a response body is never possible
// and therefore doesn't have to include header information which indicates no
// body is present. This is distinct from responses that also do not contain
// response bodies (possibly ever) but which are required to have content length
// info in the header (eg 205). Those cases do not have to be handled specially
private static boolean bodyNotPermitted(Response r) {
return r.statusCode == 204;
}
private boolean bodyIsPresent(Response r) {
HttpHeaders headers = r.headers();
if (headers.firstValue("Content-length").isPresent())
return true;
if (headers.firstValue("Transfer-encoding").isPresent())
return true;
return false;
}
// Call the user's body handler to get an empty body object
private CompletableFuture<HttpResponse<T>> handleNoBody(Response r, Exchange<T> exch) {
BodySubscriber<T> bs = responseHandler.apply(new ResponseInfoImpl(r.statusCode(),
r.headers(), r.version()));
CompletionStage<T> cs = bs.getBody();
bs.onSubscribe(new NullSubscription());
bs.onComplete();
MinimalFuture<HttpResponse<T>> result = new MinimalFuture<>();
cs.whenComplete((nullBody, exception) -> {
if (exception != null)
result.completeExceptionally(exception);
else {
this.response =
new HttpResponseImpl<>(r.request(), r, this.response, nullBody, exch);
result.complete(this.response);
}
});
return result;
}
private CompletableFuture<HttpResponse<T>>
responseAsync0(CompletableFuture<Void> start) {
return start.thenCompose( v -> responseAsyncImpl())
.thenCompose((Response r) -> {
Exchange<T> exch = getExchange();
if (bodyNotPermitted(r)) {
if (bodyIsPresent(r)) {
IOException ioe = new IOException(
"unexpected content length header with 204 response");
exch.cancel(ioe);
return MinimalFuture.failedFuture(ioe);
} else
return handleNoBody(r, exch);
}
return exch.readBodyAsync(responseHandler)
.thenApply((T body) -> {
this.response =
@ -214,6 +268,16 @@ class MultiExchange<T> {
});
}
static class NullSubscription implements Flow.Subscription {
@Override
public void request(long n) {
}
@Override
public void cancel() {
}
}
private CompletableFuture<Response> responseAsyncImpl() {
CompletableFuture<Response> cf;
if (attempts.incrementAndGet() > max_attempts) {

View File

@ -255,9 +255,7 @@ class ExchangeImpl {
noContentToSend = true;
contentLen = 0;
}
if (noContentLengthHeader) {
rspHdrs.remove("Content-length");
} else {
if (!noContentLengthHeader) {
rspHdrs.set("Content-length", Long.toString(contentLen));
}
o.setWrappedStream (new FixedLengthOutputStream (this, ros, contentLen));

View File

@ -0,0 +1,109 @@
/*
* Copyright (c) 2018, 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.
*/
/**
* @test
* @bug 8211437
* @run main/othervm -Djdk.httpclient.HttpClient.log=headers,requests Response204
* @summary
*/
import com.sun.net.httpserver.*;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.util.*;
import java.util.concurrent.*;
import java.util.logging.*;
import java.io.*;
import java.net.*;
/**
* Verify that a 204 response code with no content-length is handled correctly
*/
public class Response204 {
public static void main (String[] args) throws Exception {
Logger logger = Logger.getLogger ("com.sun.net.httpserver");
ConsoleHandler c = new ConsoleHandler();
c.setLevel (Level.WARNING);
logger.addHandler (c);
logger.setLevel (Level.WARNING);
Handler handler = new Handler();
InetSocketAddress addr = new InetSocketAddress (0);
HttpServer server = HttpServer.create (addr, 0);
HttpContext ctx = server.createContext ("/test", handler);
ExecutorService executor = Executors.newCachedThreadPool();
server.setExecutor (executor);
server.start ();
URI uri = new URI("http://localhost:"+server.getAddress().getPort()+"/test/foo.html");
try {
HttpClient client = HttpClient.newHttpClient();
HttpRequest request = HttpRequest.newBuilder(uri)
.GET()
.build();
HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
if (response.statusCode() != 204)
throw new RuntimeException("wrong response code");
if (response.body() != null && !response.body().equals(""))
throw new RuntimeException("should have received empty response");
System.out.println(response.headers().firstValue("content-length").orElse("nichts"));
System.out.println ("OK 1");
// Send a second time. This time we should get exception because the server
// is going to send an invalid 204 with a Content-length
try {
response = client.send(request, HttpResponse.BodyHandlers.ofString());
throw new RuntimeException("send should have thrown exception");
} catch (IOException ioe) {
System.out.println("OK 2");
}
} finally {
server.stop(2);
executor.shutdown();
}
}
public static boolean error = false;
static class Handler implements HttpHandler {
volatile int counter = 0;
public void handle(HttpExchange t)
throws IOException {
InputStream is = t.getRequestBody();
Headers map = t.getRequestHeaders();
Headers rmap = t.getResponseHeaders();
while (is.read() != -1) ;
is.close();
if (counter++ == 1) {
// pretend there is a body
rmap.set("Content-length", "10");
}
t.sendResponseHeaders(204, -1);
t.close();
}
}
}

View File

@ -0,0 +1,221 @@
/*
* Copyright (c) 2015, 2018, 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.
*/
/*
* @test
* @bug 8087112
* @library /test/lib server
* @build jdk.test.lib.net.SimpleSSLContext
* @modules java.base/sun.net.www.http
* java.net.http/jdk.internal.net.http.common
* java.net.http/jdk.internal.net.http.frame
* java.net.http/jdk.internal.net.http.hpack
* @run testng/othervm -Djdk.httpclient.HttpClient.log=ssl,requests,responses,errors NoBodyTest
*/
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.*;
import javax.net.ssl.*;
import java.net.http.HttpClient;
import java.net.http.HttpHeaders;
import java.net.http.HttpRequest;
import java.net.http.HttpRequest.BodyPublishers;
import java.net.http.HttpResponse;
import java.net.http.HttpResponse.BodyHandlers;
import java.util.concurrent.*;
import jdk.test.lib.net.SimpleSSLContext;
import org.testng.annotations.Test;
import static java.net.http.HttpClient.Version.HTTP_2;
@Test
public class NoBodyTest {
static int httpPort, httpsPort;
static Http2TestServer httpServer, httpsServer;
static HttpClient client = null;
static ExecutorService clientExec;
static ExecutorService serverExec;
static SSLContext sslContext;
static String TEST_STRING = "The quick brown fox jumps over the lazy dog ";
static String httpURIString, httpsURIString;
static void initialize() throws Exception {
try {
SimpleSSLContext sslct = new SimpleSSLContext();
sslContext = sslct.get();
client = getClient();
httpServer = new Http2TestServer(false, 0, serverExec, sslContext);
httpServer.addHandler(new Handler(), "/");
httpPort = httpServer.getAddress().getPort();
httpsServer = new Http2TestServer(true, 0, serverExec, sslContext);
httpsServer.addHandler(new Handler(), "/");
httpsPort = httpsServer.getAddress().getPort();
httpURIString = "http://localhost:" + httpPort + "/foo/";
httpsURIString = "https://localhost:" + httpsPort + "/bar/";
httpServer.start();
httpsServer.start();
} catch (Throwable e) {
System.err.println("Throwing now");
e.printStackTrace(System.err);
throw e;
}
}
@Test
public static void runtest() throws Exception {
try {
initialize();
warmup(false);
warmup(true);
test(false);
test(true);
} catch (Throwable tt) {
System.err.println("tt caught");
tt.printStackTrace(System.err);
throw tt;
} finally {
httpServer.stop();
httpsServer.stop();
}
}
static HttpClient getClient() {
if (client == null) {
serverExec = Executors.newCachedThreadPool();
clientExec = Executors.newCachedThreadPool();
client = HttpClient.newBuilder()
.executor(clientExec)
.sslContext(sslContext)
.version(HTTP_2)
.build();
}
return client;
}
static URI getURI(boolean secure) {
if (secure)
return URI.create(httpsURIString);
else
return URI.create(httpURIString);
}
static void checkStatus(int expected, int found) throws Exception {
if (expected != found) {
System.err.printf ("Test failed: wrong status code %d/%d\n",
expected, found);
throw new RuntimeException("Test failed");
}
}
static void checkStrings(String expected, String found) throws Exception {
if (!expected.equals(found)) {
System.err.printf ("Test failed: wrong string %s/%s\n",
expected, found);
throw new RuntimeException("Test failed");
}
}
static final int LOOPS = 13;
static void warmup(boolean secure) throws Exception {
URI uri = getURI(secure);
String type = secure ? "https" : "http";
System.err.println("Request to " + uri);
// Do a simple warmup request
HttpClient client = getClient();
HttpRequest req = HttpRequest.newBuilder(uri)
.POST(BodyPublishers.ofString("Random text"))
.build();
HttpResponse<String> response = client.send(req, BodyHandlers.ofString());
checkStatus(200, response.statusCode());
String responseBody = response.body();
HttpHeaders h = response.headers();
checkStrings(TEST_STRING + type, responseBody);
}
static void test(boolean secure) throws Exception {
URI uri = getURI(secure);
String type = secure ? "https" : "http";
System.err.println("Request to " + uri);
HttpRequest request = HttpRequest.newBuilder(uri)
.POST(BodyPublishers.ofString(TEST_STRING))
.build();
for (int i = 0; i < LOOPS; i++) {
System.out.println("Loop " + i);
HttpResponse<String> response = client.send(request, BodyHandlers.ofString());
int expectedResponse = (i % 2) == 0 ? 204 : 200;
if (response.statusCode() != expectedResponse)
throw new RuntimeException("wrong response code " + Integer.toString(response.statusCode()));
if (expectedResponse == 200 && !response.body().equals(TEST_STRING + type)) {
System.err.printf("response received/expected %s/%s\n", response.body(), TEST_STRING + type);
throw new RuntimeException("wrong response body");
}
}
System.err.println("test: DONE");
}
static class Handler implements Http2Handler {
public Handler() {}
volatile int invocation = 0;
@Override
public void handle(Http2TestExchange t)
throws IOException {
try {
URI uri = t.getRequestURI();
System.err.printf("Handler received request to %s from %s\n",
uri, t.getRemoteAddress());
String type = uri.getScheme().toLowerCase();
InputStream is = t.getRequestBody();
while (is.read() != -1);
is.close();
// every second response is 204.
if ((invocation++ % 2) == 1) {
System.err.println("Server sending 204");
t.sendResponseHeaders(204, -1);
} else {
String body = TEST_STRING + type;
t.sendResponseHeaders(200, body.length());
OutputStream os = t.getResponseBody();
os.write(body.getBytes());
os.close();
}
} catch (Throwable e) {
e.printStackTrace(System.err);
throw new IOException(e);
}
}
}
}

View File

@ -129,7 +129,7 @@ public class Http2TestExchangeImpl implements Http2TestExchange {
@Override
public void sendResponseHeaders(int rCode, long responseLength) throws IOException {
this.responseLength = responseLength;
if (responseLength > 0 || responseLength < 0) {
if (responseLength !=0 && rCode != 204) {
long clen = responseLength > 0 ? responseLength : 0;
rspheadersBuilder.setHeader("Content-length", Long.toString(clen));
}