8207966: HttpClient response without content-length does not return body

Reviewed-by: chegar

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

@@ -40,6 +40,7 @@ import java.util.function.Function;
 import java.net.http.HttpHeaders;
 import java.net.http.HttpResponse;
 import jdk.internal.net.http.ResponseContent.BodyParser;
+import jdk.internal.net.http.ResponseContent.UnknownLengthBodyParser;
 import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.Logger;
 import jdk.internal.net.http.common.MinimalFuture;
@@ -67,6 +68,7 @@ class Http1Response<T> {
     private final BodyReader bodyReader; // used to read the body
     private final Http1AsyncReceiver asyncReceiver;
     private volatile EOFException eof;
+    private volatile BodyParser bodyParser;
     // max number of bytes of (fixed length) body to ignore on redirect
     private final static int MAX_IGNORE = 1024;
 
@@ -230,6 +232,10 @@ class Http1Response<T> {
         return finished;
     }
 
+    /**
+     * Return known fixed content length or -1 if chunked, or -2 if no content-length
+     * information in which case, connection termination delimits the response body
+     */
     int fixupContentLen(int clen) {
         if (request.method().equalsIgnoreCase("HEAD") || responseCode == HTTP_NOT_MODIFIED) {
             return 0;
@@ -239,7 +245,11 @@ class Http1Response<T> {
                        .equalsIgnoreCase("chunked")) {
                 return -1;
             }
-            return 0;
+            if (responseCode == 101) {
+                // this is a h2c or websocket upgrade, contentlength must be zero
+                return 0;
+            }
+            return -2;
         }
         return clen;
     }
@@ -401,7 +411,7 @@ class Http1Response<T> {
                 // to prevent the SelectorManager thread from exiting until
                 // the body is fully read.
                 refCountTracker.acquire();
-                bodyReader.start(content.getBodyParser(
+                bodyParser = content.getBodyParser(
                     (t) -> {
                         try {
                             if (t != null) {
@@ -417,7 +427,8 @@ class Http1Response<T> {
                                 connection.close();
                             }
                         }
-                    }));
+                    });
+                bodyReader.start(bodyParser);
                 CompletableFuture<State> bodyReaderCF = bodyReader.completion();
                 asyncReceiver.subscribe(bodyReader);
                 assert bodyReaderCF != null : "parsing not started";
@@ -723,6 +734,11 @@ class Http1Response<T> {
 
         @Override
         public final void onReadError(Throwable t) {
+            if (t instanceof EOFException && bodyParser != null &&
+                    bodyParser instanceof UnknownLengthBodyParser) {
+                ((UnknownLengthBodyParser)bodyParser).complete();
+                return;
+            }
             t = wrapWithExtraDetail(t, parser::currentStateMessage);
             Http1Response.this.onReadError(t);
         }

src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java

@@ -75,6 +75,12 @@ class ResponseContent {
         if (chunkedContentInitialized) {
             return chunkedContent;
         }
+        if (contentLength == -2) {
+            // HTTP/1.0 content
+            chunkedContentInitialized = true;
+            chunkedContent = false;
+            return chunkedContent;
+        }
         if (contentLength == -1) {
             String tc = headers.firstValue("Transfer-Encoding")
                                .orElse("");
@@ -111,7 +117,9 @@ class ResponseContent {
         if (contentChunked()) {
             return new ChunkedBodyParser(onComplete);
         } else {
-            return new FixedLengthBodyParser(contentLength, onComplete);
+            return contentLength == -2
+                ? new UnknownLengthBodyParser(onComplete)
+                : new FixedLengthBodyParser(contentLength, onComplete);
         }
     }
 
@@ -392,6 +400,79 @@ class ResponseContent {
 
     }
 
+    class UnknownLengthBodyParser implements BodyParser {
+        final Consumer<Throwable> onComplete;
+        final Logger debug = Utils.getDebugLogger(this::dbgString, Utils.DEBUG);
+        final String dbgTag = ResponseContent.this.dbgTag + "/UnknownLengthBodyParser";
+        volatile Throwable closedExceptionally;
+        volatile AbstractSubscription sub;
+        volatile int breceived = 0;
+
+        UnknownLengthBodyParser(Consumer<Throwable> onComplete) {
+            this.onComplete = onComplete;
+        }
+
+        String dbgString() {
+            return dbgTag;
+        }
+
+        @Override
+        public void onSubscribe(AbstractSubscription sub) {
+            if (debug.on())
+                debug.log("onSubscribe: " + pusher.getClass().getName());
+            pusher.onSubscribe(this.sub = sub);
+        }
+
+        @Override
+        public String currentStateMessage() {
+            return format("http1_0 content, bytes received: %d", breceived);
+        }
+
+        @Override
+        public void accept(ByteBuffer b) {
+            if (closedExceptionally != null) {
+                if (debug.on())
+                    debug.log("already closed: " + closedExceptionally);
+                return;
+            }
+            boolean completed = false;
+            try {
+                if (debug.on())
+                    debug.log("Parser got %d bytes ", b.remaining());
+
+                if (b.hasRemaining()) {
+                    // only reduce demand if we actually push something.
+                    // we would not have come here if there was no
+                    // demand.
+                    boolean hasDemand = sub.demand().tryDecrement();
+                    assert hasDemand;
+                    breceived += b.remaining();
+                    pusher.onNext(List.of(b.asReadOnlyBuffer()));
+                }
+            } catch (Throwable t) {
+                if (debug.on()) debug.log("Unexpected exception", t);
+                closedExceptionally = t;
+                if (!completed) {
+                    onComplete.accept(t);
+                }
+            }
+        }
+
+        /**
+         * Must be called externally when connection has closed
+         * and therefore no more bytes can be read
+         */
+        public void complete() {
+            // We're done! All data has been received.
+            if (debug.on())
+                debug.log("Parser got all expected bytes: completing");
+            assert closedExceptionally == null;
+            onFinished.run();
+            pusher.onComplete();
+            onComplete.accept(closedExceptionally); // should be null
+        }
+    }
+
     class FixedLengthBodyParser implements BodyParser {
         final int contentLength;
         final Consumer<Throwable> onComplete;

test/jdk/java/net/httpclient/UnknownBodyLengthTest.java

@@ -0,0 +1,164 @@
+/*
+ * 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.
+ */
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.URI;
+import java.net.SocketTimeoutException;
+import java.time.Duration;
+import javax.net.ssl.SSLServerSocketFactory;
+import javax.net.ServerSocketFactory;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLParameters;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import jdk.testlibrary.SimpleSSLContext;
+
+/**
+ * @test
+ * @bug 8207966
+ * @library /lib/testlibrary
+ * @build jdk.testlibrary.SimpleSSLContext
+ * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true UnknownBodyLengthTest plain false
+ * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true UnknownBodyLengthTest SSL false
+ * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true UnknownBodyLengthTest plain true
+ * @run main/othervm -Djdk.tls.acknowledgeCloseNotify=true UnknownBodyLengthTest SSL true
+ */
+
+public class UnknownBodyLengthTest {
+    static final byte[] BUF = new byte[32 * 10234 + 2];
+
+    volatile SSLContext ctx;
+    volatile ServerSocketFactory factory;
+    volatile String clientURL;
+    volatile int port;
+    final ServerSocket ss;
+
+    UnknownBodyLengthTest(boolean useSSL) throws Exception {
+        ctx = new SimpleSSLContext().get();
+        SSLContext.setDefault(ctx);
+        factory = useSSL ? SSLServerSocketFactory.getDefault()
+                         : ServerSocketFactory.getDefault();
+        ss = factory.createServerSocket();
+        ss.setReuseAddress(true);
+        ss.bind(new InetSocketAddress("127.0.0.1", 0));
+        System.out.println("ServerSocket = " + ss.getClass() + " " + ss);
+        port = ss.getLocalPort();
+        clientURL = (useSSL ? "https" : "http") + "://localhost:"
+            + Integer.toString(port) + "/test";
+    }
+
+    static void fillBuf(byte[] buf) {
+        for (int i=0; i<buf.length; i++)
+            buf[i] = (byte)i;
+    }
+
+    static void checkBuf(byte[] buf) {
+        if (buf.length != BUF.length)
+            throw new RuntimeException("buffer lengths not the same");
+        for (int i=0; i<buf.length; i++)
+            if (buf[i] != BUF[i])
+                throw new RuntimeException("error at position " + i);
+    }
+
+    void server(final boolean withContentLength) {
+        fillBuf(BUF);
+        try {
+            Socket s = ss.accept();
+            s.setTcpNoDelay(true);
+            s.setSoLinger(true, 1);
+            System.out.println("Accepted: "+s.getRemoteSocketAddress());
+            System.out.println("Accepted: "+s);
+            OutputStream os = s.getOutputStream();
+            InputStream is = s.getInputStream();
+            boolean done = false;
+            byte[] buf = new byte[1024];
+            String rsp = "";
+            while (!done) {
+                int c = is.read(buf);
+                if (c < 0) break;
+                String s1 = new String(buf, 0, c, "ISO-8859-1");
+                rsp += s1;
+                done = rsp.endsWith("!#!#");
+            }
+            String r = "HTTP/1.0 200 OK\r\nConnection: close\r\nContent-Type:" +
+                       " text/xml; charset=UTF-8\r\n";
+            os.write(r.getBytes());
+            String chdr = "Content-Length: " + Integer.toString(BUF.length) +
+                     "\r\n";
+            System.out.println(chdr);
+            if(withContentLength)
+                os.write(chdr.getBytes());
+            os.write("\r\n".getBytes());
+            os.write(BUF);
+            if (is.available() > 0)
+                is.read(buf);
+            os.flush();
+            os.close();
+            s.shutdownOutput();
+            s.close();
+        } catch(final Throwable t) {
+          t.printStackTrace();
+        } finally {
+            try {ss.close(); } catch (Exception e) {}
+        }
+    }
+
+    void client(boolean useSSL) throws Exception {
+        SSLContext ctx = SSLContext.getDefault();
+        HttpClient.Builder clientB = HttpClient.newBuilder()
+            .version(HttpClient.Version.HTTP_2);
+        if (useSSL) {
+            clientB = clientB.sslContext(ctx)
+                .sslParameters(ctx.getSupportedSSLParameters());
+        }
+        final HttpClient client = clientB.build();
+
+        System.out.println("URL: " + clientURL);
+        final HttpResponse<byte[]> response = client
+            .send(HttpRequest
+                .newBuilder(new URI(clientURL))
+                .timeout(Duration.ofMillis(120_000))
+                .POST(HttpRequest.BodyPublishers.ofString("body!#!#"))
+                .build(), HttpResponse.BodyHandlers.ofByteArray());
+
+        System.out.println("Received reply: " + response.statusCode());
+        byte[] bb = response.body();
+        checkBuf(bb);
+    }
+
+    public static void main(final String[] args) throws Exception {
+        boolean ssl = args[0].equals("SSL");
+        boolean fixedlen = args[1].equals("true");
+        UnknownBodyLengthTest test = new UnknownBodyLengthTest(ssl);
+        test.run(ssl, fixedlen);
+    }
+
+    public void run(boolean ssl, boolean fixedlen) throws Exception {
+        new Thread(()->server(fixedlen)).start();
+        client(ssl);
+    }
+}