8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0
Reviewed-by: chegar, dfuchs
--- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java Thu Oct 18 23:05:01 2018 -0700
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java Fri Oct 19 14:23:43 2018 +0100
@@ -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 @@
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 @@
});
}
+ 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) {
--- a/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java Thu Oct 18 23:05:01 2018 -0700
+++ b/src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java Fri Oct 19 14:23:43 2018 +0100
@@ -255,9 +255,7 @@
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));
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/Response204.java Fri Oct 19 14:23:43 2018 +0100
@@ -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();
+ }
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/http2/NoBodyTest.java Fri Oct 19 14:23:43 2018 +0100
@@ -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);
+ }
+ }
+ }
+}
--- a/test/jdk/java/net/httpclient/http2/server/Http2TestExchangeImpl.java Thu Oct 18 23:05:01 2018 -0700
+++ b/test/jdk/java/net/httpclient/http2/server/Http2TestExchangeImpl.java Fri Oct 19 14:23:43 2018 +0100
@@ -129,7 +129,7 @@
@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));
}