# HG changeset patch # User michaelm # Date 1534411368 -3600 # Node ID 9d7d74c6f2cbe522e39fa22dc557fdd3f79b32ad # Parent 58d7aa066071d05274d0a3e0b6e204c59028478b 8207966: HttpClient response without content-length does not return body Reviewed-by: chegar diff -r 58d7aa066071 -r 9d7d74c6f2cb src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java Thu Aug 16 02:00:31 2018 +0800 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java Thu Aug 16 10:22:48 2018 +0100 @@ -40,6 +40,7 @@ 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 @@ 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 @@ 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 @@ .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 @@ // 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 @@ connection.close(); } } - })); + }); + bodyReader.start(bodyParser); CompletableFuture bodyReaderCF = bodyReader.completion(); asyncReceiver.subscribe(bodyReader); assert bodyReaderCF != null : "parsing not started"; @@ -723,6 +734,11 @@ @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); } diff -r 58d7aa066071 -r 9d7d74c6f2cb src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java --- a/src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java Thu Aug 16 02:00:31 2018 +0800 +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ResponseContent.java Thu Aug 16 10:22:48 2018 +0100 @@ -75,6 +75,12 @@ 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 @@ 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 UnknownLengthBodyParser implements BodyParser { + final Consumer 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 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 onComplete; diff -r 58d7aa066071 -r 9d7d74c6f2cb test/jdk/java/net/httpclient/UnknownBodyLengthTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/net/httpclient/UnknownBodyLengthTest.java Thu Aug 16 10:22:48 2018 +0100 @@ -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 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 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); + } +}