8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0
authormichaelm
Fri, 19 Oct 2018 14:23:43 +0100
changeset 52196 420445d16008
parent 52195 f08c1d7a5c53
child 52198 cb94f3a51aed
8211437: java.net.http.HttpClient hangs on 204 reply without Content-length 0 Reviewed-by: chegar, dfuchs
src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java
src/jdk.httpserver/share/classes/sun/net/httpserver/ExchangeImpl.java
test/jdk/java/net/httpclient/Response204.java
test/jdk/java/net/httpclient/http2/NoBodyTest.java
test/jdk/java/net/httpclient/http2/server/Http2TestExchangeImpl.java
--- 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));
         }