http-client-branch: review comment: better handling for error conditions and tests to verify
--- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java Mon Jun 11 12:16:26 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java Mon Jun 11 13:07:18 2018 +0100
@@ -197,37 +197,6 @@
}
this.requestAction = new Http1Request(request, this);
this.asyncReceiver = new Http1AsyncReceiver(executor, this);
- asyncReceiver.subscribe(new InitialErrorReceiver());
- }
-
- /** An initial receiver that handles no data, but cancels the request if
- * it receives an error. Will be replaced when reading response body. */
- final class InitialErrorReceiver implements Http1AsyncReceiver.Http1AsyncDelegate {
- volatile AbstractSubscription s;
- @Override
- public boolean tryAsyncReceive(ByteBuffer ref) {
- return false; // no data has been processed, leave it in the queue
- }
-
- @Override
- public void onReadError(Throwable t) {
- if (!bodySentCF.isDone() && bodySubscriber != null)
- t = wrapWithExtraDetail(t, bodySubscriber::currentStateMessage);
- cancelImpl(t);
- }
-
- @Override
- public void onSubscribe(AbstractSubscription s) {
- this.s = s;
- }
-
- @Override
- public AbstractSubscription subscription() {
- return s;
- }
-
- @Override
- public void close(Throwable error) {}
}
@Override
@@ -517,7 +486,7 @@
private void requestMoreBody() {
try {
- if (debug.on()) debug.log("requesting more body from the subscriber");
+ if (debug.on()) debug.log("requesting more request body from the subscriber");
bodySubscriber.request(1);
} catch (Throwable t) {
if (debug.on()) debug.log("Subscription::request failed", t);
@@ -533,6 +502,18 @@
final Executor exec = client.theExecutor();
final DataPair dp = outgoing.pollFirst();
+ if (writePublisher.cancelled) {
+ if (debug.on()) debug.log("cancelling upstream publisher");
+ if (bodySubscriber != null) {
+ exec.execute(bodySubscriber::cancelSubscription);
+ } else if (debug.on()) {
+ debug.log("bodySubscriber is null");
+ }
+ headersSentCF.completeAsync(() -> this, exec);
+ bodySentCF.completeAsync(() -> this, exec);
+ return null;
+ }
+
if (dp == null) // publisher has not published anything yet
return null;
@@ -618,6 +599,14 @@
public void run() {
assert state != State.COMPLETED : "Unexpected state:" + state;
if (debug.on()) debug.log("WriteTask");
+
+ if (cancelled) {
+ if (debug.on()) debug.log("handling cancellation");
+ writeScheduler.stop();
+ getOutgoing();
+ return;
+ }
+
if (subscriber == null) {
if (debug.on()) debug.log("no subscriber yet");
return;
@@ -625,6 +614,8 @@
if (debug.on()) debug.log(() -> "hasOutgoing = " + hasOutgoing());
while (hasOutgoing() && demand.tryDecrement()) {
DataPair dp = getOutgoing();
+ if (dp == null)
+ break;
if (dp.throwable != null) {
if (debug.on()) debug.log("onError");
@@ -671,7 +662,7 @@
if (cancelled)
return; //no-op
cancelled = true;
- writeScheduler.stop();
+ writeScheduler.runOrSchedule(client.theExecutor());
}
}
}
--- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java Mon Jun 11 12:16:26 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java Mon Jun 11 13:07:18 2018 +0100
@@ -77,6 +77,7 @@
final Logger debug = Utils.getDebugLogger(this::dbgString, Utils.DEBUG);
final static AtomicLong responseCount = new AtomicLong();
final long id = responseCount.incrementAndGet();
+ private Http1HeaderParser hd;
Http1Response(HttpConnection conn,
Http1Exchange<T> exchange,
@@ -88,6 +89,11 @@
this.asyncReceiver = asyncReceiver;
headersReader = new HeadersReader(this::advance);
bodyReader = new BodyReader(this::advance);
+
+ hd = new Http1HeaderParser();
+ readProgress = State.READING_HEADERS;
+ headersReader.start(hd);
+ asyncReceiver.subscribe(headersReader);
}
String dbgTag;
@@ -151,17 +157,27 @@
}
}
+ private volatile boolean firstTimeAround = true;
+
public CompletableFuture<Response> readHeadersAsync(Executor executor) {
if (debug.on())
debug.log("Reading Headers: (remaining: "
+ asyncReceiver.remaining() +") " + readProgress);
- // with expect continue we will resume reading headers + body.
- asyncReceiver.unsubscribe(bodyReader);
- bodyReader.reset();
- Http1HeaderParser hd = new Http1HeaderParser();
- readProgress = State.READING_HEADERS;
- headersReader.start(hd);
- asyncReceiver.subscribe(headersReader);
+
+ if (firstTimeAround) {
+ firstTimeAround = false;
+ } else {
+ // with expect continue we will resume reading headers + body.
+ asyncReceiver.unsubscribe(bodyReader);
+ bodyReader.reset();
+
+ hd = new Http1HeaderParser();
+ readProgress = State.READING_HEADERS;
+ headersReader.reset();
+ headersReader.start(hd);
+ asyncReceiver.subscribe(headersReader);
+ }
+
CompletableFuture<State> cf = headersReader.completion();
assert cf != null : "parsing not started";
--- a/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java Mon Jun 11 12:16:26 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java Mon Jun 11 13:07:18 2018 +0100
@@ -455,6 +455,7 @@
@Override
public void cancel() {
+ if (debug.on()) debug.log("write: cancel");
dropSubscription();
upstreamSubscription.cancel();
}
@@ -774,6 +775,7 @@
+ " to subscriber " + subscriber);
current.errorRef.compareAndSet(null, error);
current.signalCompletion();
+ writeSubscriber.subscription.cancel();
readScheduler.stop();
debugState("leaving read() loop with error: ");
return;
--- a/src/java.net.http/share/classes/jdk/internal/net/http/common/SSLTube.java Mon Jun 11 12:16:26 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/SSLTube.java Mon Jun 11 13:07:18 2018 +0100
@@ -309,7 +309,7 @@
synchronized (this) {
previous = pendingDelegate.getAndSet(delegateWrapper);
subscription = readSubscription;
- handleNow = this.errorRef.get() != null || finished;
+ handleNow = this.errorRef.get() != null || finished || readSubscriber.onCompleteReceived;
}
if (previous != null) {
previous.dropSubscription();
@@ -424,12 +424,19 @@
// if onError is invoked concurrently with setDelegate.
synchronized (this) {
failed = this.errorRef.get();
- completed = finished;
+ completed = finished || onCompleteReceived;
subscribed = subscriberImpl;
}
+
if (failed != null) {
+ if (debug.on())
+ debug.log("onNewSubscription: subscriberImpl:%s, invoking onError:%s",
+ subscriberImpl, failed);
subscriberImpl.onError(failed);
} else if (completed) {
+ if (debug.on())
+ debug.log("onNewSubscription: subscriberImpl:%s, invoking onCompleted",
+ subscriberImpl);
subscriberImpl.onComplete();
}
}
@@ -528,12 +535,17 @@
final class SSLSubscriptionWrapper implements Flow.Subscription {
volatile Flow.Subscription delegate;
+ private volatile boolean cancelled;
void setSubscription(Flow.Subscription sub) {
long demand = writeDemand.get(); // FIXME: isn't it a racy way of passing the demand?
delegate = sub;
- if (debug.on()) debug.log("setSubscription: demand=%d", demand);
- if (demand > 0)
+ if (debug.on())
+ debug.log("setSubscription: demand=%d, cancelled:%s", demand, cancelled);
+
+ if (cancelled)
+ delegate.cancel();
+ else if (demand > 0)
sub.request(demand);
}
@@ -549,7 +561,9 @@
@Override
public void cancel() {
- // TODO: no-op or error?
+ cancelled = true;
+ if (delegate != null)
+ delegate.cancel();
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/ResponseBodyBeforeError.java Mon Jun 11 13:07:18 2018 +0100
@@ -0,0 +1,530 @@
+/*
+ * 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
+ * @summary Tests that all response body is delivered to the BodySubscriber
+ * before an abortive error terminates the flow
+ * @library /lib/testlibrary
+ * @build jdk.testlibrary.SimpleSSLContext
+ * @run testng/othervm ResponseBodyBeforeError
+ */
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.UncheckedIOException;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodySubscriber;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Flow;
+import jdk.testlibrary.SimpleSSLContext;
+import org.testng.annotations.AfterTest;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLServerSocketFactory;
+import static java.lang.System.out;
+import static java.net.http.HttpClient.Builder.NO_PROXY;
+import static java.net.http.HttpResponse.BodyHandlers.ofString;
+import static java.nio.charset.StandardCharsets.US_ASCII;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.fail;
+
+public class ResponseBodyBeforeError {
+
+ ReplyingServer variableLengthServer;
+ ReplyingServer variableLengthHttpsServer;
+ ReplyingServer fixedLengthServer;
+ ReplyingServer fixedLengthHttpsServer;
+
+ String httpURIVarLen;
+ String httpsURIVarLen;
+ String httpURIFixLen;
+ String httpsURIFixLen;
+
+ SSLContext sslContext;
+
+ static final String EXPECTED_RESPONSE_BODY =
+ "<html><body><h1>Heading</h1><p>Some Text</p></body></html>";
+
+ @DataProvider(name = "sanity")
+ public Object[][] sanity() {
+ return new Object[][]{
+ { httpURIVarLen + "?length=all" },
+ { httpsURIVarLen + "?length=all" },
+ { httpURIFixLen + "?length=all" },
+ { httpsURIFixLen + "?length=all" },
+ };
+ }
+
+ @Test(dataProvider = "sanity")
+ void sanity(String url) throws Exception {
+ HttpClient client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
+ HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
+ HttpResponse<String> response = client.send(request, ofString());
+ String body = response.body();
+ assertEquals(body, EXPECTED_RESPONSE_BODY);
+ client.sendAsync(request, ofString())
+ .thenApply(resp -> resp.body())
+ .thenAccept(b -> assertEquals(b, EXPECTED_RESPONSE_BODY))
+ .join();
+ }
+
+ @DataProvider(name = "uris")
+ public Object[][] variants() {
+ Object[][] cases = new Object[][] {
+ // The length query string is the total number of response body
+ // bytes in the reply, before the server closes the connection. The
+ // second arg is a partial-expected-body that the body subscriber
+ // should receive before onError is invoked.
+
+ { httpURIFixLen + "?length=0", "" },
+ { httpURIFixLen + "?length=1", "<" },
+ { httpURIFixLen + "?length=2", "<h" },
+ { httpURIFixLen + "?length=10", "<html><bod" },
+ { httpURIFixLen + "?length=19", "<html><body><h1>Hea" },
+ { httpURIFixLen + "?length=31", "<html><body><h1>Heading</h1><p>" },
+
+ { httpsURIFixLen + "?length=0", "" },
+ { httpsURIFixLen + "?length=1", "<" },
+ { httpsURIFixLen + "?length=2", "<h" },
+ { httpsURIFixLen + "?length=10", "<html><bod" },
+ { httpsURIFixLen + "?length=19", "<html><body><h1>Hea" },
+ { httpsURIFixLen + "?length=31", "<html><body><h1>Heading</h1><p>" },
+
+ // accounts for chunk framing
+ { httpURIVarLen + "?length=0", "" },
+ { httpURIVarLen + "?length=1", "" },
+ { httpURIVarLen + "?length=2", "" },
+ { httpURIVarLen + "?length=4", "<" },
+ { httpURIVarLen + "?length=5", "<h" },
+ { httpURIVarLen + "?length=18", "<html><bod" },
+ { httpURIVarLen + "?length=20", "<html><body>" },
+ { httpURIVarLen + "?length=21", "<html><body>" }, // boundary around chunk framing
+ { httpURIVarLen + "?length=22", "<html><body>" },
+ { httpURIVarLen + "?length=23", "<html><body>" },
+ { httpURIVarLen + "?length=24", "<html><body>" },
+ { httpURIVarLen + "?length=25", "<html><body>" },
+ { httpURIVarLen + "?length=26", "<html><body>" },
+ { httpURIVarLen + "?length=27", "<html><body><" },
+ { httpURIVarLen + "?length=51", "<html><body><h1>Heading</h1><p>" },
+
+ { httpsURIVarLen + "?length=0", "" },
+ { httpsURIVarLen + "?length=1", "" },
+ { httpsURIVarLen + "?length=2", "" },
+ { httpsURIVarLen + "?length=4", "<" },
+ { httpsURIVarLen + "?length=5", "<h" },
+ { httpsURIVarLen + "?length=18", "<html><bod" },
+ { httpsURIVarLen + "?length=20", "<html><body>" },
+ { httpsURIVarLen + "?length=21", "<html><body>" },
+ { httpsURIVarLen + "?length=22", "<html><body>" },
+ { httpsURIVarLen + "?length=23", "<html><body>" },
+ { httpsURIVarLen + "?length=24", "<html><body>" },
+ { httpsURIVarLen + "?length=25", "<html><body>" },
+ { httpsURIVarLen + "?length=26", "<html><body>" },
+ { httpsURIVarLen + "?length=27", "<html><body><" },
+ { httpsURIVarLen + "?length=51", "<html><body><h1>Heading</h1><p>" },
+ };
+
+ List<Object[]> list = new ArrayList<>();
+ Arrays.asList(cases).stream()
+ .map(e -> new Object[] {e[0], e[1], true}) // reuse client
+ .forEach(list::add);
+ Arrays.asList(cases).stream()
+ .map(e -> new Object[] {e[0], e[1], false}) // do not reuse client
+ .forEach(list::add);
+ return list.stream().toArray(Object[][]::new);
+ }
+
+ static final int ITERATION_COUNT = 3;
+
+ @Test(dataProvider = "uris")
+ void testSynchronousAllRequestBody(String url,
+ String expectedPatrialBody,
+ boolean sameClient)
+ throws Exception
+ {
+ out.print("---\n");
+ HttpClient client = null;
+ for (int i=0; i< ITERATION_COUNT; i++) {
+ if (!sameClient || client == null)
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
+ HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
+ CustomBodySubscriber bs = new CustomBodySubscriber();
+ try {
+ HttpResponse<String> response = client.send(request, r -> bs);
+ String body = response.body();
+ out.println(response + ": " + body);
+ fail("UNEXPECTED RESPONSE: " + response);
+ } catch (IOException expected) {
+ String pm = bs.receivedAsString();
+ out.println("partial body received: " + pm);
+ assertEquals(pm, expectedPatrialBody);
+ }
+ }
+ }
+
+ @Test(dataProvider = "uris")
+ void testAsynchronousAllRequestBody(String url,
+ String expectedPatrialBody,
+ boolean sameClient)
+ throws Exception
+ {
+ out.print("---\n");
+ HttpClient client = null;
+ for (int i=0; i< ITERATION_COUNT; i++) {
+ if (!sameClient || client == null)
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
+ HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
+ CustomBodySubscriber bs = new CustomBodySubscriber();
+ try {
+ HttpResponse<String> response = client.sendAsync(request, r -> bs).get();
+ String body = response.body();
+ out.println(response + ": " + body);
+ fail("UNEXPECTED RESPONSE: " + response);
+ } catch (ExecutionException ee) {
+ if (ee.getCause() instanceof IOException) {
+ String pm = bs.receivedAsString();
+ out.println("partial body received: " + pm);
+ assertEquals(pm, expectedPatrialBody);
+ } else {
+ throw ee;
+ }
+ }
+ }
+ }
+
+ static final class CustomBodySubscriber implements BodySubscriber<String> {
+
+ Flow.Subscription subscription;
+ private final List<ByteBuffer> received = new ArrayList<>();
+ private final CompletableFuture<String> cf = new CompletableFuture<>();
+
+ @Override
+ public CompletionStage<String> getBody() {
+ return cf;
+ }
+
+ @Override
+ public void onSubscribe(Flow.Subscription subscription) {
+ out.println("CustomBodySubscriber got onSubscribe: ");
+ this.subscription = subscription;
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(List<ByteBuffer> items) {
+ out.println("CustomBodySubscriber got onNext: " + items);
+ received.addAll(items);
+ subscription.request(1);
+ }
+
+ @Override
+ public void onError(Throwable expected) {
+ out.println("CustomBodySubscriber got expected: " + expected);
+ cf.completeExceptionally(expected);
+ }
+
+ String receivedAsString() {
+ int size = received.stream().mapToInt(ByteBuffer::remaining).sum();
+ byte[] res = new byte[size];
+ int from = 0;
+ for (ByteBuffer b : received) {
+ int l = b.remaining();
+ b.get(res, from, l);
+ from += l;
+ }
+ return new String(res, UTF_8);
+ }
+
+ @Override
+ public void onComplete() {
+ out.println("CustomBodySubscriber got complete: ");
+ assert false : "Unexpected onComplete";
+ }
+ }
+
+ // -- infra
+
+ /**
+ * A server that replies with headers and a, possibly partial, reply, before
+ * closing the connection. The number of body bytes of written, is
+ * controllable through the "length" query string param in the requested
+ * URI.
+ */
+ static abstract class ReplyingServer extends Thread implements Closeable {
+
+ private final String name;
+ private final ServerSocket ss;
+ private volatile boolean closed;
+
+ private ReplyingServer(String name) throws IOException {
+ super(name);
+ this.name = name;
+ ss = newServerSocket();
+ ss.bind(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0));
+ this.start();
+ }
+
+ protected ServerSocket newServerSocket() throws IOException {
+ return new ServerSocket();
+ }
+
+ abstract String responseHeaders();
+
+ abstract String responseBody();
+
+ @Override
+ public void run() {
+ while (!closed) {
+ try (Socket s = ss.accept()) {
+ out.print(name + ": got connection ");
+ InputStream is = s.getInputStream();
+ URI requestMethod = readRequestMethod(is);
+ out.print(requestMethod + " ");
+ URI uriPath = readRequestPath(is);
+ out.println(uriPath);
+ readRequestHeaders(is);
+
+ String query = uriPath.getRawQuery();
+ assert query != null;
+ String qv = query.split("=")[1];
+ int len;
+ if (qv.equals("all")) {
+ len = responseBody().getBytes(US_ASCII).length;
+ } else {
+ len = Integer.parseInt(query.split("=")[1]);
+ }
+
+ OutputStream os = s.getOutputStream();
+ os.write(responseHeaders().getBytes(US_ASCII));
+ out.println(name + ": headers written, writing " + len + " body bytes");
+ byte[] responseBytes = responseBody().getBytes(US_ASCII);
+ for (int i = 0; i< len; i++) {
+ os.write(responseBytes[i]);
+ os.flush();
+ }
+ } catch (IOException e) {
+ if (!closed)
+ throw new UncheckedIOException("Unexpected", e);
+ }
+ }
+ }
+
+ static final byte[] requestEnd = new byte[] { '\r', '\n', '\r', '\n' };
+
+ // Read the request method
+ static URI readRequestMethod(InputStream is) throws IOException {
+ StringBuilder sb = new StringBuilder();
+ int r;
+ while ((r = is.read()) != -1 && r != 0x20) {
+ sb.append((char)r);
+ }
+ return URI.create(sb.toString());
+ }
+
+ // Read the request URI path
+ static URI readRequestPath(InputStream is) throws IOException {
+ StringBuilder sb = new StringBuilder();
+ int r;
+ while ((r = is.read()) != -1 && r != 0x20) {
+ sb.append((char)r);
+ }
+ return URI.create(sb.toString());
+ }
+
+ // Read until the end of a HTTP request headers
+ static void readRequestHeaders(InputStream is) throws IOException {
+ int requestEndCount = 0, r;
+ while ((r = is.read()) != -1) {
+ if (r == requestEnd[requestEndCount]) {
+ requestEndCount++;
+ if (requestEndCount == 4) {
+ break;
+ }
+ } else {
+ requestEndCount = 0;
+ }
+ }
+ }
+
+ public int getPort() { return ss.getLocalPort(); }
+
+ @Override
+ public void close() {
+ if (closed)
+ return;
+ closed = true;
+ try {
+ ss.close();
+ } catch (IOException e) {
+ throw new UncheckedIOException("Unexpected", e);
+ }
+ }
+ }
+
+ /** A server that issues a possibly-partial chunked reply. */
+ static class PlainVariableLengthServer extends ReplyingServer {
+
+ static final String CHUNKED_RESPONSE_BODY =
+ "6\r\n"+ "<html>\r\n" +
+ "6\r\n"+ "<body>\r\n" +
+ "10\r\n"+ "<h1>Heading</h1>\r\n" +
+ "10\r\n"+ "<p>Some Text</p>\r\n" +
+ "7\r\n"+ "</body>\r\n" +
+ "7\r\n"+ "</html>\r\n" +
+ "0\r\n"+ "\r\n";
+
+ static final String RESPONSE_HEADERS =
+ "HTTP/1.1 200 OK\r\n" +
+ "Content-Type: text/html; charset=utf-8\r\n" +
+ "Transfer-Encoding: chunked\r\n" +
+ "Connection: close\r\n\r\n";
+
+
+ PlainVariableLengthServer() throws IOException {
+ super("PlainVariableLengthServer");
+ }
+
+ protected PlainVariableLengthServer(String name) throws IOException {
+ super(name);
+ }
+
+ @Override
+ String responseHeaders( ) { return RESPONSE_HEADERS; }
+
+ @Override
+ String responseBody( ) { return CHUNKED_RESPONSE_BODY; }
+ }
+
+ /** A server that issues a, possibly-partial, chunked reply over SSL */
+ static final class SSLVariableLengthServer extends PlainVariableLengthServer {
+ SSLVariableLengthServer() throws IOException {
+ super("SSLVariableLengthServer");
+ }
+ @Override
+ public ServerSocket newServerSocket() throws IOException {
+ return SSLServerSocketFactory.getDefault().createServerSocket();
+ }
+ }
+
+ /** A server that issues a, possibly-partial, fixed-length reply. */
+ static class PlainFixedLengthServer extends ReplyingServer {
+
+ static final String RESPONSE_BODY = EXPECTED_RESPONSE_BODY;
+
+ static final String RESPONSE_HEADERS =
+ "HTTP/1.1 200 OK\r\n" +
+ "Content-Type: text/html; charset=utf-8\r\n" +
+ "Content-Length: " + RESPONSE_BODY.length() + "\r\n" +
+ "Connection: close\r\n\r\n";
+
+ PlainFixedLengthServer() throws IOException {
+ super("PlainFixedLengthServer");
+ }
+
+ protected PlainFixedLengthServer(String name) throws IOException {
+ super(name);
+ }
+
+ @Override
+ String responseHeaders( ) { return RESPONSE_HEADERS; }
+
+ @Override
+ String responseBody( ) { return RESPONSE_BODY; }
+ }
+
+ /** A server that issues a, possibly-partial, fixed-length reply over SSL */
+ static final class SSLFixedLengthServer extends PlainFixedLengthServer {
+ SSLFixedLengthServer() throws IOException {
+ super("SSLFixedLengthServer");
+ }
+ @Override
+ public ServerSocket newServerSocket() throws IOException {
+ return SSLServerSocketFactory.getDefault().createServerSocket();
+ }
+ }
+
+ static String serverAuthority(ReplyingServer server) {
+ return InetAddress.getLoopbackAddress().getHostName() + ":"
+ + server.getPort();
+ }
+
+ @BeforeTest
+ public void setup() throws Exception {
+ sslContext = new SimpleSSLContext().get();
+ if (sslContext == null)
+ throw new AssertionError("Unexpected null sslContext");
+ SSLContext.setDefault(sslContext);
+
+ variableLengthServer = new PlainVariableLengthServer();
+ httpURIVarLen = "http://" + serverAuthority(variableLengthServer)
+ + "/http1/variable/foo";
+
+ variableLengthHttpsServer = new SSLVariableLengthServer();
+ httpsURIVarLen = "https://" + serverAuthority(variableLengthHttpsServer)
+ + "/https1/variable/bar";
+
+ fixedLengthServer = new PlainFixedLengthServer();
+ httpURIFixLen = "http://" + serverAuthority(fixedLengthServer)
+ + "/http1/fixed/baz";
+
+ fixedLengthHttpsServer = new SSLFixedLengthServer();
+ httpsURIFixLen = "https://" + serverAuthority(fixedLengthHttpsServer)
+ + "/https1/fixed/foz";
+ }
+
+ @AfterTest
+ public void teardown() throws Exception {
+ variableLengthServer.close();
+ variableLengthHttpsServer.close();
+ fixedLengthServer.close();
+ fixedLengthHttpsServer.close();
+ }
+}
--- a/test/jdk/java/net/httpclient/ShortResponseBody.java Mon Jun 11 12:16:26 2018 +0100
+++ b/test/jdk/java/net/httpclient/ShortResponseBody.java Mon Jun 11 13:07:18 2018 +0100
@@ -25,8 +25,15 @@
* @test
* @summary Tests Exception detail message when too few response bytes are
* received before a socket exception or eof.
- * @run testng/othervm ShortResponseBody
- * @run testng/othervm -Djdk.httpclient.enableAllMethodRetry ShortResponseBody
+ * @library /lib/testlibrary
+ * @build jdk.testlibrary.SimpleSSLContext
+ * @run testng/othervm
+ * -Djdk.internal.httpclient.debug=true
+ * ShortResponseBody
+ * @run testng/othervm
+ * -Djdk.internal.httpclient.debug=true
+ * -Djdk.httpclient.enableAllMethodRetry
+ * ShortResponseBody
*/
import java.io.IOException;
@@ -40,16 +47,21 @@
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
+import java.net.http.HttpRequest.BodyPublishers;
import java.net.http.HttpResponse;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.stream.Stream;
+import jdk.testlibrary.SimpleSSLContext;
import org.testng.annotations.AfterTest;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLServerSocketFactory;
+import javax.net.ssl.SSLSocket;
import static java.lang.System.out;
import static java.net.http.HttpClient.Builder.NO_PROXY;
import static java.net.http.HttpResponse.BodyHandlers.ofString;
@@ -62,27 +74,37 @@
public class ShortResponseBody {
Server closeImmediatelyServer;
+ Server closeImmediatelyHttpsServer;
Server variableLengthServer;
+ Server variableLengthHttpsServer;
Server fixedLengthServer;
String httpURIClsImed;
+ String httpsURIClsImed;
String httpURIVarLen;
+ String httpsURIVarLen;
String httpURIFixLen;
+ SSLContext sslContext;
+
static final String EXPECTED_RESPONSE_BODY =
"<html><body><h1>Heading</h1><p>Some Text</p></body></html>";
@DataProvider(name = "sanity")
public Object[][] sanity() {
return new Object[][]{
- { httpURIVarLen + "?length=all" },
- { httpURIFixLen + "?length=all" },
+ { httpURIVarLen + "?length=all" },
+ { httpsURIVarLen + "?length=all" },
+ { httpURIFixLen + "?length=all" },
};
}
@Test(dataProvider = "sanity")
void sanity(String url) throws Exception {
- HttpClient client = HttpClient.newBuilder().build();
+ HttpClient client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
HttpResponse<String> response = client.send(request, ofString());
String body = response.body();
@@ -99,22 +121,38 @@
// The length query string is the total number of bytes in the reply,
// including headers, before the server closes the connection. The
// second arg is a partial-expected-detail message in the exception.
- { httpURIVarLen + "?length=0", "no bytes" }, // EOF without receiving anything
- { httpURIVarLen + "?length=1", "status line" }, // EOF during status-line
- { httpURIVarLen + "?length=2", "status line" },
- { httpURIVarLen + "?length=10", "status line" },
- { httpURIVarLen + "?length=19", "header" }, // EOF during Content-Type header
- { httpURIVarLen + "?length=30", "header" },
- { httpURIVarLen + "?length=45", "header" },
- { httpURIVarLen + "?length=48", "header" },
- { httpURIVarLen + "?length=51", "header" },
- { httpURIVarLen + "?length=98", "header" }, // EOF during Connection header
- { httpURIVarLen + "?length=100", "header" },
- { httpURIVarLen + "?length=101", "header" },
- { httpURIVarLen + "?length=104", "header" },
+ { httpURIVarLen + "?length=0", "no bytes" }, // EOF without receiving anything
+ { httpURIVarLen + "?length=1", "status line" }, // EOF during status-line
+ { httpURIVarLen + "?length=2", "status line" },
+ { httpURIVarLen + "?length=10", "status line" },
+ { httpURIVarLen + "?length=19", "header" }, // EOF during Content-Type header
+ { httpURIVarLen + "?length=30", "header" },
+ { httpURIVarLen + "?length=45", "header" },
+ { httpURIVarLen + "?length=48", "header" },
+ { httpURIVarLen + "?length=51", "header" },
+ { httpURIVarLen + "?length=98", "header" }, // EOF during Connection header
+ { httpURIVarLen + "?length=100", "header" },
+ { httpURIVarLen + "?length=101", "header" },
+ { httpURIVarLen + "?length=104", "header" },
{ httpURIVarLen + "?length=106", "chunked transfer encoding" }, // EOF during chunk header ( length )
{ httpURIVarLen + "?length=110", "chunked transfer encoding" }, // EOF during chunk response body data
+ { httpsURIVarLen + "?length=0", "no bytes" },
+ { httpsURIVarLen + "?length=1", "status line" },
+ { httpsURIVarLen + "?length=2", "status line" },
+ { httpsURIVarLen + "?length=10", "status line" },
+ { httpsURIVarLen + "?length=19", "header" },
+ { httpsURIVarLen + "?length=30", "header" },
+ { httpsURIVarLen + "?length=45", "header" },
+ { httpsURIVarLen + "?length=48", "header" },
+ { httpsURIVarLen + "?length=51", "header" },
+ { httpsURIVarLen + "?length=98", "header" },
+ { httpsURIVarLen + "?length=100", "header" },
+ { httpsURIVarLen + "?length=101", "header" },
+ { httpsURIVarLen + "?length=104", "header" },
+ { httpsURIVarLen + "?length=106", "chunked transfer encoding" },
+ { httpsURIVarLen + "?length=110", "chunked transfer encoding" },
+
{ httpURIFixLen + "?length=0", "no bytes" }, // EOF without receiving anything
{ httpURIFixLen + "?length=1", "status line" }, // EOF during status-line
{ httpURIFixLen + "?length=2", "status line" },
@@ -131,7 +169,10 @@
{ httpURIFixLen + "?length=106", "fixed content-length" },
{ httpURIFixLen + "?length=110", "fixed content-length" },
+ // ## ADD https fixed
+
{ httpURIClsImed, "no bytes"},
+ { httpsURIClsImed, "no bytes"},
};
List<Object[]> list = new ArrayList<>();
@@ -154,7 +195,10 @@
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {
if (!sameClient || client == null)
- client = HttpClient.newBuilder().proxy(NO_PROXY).build();
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
try {
HttpResponse<String> response = client.send(request, ofString());
@@ -180,7 +224,10 @@
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {
if (!sameClient || client == null)
- client = HttpClient.newBuilder().proxy(NO_PROXY).build();
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
HttpRequest request = HttpRequest.newBuilder(URI.create(url)).build();
try {
HttpResponse<String> response = client.sendAsync(request, ofString()).get();
@@ -209,17 +256,28 @@
}
}
+ // POST tests are racy in what may be received before writing may cause a
+ // broken pipe or reset exception, before all the received data can be read.
+ // Any message up to, and including, the "expected" error message can occur.
+ // Strictly ordered list, in order of possible occurrence.
+ static final List<String> MSGS_ORDER =
+ List.of("no bytes", "status line", "header");
+
+
@Test(dataProvider = "uris")
- void testSynchronousPOST(String url, String unused, boolean sameClient)
+ void testSynchronousPOST(String url, String expectedMsg, boolean sameClient)
throws Exception
{
out.print("---\n");
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {
if (!sameClient || client == null)
- client = HttpClient.newBuilder().proxy(NO_PROXY).build();
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
- .POST(HttpRequest.BodyPublishers.ofInputStream(() -> new InfiniteInputStream()))
+ .POST(BodyPublishers.ofInputStream(() -> new InfiniteInputStream()))
.build();
try {
HttpResponse<String> response = client.send(request, ofString());
@@ -229,8 +287,14 @@
} catch (IOException ioe) {
out.println("Caught expected exception:" + ioe);
String msg = ioe.getMessage();
- // "incomplete" since the chunked request body is not completely sent
- assertTrue(msg.contains("incomplete"), "exception msg:[" + msg + "]");
+
+ List<String> expectedMessages = new ArrayList<>();
+ expectedMessages.add(expectedMsg);
+ MSGS_ORDER.stream().takeWhile(s -> !s.equals(expectedMsg))
+ .forEach(expectedMessages::add);
+
+ assertTrue(expectedMessages.stream().anyMatch(s -> msg.indexOf(s) != -1),
+ "exception msg:[" + msg + "], not in [" + expectedMessages);
// synchronous API must have the send method on the stack
assertSendMethodOnStack(ioe);
assertNoConnectionExpiredException(ioe);
@@ -239,16 +303,19 @@
}
@Test(dataProvider = "uris")
- void testAsynchronousPOST(String url, String unused, boolean sameClient)
+ void testAsynchronousPOST(String url, String expectedMsg, boolean sameClient)
throws Exception
{
out.print("---\n");
HttpClient client = null;
for (int i=0; i< ITERATION_COUNT; i++) {
if (!sameClient || client == null)
- client = HttpClient.newBuilder().proxy(NO_PROXY).build();
+ client = HttpClient.newBuilder()
+ .proxy(NO_PROXY)
+ .sslContext(sslContext)
+ .build();
HttpRequest request = HttpRequest.newBuilder(URI.create(url))
- .POST(HttpRequest.BodyPublishers.ofInputStream(() -> new InfiniteInputStream()))
+ .POST(BodyPublishers.ofInputStream(() -> new InfiniteInputStream()))
.build();
try {
HttpResponse<String> response = client.sendAsync(request, ofString()).get();
@@ -260,8 +327,14 @@
IOException ioe = (IOException) ee.getCause();
out.println("Caught expected exception:" + ioe);
String msg = ioe.getMessage();
- // "incomplete" since the chunked request body is not completely sent
- assertTrue(msg.contains("incomplete"), "exception msg:[" + msg + "]");
+
+ List<String> expectedMessages = new ArrayList<>();
+ expectedMessages.add(expectedMsg);
+ MSGS_ORDER.stream().takeWhile(s -> !s.equals(expectedMsg))
+ .forEach(expectedMessages::add);
+
+ assertTrue(expectedMessages.stream().anyMatch(s -> msg.indexOf(s) != -1),
+ "exception msg:[" + msg + "], not in [" + expectedMessages);
assertNoConnectionExpiredException(ioe);
} else {
throw ee;
@@ -309,11 +382,15 @@
Server(String name) throws IOException {
super(name);
- ss = new ServerSocket();
+ ss = newServerSocket();
ss.bind(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0));
this.start();
}
+ protected ServerSocket newServerSocket() throws IOException {
+ return new ServerSocket();
+ }
+
public int getPort() { return ss.getLocalPort(); }
@Override
@@ -332,15 +409,22 @@
/**
* A server that closes the connection immediately, without reading or writing.
*/
- static final class CloseImmediatelyServer extends Server {
- CloseImmediatelyServer() throws IOException {
- super("CloseImmediateServer");
+ static class PlainCloseImmediatelyServer extends Server {
+ PlainCloseImmediatelyServer() throws IOException {
+ super("PlainCloseImmediatelyServer");
+ }
+
+ protected PlainCloseImmediatelyServer(String name) throws IOException {
+ super(name);
}
@Override
public void run() {
while (!closed) {
try (Socket s = ss.accept()) {
+ if (s instanceof SSLSocket) {
+ ((SSLSocket)s).startHandshake();
+ }
out.println("Server: got connection, closing immediately ");
} catch (IOException e) {
if (!closed)
@@ -351,6 +435,20 @@
}
/**
+ * A server that closes the connection immediately, without reading or writing,
+ * after completing the SSL handshake.
+ */
+ static final class SSLCloseImmediatelyServer extends PlainCloseImmediatelyServer {
+ SSLCloseImmediatelyServer() throws IOException {
+ super("SSLCloseImmediatelyServer");
+ }
+ @Override
+ public ServerSocket newServerSocket() throws IOException {
+ return SSLServerSocketFactory.getDefault().createServerSocket();
+ }
+ }
+
+ /**
* A server that replies with headers and a, possibly partial, reply, before
* closing the connection. The number of bytes of written ( header + body),
* is controllable through the "length" query string param in the requested
@@ -390,10 +488,11 @@
}
OutputStream os = s.getOutputStream();
- out.println("Server: writing " + len + " bytes");
+ out.println(name + ": writing " + len + " bytes");
byte[] responseBytes = response().getBytes(US_ASCII);
for (int i = 0; i< len; i++) {
os.write(responseBytes[i]);
+ os.flush();
}
} catch (IOException e) {
if (!closed)
@@ -440,8 +539,8 @@
}
}
- /** A server that issues a chunked reply. */
- static final class VariableLengthServer extends ReplyingServer {
+ /** A server that issues a, possibly-partial, chunked reply. */
+ static class PlainVariableLengthServer extends ReplyingServer {
static final String CHUNKED_RESPONSE_BODY =
"6\r\n"+ "<html>\r\n" +
@@ -460,14 +559,29 @@
static final String RESPONSE = RESPONSE_HEADERS + CHUNKED_RESPONSE_BODY;
- VariableLengthServer() throws IOException {
- super("VariableLengthServer");
+ PlainVariableLengthServer() throws IOException {
+ super("PlainVariableLengthServer");
+ }
+
+ protected PlainVariableLengthServer(String name) throws IOException {
+ super(name);
}
@Override
String response( ) { return RESPONSE; }
}
+ /** A server that issues a, possibly-partial, chunked reply over SSL. */
+ static final class SSLVariableLengthServer extends PlainVariableLengthServer {
+ SSLVariableLengthServer() throws IOException {
+ super("SSLVariableLengthServer");
+ }
+ @Override
+ public ServerSocket newServerSocket() throws IOException {
+ return SSLServerSocketFactory.getDefault().createServerSocket();
+ }
+ }
+
/** A server that issues a fixed-length reply. */
static final class FixedLengthServer extends ReplyingServer {
@@ -496,14 +610,27 @@
@BeforeTest
public void setup() throws Exception {
- closeImmediatelyServer = new CloseImmediatelyServer();
+ sslContext = new SimpleSSLContext().get();
+ if (sslContext == null)
+ throw new AssertionError("Unexpected null sslContext");
+ SSLContext.setDefault(sslContext);
+
+ closeImmediatelyServer = new PlainCloseImmediatelyServer();
httpURIClsImed = "http://" + serverAuthority(closeImmediatelyServer)
+ "/http1/closeImmediately/foo";
- variableLengthServer = new VariableLengthServer();
+ closeImmediatelyHttpsServer = new SSLCloseImmediatelyServer();
+ httpsURIClsImed = "https://" + serverAuthority(closeImmediatelyHttpsServer)
+ + "/https1/closeImmediately/foo";
+
+ variableLengthServer = new PlainVariableLengthServer();
httpURIVarLen = "http://" + serverAuthority(variableLengthServer)
+ "/http1/variable/bar";
+ variableLengthHttpsServer = new SSLVariableLengthServer();
+ httpsURIVarLen = "https://" + serverAuthority(variableLengthHttpsServer)
+ + "/https1/variable/bar";
+
fixedLengthServer = new FixedLengthServer();
httpURIFixLen = "http://" + serverAuthority(fixedLengthServer)
+ "/http1/fixed/baz";
@@ -512,7 +639,9 @@
@AfterTest
public void teardown() throws Exception {
closeImmediatelyServer.close();
+ closeImmediatelyHttpsServer.close();
variableLengthServer.close();
+ variableLengthHttpsServer.close();
fixedLengthServer.close();
}
-}
\ No newline at end of file
+}