8207966: HttpClient response without content-length does not return body
Reviewed-by: chegar
--- 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<State> 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);
}
--- 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<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;
--- /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<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);
+ }
+}