http-client-branch: review comment - add BodySubscribers::ofPublisher and BodyHandlers::ofPublisher methods http-client-branch
authordfuchs
Tue, 10 Apr 2018 10:25:34 +0100
branchhttp-client-branch
changeset 56405 3642d0ef7755
parent 56404 f14bec8e8d8e
child 56408 39c649170e9c
http-client-branch: review comment - add BodySubscribers::ofPublisher and BodyHandlers::ofPublisher methods
src/java.net.http/share/classes/java/net/http/HttpResponse.java
src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java
test/jdk/java/net/httpclient/InvalidSubscriptionRequest.java
test/jdk/java/net/httpclient/ResponsePublisher.java
--- a/src/java.net.http/share/classes/java/net/http/HttpResponse.java	Mon Apr 09 15:28:22 2018 +0100
+++ b/src/java.net.http/share/classes/java/net/http/HttpResponse.java	Tue Apr 10 10:25:34 2018 +0100
@@ -43,6 +43,8 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.Flow;
 import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscription;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -663,6 +665,28 @@
         }
 
         /**
+         * Returns a {@code BodyHandler<Publisher<List<ByteBuffer>>>} that creates a
+         * {@link BodySubscriber BodySubscriber}{@code <Publisher<List<ByteBuffer>>>}
+         * obtained from {@link BodySubscribers#ofPublisher()
+         * BodySubscribers.ofPublisher()}.
+         *
+         * <p> When the {@code HttpResponse} object is returned, the response
+         * headers will have been completely read, but the body may not have
+         * been fully received yet. The {@link #body()} method returns a
+         * {@link Publisher Publisher<List<ByteBuffer>>} from which the body
+         * response bytes can be obtained as they are received. The publisher
+         * can and must be subscribed to only once.
+         *
+         * @apiNote See {@link BodySubscribers#ofPublisher()} for more
+         * information.
+         *
+         * @return a response body handler
+         */
+        public static BodyHandler<Publisher<List<ByteBuffer>>> ofPublisher() {
+            return (status, headers) -> BodySubscribers.ofPublisher();
+        }
+
+        /**
          * Returns a {@code BodyHandler} which, when invoked, returns a {@linkplain
          * BodySubscribers#buffering(BodySubscriber,int) buffering BodySubscriber}
          * that buffers data before delivering it to the downstream subscriber.
@@ -1138,6 +1162,47 @@
         }
 
         /**
+         * Returns a response subscriber which publishes the response body
+         * through a {@link Publisher Publisher<List<ByteBuffer>>}.
+         *
+         * <p> The {@link HttpResponse} using this subscriber is available
+         * immediately after the response headers have been read, without
+         * requiring to wait for the entire body to be processed. The response
+         * body bytes can then be obtained by subscribing to the publisher
+         * returned by the {@code HttpResponse} {@link HttpResponse#body() body}
+         * method.
+         *
+         * <p>The publisher returned by the {@link HttpResponse#body() body}
+         * method can be subscribed to only once. The first subscriber will
+         * receive the body response bytes if successfully subscribed, or will
+         * cause the subscription to be cancelled otherwise.
+         * If more subscriptions are attempted, the subsequent subscribers will
+         * be immediately subscribed with an empty subscription and their
+         * {@link Subscriber#onError(Throwable) onError} method
+         * will be invoked with an {@code IllegalStateException}.
+         *
+         * @apiNote To ensure that all resources associated with the
+         * corresponding exchange are properly released the caller must
+         * ensure that the provided publisher is subscribed once, and either
+         * {@linkplain Subscription#request(long) requests} all bytes
+         * until {@link Subscriber#onComplete() onComplete} or
+         * {@link Subscriber#onError(Throwable) onError} are invoked, or
+         * cancel the provided {@linkplain Subscriber#onSubscribe(Subscription)
+         * subscription} if it is unable or unwilling to do so.
+         * Note that depending on the actual HTTP protocol {@linkplain
+         * HttpClient.Version version} used for the exchange, cancelling the
+         * subscription instead of exhausting the flow may cause the underlying
+         * HTTP connection to be closed and prevent it from being reused for
+         * subsequent operations.
+         *
+         * @return A {@code BodySubscriber} which publishes the response body
+         *         through a {@code Publisher<List<ByteBuffer>>}.
+         */
+        public static BodySubscriber<Publisher<List<ByteBuffer>>> ofPublisher() {
+            return ResponseSubscribers.createPublisher();
+        }
+
+        /**
          * Returns a response subscriber which discards the response body. The
          * supplied value is the value that will be returned from
          * {@link HttpResponse#body()}.
--- a/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java	Mon Apr 09 15:28:22 2018 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/ResponseSubscribers.java	Tue Apr 10 10:25:34 2018 +0100
@@ -31,6 +31,8 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.lang.System.Logger.Level;
+import java.net.http.HttpHeaders;
+import java.net.http.HttpResponse;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.charset.Charset;
@@ -52,10 +54,12 @@
 import java.util.concurrent.Flow.Subscriber;
 import java.util.concurrent.Flow.Subscription;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.stream.Stream;
 import java.net.http.HttpResponse.BodySubscriber;
+import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.MinimalFuture;
 import jdk.internal.net.http.common.Utils;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -677,4 +681,216 @@
             upstream.onComplete();
         }
     }
+
+    // A BodySubscriber that returns a Publisher<List<ByteBuffer>>
+    static class PublishingBodySubscriber
+            implements BodySubscriber<Flow.Publisher<List<ByteBuffer>>> {
+        private final MinimalFuture<Flow.Subscription>
+                subscriptionCF = new MinimalFuture<>();
+        private final MinimalFuture<SubscriberRef>
+                subscribedCF = new MinimalFuture<>();
+        private AtomicReference<SubscriberRef>
+                subscriberRef = new AtomicReference<>();
+        private final CompletionStage<Flow.Publisher<List<ByteBuffer>>> body =
+                subscriptionCF.thenCompose(
+                        (s) -> MinimalFuture.completedFuture(this::subscribe));
+
+        // We use the completionCF to ensure that only one of
+        // onError or onComplete is ever called.
+        private final MinimalFuture<Void> completionCF;
+        private PublishingBodySubscriber() {
+            completionCF = new MinimalFuture<>();
+            completionCF.whenComplete(
+                    (r,t) -> subscribedCF.thenAccept( s -> complete(s, t)));
+        }
+
+        // An object that holds a reference to a Flow.Subscriber.
+        // The reference is cleared when the subscriber is completed - either
+        // normally or exceptionally, or when the subscription is cancelled.
+        static final class SubscriberRef {
+            volatile Flow.Subscriber<? super List<ByteBuffer>> ref;
+            SubscriberRef(Flow.Subscriber<? super List<ByteBuffer>> subscriber) {
+                ref = subscriber;
+            }
+            Flow.Subscriber<? super List<ByteBuffer>> get() {
+                return ref;
+            }
+            Flow.Subscriber<? super List<ByteBuffer>> clear() {
+                Flow.Subscriber<? super List<ByteBuffer>> res = ref;
+                ref = null;
+                return res;
+            }
+        }
+
+        // A subscription that wraps an upstream subscription and
+        // holds a reference to a subscriber. The subscriber reference
+        // is cleared when the subscription is cancelled
+        final static class SubscriptionRef implements Flow.Subscription {
+            final Flow.Subscription subscription;
+            final SubscriberRef subscriberRef;
+            SubscriptionRef(Flow.Subscription subscription,
+                            SubscriberRef subscriberRef) {
+                this.subscription = subscription;
+                this.subscriberRef = subscriberRef;
+            }
+            @Override
+            public void request(long n) {
+                if (subscriberRef.get() != null) {
+                    subscription.request(n);
+                }
+            }
+            @Override
+            public void cancel() {
+                subscription.cancel();
+                subscriberRef.clear();
+            }
+
+            void subscribe() {
+                Subscriber<?> subscriber = subscriberRef.get();
+                if (subscriber != null) {
+                    subscriber.onSubscribe(this);
+                }
+            }
+
+            @Override
+            public String toString() {
+                return "SubscriptionRef/"
+                        + subscription.getClass().getName()
+                        + "@"
+                        + System.identityHashCode(subscription);
+            }
+        }
+
+        // This is a callback for the subscribedCF.
+        // Do not call directly!
+        private void complete(SubscriberRef ref, Throwable t) {
+            assert ref != null;
+            Subscriber<?> s = ref.clear();
+            // maybe null if subscription was cancelled
+            if (s == null) return;
+            if (t == null) {
+                try {
+                    s.onComplete();
+                } catch (Throwable x) {
+                    s.onError(x);
+                }
+            } else {
+                s.onError(t);
+            }
+        }
+
+        private void signalError(Throwable err) {
+            if (err == null) {
+                err = new NullPointerException("null throwable");
+            }
+            completionCF.completeExceptionally(err);
+        }
+
+        private void signalComplete() {
+            completionCF.complete(null);
+        }
+
+        private void subscribe(Flow.Subscriber<? super List<ByteBuffer>> subscriber) {
+            Objects.requireNonNull(subscriber, "subscriber must not be null");
+            SubscriberRef ref = new SubscriberRef(subscriber);
+            if (subscriberRef.compareAndSet(null, ref)) {
+                subscriptionCF.thenAccept((s) -> {
+                    SubscriptionRef subscription = new SubscriptionRef(s,ref);
+                    try {
+                        subscription.subscribe();
+                        subscribedCF.complete(ref);
+                    } catch (Throwable t) {
+                        if (Log.errors()) {
+                            Log.logError("Failed to call onSubscribe: " +
+                                    "cancelling subscription: " + t);
+                            Log.logError(t);
+                        }
+                        subscription.cancel();
+                    }
+                });
+            } else {
+                subscriber.onSubscribe(new Flow.Subscription() {
+                    @Override public void request(long n) { }
+                    @Override public void cancel() { }
+                });
+                subscriber.onError(new IllegalStateException(
+                        "This publisher has already one subscriber"));
+            }
+        }
+
+        @Override
+        public void onSubscribe(Flow.Subscription subscription) {
+            subscriptionCF.complete(subscription);
+        }
+
+        @Override
+        public void onNext(List<ByteBuffer> item) {
+            try {
+                // cannot be called before onSubscribe()
+                assert subscriptionCF.isDone();
+                SubscriberRef ref = subscriberRef.get();
+                // cannot be called before subscriber calls request(1)
+                assert ref != null;
+                Flow.Subscriber<? super List<ByteBuffer>>
+                        subscriber = ref.get();
+                if (subscriber != null) {
+                    // may be null if subscription was cancelled.
+                    subscriber.onNext(item);
+                }
+            } catch (Throwable err) {
+                signalError(err);
+                subscriptionCF.thenAccept(s -> s.cancel());
+            }
+        }
+
+        @Override
+        public void onError(Throwable throwable) {
+            // cannot be called before onSubscribe();
+            assert suppress(subscriptionCF.isDone(),
+                    "onError called before onSubscribe",
+                    throwable);
+            // onError can be called before request(1), and therefore can
+            // be called before subscriberRef is set.
+            signalError(throwable);
+        }
+
+        @Override
+        public void onComplete() {
+            // cannot be called before onSubscribe()
+            if (!subscriptionCF.isDone()) {
+                signalError(new InternalError(
+                        "onComplete called before onSubscribed"));
+            } else {
+                // onComplete can be called before request(1),
+                // and therefore can be called before subscriberRef
+                // is set.
+                signalComplete();
+            }
+        }
+
+        @Override
+        public CompletionStage<Flow.Publisher<List<ByteBuffer>>> getBody() {
+            return body;
+        }
+
+        private boolean suppress(boolean condition,
+                                 String assertion,
+                                 Throwable carrier) {
+            if (!condition) {
+                if (carrier != null) {
+                    carrier.addSuppressed(new AssertionError(assertion));
+                } else if (Log.errors()) {
+                    Log.logError(new AssertionError(assertion));
+                }
+            }
+            return true;
+        }
+
+    }
+
+    public static BodySubscriber<Flow.Publisher<List<ByteBuffer>>>
+    createPublisher() {
+        return new PublishingBodySubscriber();
+    }
+
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/InvalidSubscriptionRequest.java	Tue Apr 10 10:25:34 2018 +0100
@@ -0,0 +1,483 @@
+/*
+ * 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 an asynchronous BodySubscriber that completes
+ *          immediately with a Publisher<List<ByteBuffer>> whose
+ *          subscriber issues bad requests
+ * @library /lib/testlibrary http2/server
+ * @build jdk.testlibrary.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 InvalidSubscriptionRequest
+ */
+
+import com.sun.net.httpserver.HttpServer;
+import com.sun.net.httpserver.HttpsConfigurator;
+import com.sun.net.httpserver.HttpsServer;
+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 java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.net.http.HttpResponse.BodyHandler;
+import java.net.http.HttpResponse.BodyHandlers;
+import java.net.http.HttpResponse.BodySubscriber;
+import java.net.http.HttpResponse.BodySubscribers;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.CompletionStage;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Executor;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Flow;
+import java.util.concurrent.Flow.Publisher;
+import java.util.function.Supplier;
+
+import static java.lang.System.out;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.testng.Assert.assertEquals;
+
+public class InvalidSubscriptionRequest implements HttpServerAdapters {
+
+    SSLContext sslContext;
+    HttpTestServer httpTestServer;    // HTTP/1.1    [ 4 servers ]
+    HttpTestServer httpsTestServer;   // HTTPS/1.1
+    HttpTestServer http2TestServer;   // HTTP/2 ( h2c )
+    HttpTestServer https2TestServer;  // HTTP/2 ( h2  )
+    String httpURI_fixed;
+    String httpURI_chunk;
+    String httpsURI_fixed;
+    String httpsURI_chunk;
+    String http2URI_fixed;
+    String http2URI_chunk;
+    String https2URI_fixed;
+    String https2URI_chunk;
+
+    static final int ITERATION_COUNT = 3;
+    // a shared executor helps reduce the amount of threads created by the test
+    static final Executor executor = Executors.newCachedThreadPool();
+
+    interface BHS extends Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> {
+        static BHS of(BHS impl, String name) {
+            return new BHSImpl(impl, name);
+        }
+    }
+
+    static final class BHSImpl implements BHS {
+        final BHS supplier;
+        final String name;
+        BHSImpl(BHS impl, String name) {
+            this.supplier = impl;
+            this.name = name;
+        }
+        @Override
+        public String toString() {
+            return name;
+        }
+
+        @Override
+        public BodyHandler<Publisher<List<ByteBuffer>>> get() {
+            return supplier.get();
+        }
+    }
+
+    static final Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> OF_PUBLISHER_API =
+            BHS.of(BodyHandlers::ofPublisher, "BodyHandlers::ofPublisher");
+
+    @DataProvider(name = "variants")
+    public Object[][] variants() {
+        return new Object[][]{
+                { httpURI_fixed,    false, OF_PUBLISHER_API },
+                { httpURI_chunk,    false, OF_PUBLISHER_API },
+                { httpsURI_fixed,   false, OF_PUBLISHER_API },
+                { httpsURI_chunk,   false, OF_PUBLISHER_API },
+                { http2URI_fixed,   false, OF_PUBLISHER_API },
+                { http2URI_chunk,   false, OF_PUBLISHER_API },
+                { https2URI_fixed,  false, OF_PUBLISHER_API },
+                { https2URI_chunk,  false, OF_PUBLISHER_API },
+
+                { httpURI_fixed,    true, OF_PUBLISHER_API },
+                { httpURI_chunk,    true, OF_PUBLISHER_API },
+                { httpsURI_fixed,   true, OF_PUBLISHER_API },
+                { httpsURI_chunk,   true, OF_PUBLISHER_API },
+                { http2URI_fixed,   true, OF_PUBLISHER_API },
+                { http2URI_chunk,   true, OF_PUBLISHER_API },
+                { https2URI_fixed,  true, OF_PUBLISHER_API },
+                { https2URI_chunk,  true, OF_PUBLISHER_API },
+        };
+    }
+
+    HttpClient newHttpClient() {
+        return HttpClient.newBuilder()
+                         .executor(executor)
+                         .sslContext(sslContext)
+                         .build();
+    }
+
+    @Test(dataProvider = "variants")
+    public void testNoBody(String uri, boolean sameClient, BHS handlers) throws Exception {
+        HttpClient client = null;
+        for (int i=0; i< ITERATION_COUNT; i++) {
+            if (!sameClient || client == null)
+                client = newHttpClient();
+
+            HttpRequest req = HttpRequest.newBuilder(URI.create(uri))
+                    .build();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
+            HttpResponse<Publisher<List<ByteBuffer>>> response = client.send(req, handler);
+            // We can reuse our BodySubscribers implementations to subscribe to the
+            // Publisher<List<ByteBuffer>>
+            BodySubscriber<String> ofString = new BadBodySubscriber<>(BodySubscribers.ofString(UTF_8));
+            // get the Publisher<List<ByteBuffer>> and
+            // subscribe to it.
+            response.body().subscribe(ofString);
+            // Get the final result and compare it with the expected body
+            try {
+                String body = ofString.getBody().toCompletableFuture().get();
+                assertEquals(body, "");
+                if (uri.endsWith("/chunk")
+                        && response.version() == HttpClient.Version.HTTP_1_1) {
+                    // with /fixed and 0 length
+                    // there's no need for any call to request()
+                    throw new RuntimeException("Expected IAE not thrown");
+                }
+            } catch (Exception x) {
+                Throwable cause = x;
+                if (x instanceof CompletionException || x instanceof ExecutionException) {
+                    cause = x.getCause();
+                }
+                if (cause instanceof IllegalArgumentException) {
+                    System.out.println("Got expected exception: " + cause);
+                } else throw x;
+            }
+        }
+    }
+
+    @Test(dataProvider = "variants")
+    public void testNoBodyAsync(String uri, boolean sameClient, BHS handlers) throws Exception {
+        HttpClient client = null;
+        for (int i=0; i< ITERATION_COUNT; i++) {
+            if (!sameClient || client == null)
+                client = newHttpClient();
+
+            HttpRequest req = HttpRequest.newBuilder(URI.create(uri))
+                    .build();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
+            // We can reuse our BodySubscribers implementations to subscribe to the
+            // Publisher<List<ByteBuffer>>
+            BodySubscriber<String> ofString =
+                    new BadBodySubscriber<>(BodySubscribers.ofString(UTF_8));
+            CompletableFuture<HttpResponse<Publisher<List<ByteBuffer>>>> response =
+                    client.sendAsync(req, handler);
+            CompletableFuture<String> result = response.thenCompose(
+                            (responsePublisher) -> {
+                                // get the Publisher<List<ByteBuffer>> and
+                                // subscribe to it.
+                                responsePublisher.body().subscribe(ofString);
+                                return ofString.getBody();
+                            });
+            try {
+                // Get the final result and compare it with the expected body
+                assertEquals(result.get(), "");
+                if (uri.endsWith("/chunk")
+                        && response.get().version() == HttpClient.Version.HTTP_1_1) {
+                    // with /fixed and 0 length
+                    // there's no need for any call to request()
+                    throw new RuntimeException("Expected IAE not thrown");
+                }
+            } catch (Exception x) {
+                Throwable cause = x;
+                if (x instanceof CompletionException || x instanceof ExecutionException) {
+                    cause = x.getCause();
+                }
+                if (cause instanceof IllegalArgumentException) {
+                    System.out.println("Got expected exception: " + cause);
+                } else throw x;
+            }
+        }
+    }
+
+    @Test(dataProvider = "variants")
+    public void testAsString(String uri, boolean sameClient, BHS handlers) throws Exception {
+        HttpClient client = null;
+        for (int i=0; i< ITERATION_COUNT; i++) {
+            if (!sameClient || client == null)
+                client = newHttpClient();
+
+            HttpRequest req = HttpRequest.newBuilder(URI.create(uri+"/withBody"))
+                    .build();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
+            HttpResponse<Publisher<List<ByteBuffer>>> response = client.send(req, handler);
+            // We can reuse our BodySubscribers implementations to subscribe to the
+            // Publisher<List<ByteBuffer>>
+            BodySubscriber<String> ofString = new BadBodySubscriber<>(
+                    BodySubscribers.ofString(UTF_8));
+            // get the Publisher<List<ByteBuffer>> and
+            // subscribe to it.
+            response.body().subscribe(ofString);
+            // Get the final result and compare it with the expected body
+            try {
+                String body = ofString.getBody().toCompletableFuture().get();
+                assertEquals(body, WITH_BODY);
+                throw new RuntimeException("Expected IAE not thrown");
+            } catch (Exception x) {
+                Throwable cause = x;
+                if (x instanceof CompletionException || x instanceof ExecutionException) {
+                    cause = x.getCause();
+                }
+                if (cause instanceof IllegalArgumentException) {
+                    System.out.println("Got expected exception: " + cause);
+                } else throw x;
+            }
+        }
+    }
+
+    @Test(dataProvider = "variants")
+    public void testAsStringAsync(String uri, boolean sameClient, BHS handlers) throws Exception {
+        HttpClient client = null;
+        for (int i=0; i< ITERATION_COUNT; i++) {
+            if (!sameClient || client == null)
+                client = newHttpClient();
+
+            HttpRequest req = HttpRequest.newBuilder(URI.create(uri+"/withBody"))
+                    .build();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
+            // We can reuse our BodySubscribers implementations to subscribe to the
+            // Publisher<List<ByteBuffer>>
+            BodySubscriber<String> ofString =
+                    new BadBodySubscriber<>(BodySubscribers.ofString(UTF_8));
+            CompletableFuture<String> result = client.sendAsync(req, handler)
+                    .thenCompose((responsePublisher) -> {
+                        // get the Publisher<List<ByteBuffer>> and
+                        // subscribe to it.
+                        responsePublisher.body().subscribe(ofString);
+                        return ofString.getBody();
+                    });
+            // Get the final result and compare it with the expected body
+            try {
+                String body = result.get();
+                assertEquals(body, WITH_BODY);
+                throw new RuntimeException("Expected IAE not thrown");
+            } catch (Exception x) {
+                Throwable cause = x;
+                if (x instanceof CompletionException || x instanceof ExecutionException) {
+                    cause = x.getCause();
+                }
+                if (cause instanceof IllegalArgumentException) {
+                    System.out.println("Got expected exception: " + cause);
+                } else throw x;
+            }
+        }
+    }
+
+    static final class BadSubscription implements Flow.Subscription {
+        Flow.Subscription subscription;
+        Executor executor;
+        BadSubscription(Flow.Subscription subscription) {
+            this.subscription = subscription;
+        }
+
+        @Override
+        public void request(long n) {
+            if (executor == null) {
+                subscription.request(-n);
+            } else {
+                executor.execute(() -> subscription.request(-n));
+            }
+        }
+
+        @Override
+        public void cancel() {
+            subscription.cancel();
+        }
+    }
+
+    static final class BadBodySubscriber<T> implements BodySubscriber<T> {
+        final BodySubscriber<T> subscriber;
+        BadBodySubscriber(BodySubscriber<T> subscriber) {
+            this.subscriber = subscriber;
+        }
+
+        @Override
+        public CompletionStage<T> getBody() {
+            return subscriber.getBody();
+        }
+
+        @Override
+        public void onSubscribe(Flow.Subscription subscription) {
+            System.out.println("Subscription is: " + subscription);
+            subscriber.onSubscribe(new BadSubscription(subscription));
+        }
+
+        @Override
+        public void onNext(List<ByteBuffer> item) {
+            subscriber.onNext(item);
+        }
+
+        @Override
+        public void onError(Throwable throwable) {
+            subscriber.onError(throwable);
+        }
+
+        @Override
+        public void onComplete() {
+            subscriber.onComplete();
+        }
+    }
+
+    static String serverAuthority(HttpServer server) {
+        return InetAddress.getLoopbackAddress().getHostName() + ":"
+                + server.getAddress().getPort();
+    }
+
+    @BeforeTest
+    public void setup() throws Exception {
+        sslContext = new SimpleSSLContext().get();
+        if (sslContext == null)
+            throw new AssertionError("Unexpected null sslContext");
+
+        // HTTP/1.1
+        HttpTestHandler h1_fixedLengthHandler = new HTTP_FixedLengthHandler();
+        HttpTestHandler h1_chunkHandler = new HTTP_VariableLengthHandler();
+        InetSocketAddress sa = new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
+        httpTestServer = HttpTestServer.of(HttpServer.create(sa, 0));
+        httpTestServer.addHandler( h1_fixedLengthHandler, "/http1/fixed");
+        httpTestServer.addHandler(h1_chunkHandler,"/http1/chunk");
+        httpURI_fixed = "http://" + httpTestServer.serverAuthority() + "/http1/fixed";
+        httpURI_chunk = "http://" + httpTestServer.serverAuthority() + "/http1/chunk";
+
+        HttpsServer httpsServer = HttpsServer.create(sa, 0);
+        httpsServer.setHttpsConfigurator(new HttpsConfigurator(sslContext));
+        httpsTestServer = HttpTestServer.of(httpsServer);
+        httpsTestServer.addHandler(h1_fixedLengthHandler, "/https1/fixed");
+        httpsTestServer.addHandler(h1_chunkHandler, "/https1/chunk");
+        httpsURI_fixed = "https://" + httpsTestServer.serverAuthority() + "/https1/fixed";
+        httpsURI_chunk = "https://" + httpsTestServer.serverAuthority() + "/https1/chunk";
+
+        // HTTP/2
+        HttpTestHandler h2_fixedLengthHandler = new HTTP_FixedLengthHandler();
+        HttpTestHandler h2_chunkedHandler = new HTTP_VariableLengthHandler();
+
+        http2TestServer = HttpTestServer.of(new Http2TestServer("localhost", false, 0));
+        http2TestServer.addHandler(h2_fixedLengthHandler, "/http2/fixed");
+        http2TestServer.addHandler(h2_chunkedHandler, "/http2/chunk");
+        http2URI_fixed = "http://" + http2TestServer.serverAuthority() + "/http2/fixed";
+        http2URI_chunk = "http://" + http2TestServer.serverAuthority() + "/http2/chunk";
+
+        https2TestServer = HttpTestServer.of(new Http2TestServer("localhost", true, 0));
+        https2TestServer.addHandler(h2_fixedLengthHandler, "/https2/fixed");
+        https2TestServer.addHandler(h2_chunkedHandler, "/https2/chunk");
+        https2URI_fixed = "https://" + https2TestServer.serverAuthority() + "/https2/fixed";
+        https2URI_chunk = "https://" + https2TestServer.serverAuthority() + "/https2/chunk";
+
+        httpTestServer.start();
+        httpsTestServer.start();
+        http2TestServer.start();
+        https2TestServer.start();
+    }
+
+    @AfterTest
+    public void teardown() throws Exception {
+        httpTestServer.stop();
+        httpsTestServer.stop();
+        http2TestServer.stop();
+        https2TestServer.stop();
+    }
+
+    static final String WITH_BODY = "Lorem ipsum dolor sit amet, consectetur" +
+            " adipiscing elit, sed do eiusmod tempor incididunt ut labore et" +
+            " dolore magna aliqua. Ut enim ad minim veniam, quis nostrud" +
+            " exercitation ullamco laboris nisi ut aliquip ex ea" +
+            " commodo consequat. Duis aute irure dolor in reprehenderit in " +
+            "voluptate velit esse cillum dolore eu fugiat nulla pariatur." +
+            " Excepteur sint occaecat cupidatat non proident, sunt in culpa qui" +
+            " officia deserunt mollit anim id est laborum.";
+
+    static class HTTP_FixedLengthHandler implements HttpTestHandler {
+        @Override
+        public void handle(HttpTestExchange t) throws IOException {
+            out.println("HTTP_FixedLengthHandler received request to " + t.getRequestURI());
+            try (InputStream is = t.getRequestBody()) {
+                is.readAllBytes();
+            }
+            if (t.getRequestURI().getPath().endsWith("/withBody")) {
+                byte[] bytes = WITH_BODY.getBytes(UTF_8);
+                t.sendResponseHeaders(200, bytes.length);  // body
+                try (OutputStream os = t.getResponseBody()) {
+                    os.write(bytes);
+                }
+            } else {
+                t.sendResponseHeaders(200, 0);  //no body
+            }
+        }
+    }
+
+    static class HTTP_VariableLengthHandler implements HttpTestHandler {
+        @Override
+        public void handle(HttpTestExchange t) throws IOException {
+            out.println("HTTP_VariableLengthHandler received request to " + t.getRequestURI());
+            try (InputStream is = t.getRequestBody()) {
+                is.readAllBytes();
+            }
+            t.sendResponseHeaders(200, -1);  //chunked or variable
+            if (t.getRequestURI().getPath().endsWith("/withBody")) {
+                byte[] bytes = WITH_BODY.getBytes(UTF_8);
+                try (OutputStream os = t.getResponseBody()) {
+                    int chunkLen = bytes.length/10;
+                    if (chunkLen == 0) {
+                        os.write(bytes);
+                    } else {
+                        int count = 0;
+                        for (int i=0; i<10; i++) {
+                            os.write(bytes, count, chunkLen);
+                            os.flush();
+                            count += chunkLen;
+                        }
+                        os.write(bytes, count, bytes.length % chunkLen);
+                        count += bytes.length % chunkLen;
+                        assert count == bytes.length;
+                    }
+                }
+            } else {
+                t.getResponseBody().close();   // no body
+            }
+        }
+    }
+}
--- a/test/jdk/java/net/httpclient/ResponsePublisher.java	Mon Apr 09 15:28:22 2018 +0100
+++ b/test/jdk/java/net/httpclient/ResponsePublisher.java	Tue Apr 10 10:25:34 2018 +0100
@@ -57,6 +57,7 @@
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
 import java.net.http.HttpResponse.BodyHandler;
+import java.net.http.HttpResponse.BodyHandlers;
 import java.net.http.HttpResponse.BodySubscriber;
 import java.net.http.HttpResponse.BodySubscribers;
 import java.nio.ByteBuffer;
@@ -70,6 +71,7 @@
 import java.util.concurrent.Flow;
 import java.util.concurrent.Flow.Publisher;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 
 import static java.lang.System.out;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -93,30 +95,77 @@
     String https2URI_fixed;
     String https2URI_chunk;
 
-    static final int ITERATION_COUNT = 10;
+    static final int ITERATION_COUNT = 3;
     // a shared executor helps reduce the amount of threads created by the test
     static final Executor executor = Executors.newCachedThreadPool();
 
+    interface BHS extends Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> {
+        static BHS of(BHS impl, String name) {
+            return new BHSImpl(impl, name);
+        }
+    }
+
+    static final class BHSImpl implements BHS {
+        final BHS supplier;
+        final String name;
+        BHSImpl(BHS impl, String name) {
+            this.supplier = impl;
+            this.name = name;
+        }
+        @Override
+        public String toString() {
+            return name;
+        }
+
+        @Override
+        public BodyHandler<Publisher<List<ByteBuffer>>> get() {
+            return supplier.get();
+        }
+    }
+
+    static final Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> OF_PUBLISHER_API =
+            BHS.of(BodyHandlers::ofPublisher, "BodyHandlers::ofPublisher");
+    static final Supplier<BodyHandler<Publisher<List<ByteBuffer>>>> OF_PUBLISHER_TEST =
+            BHS.of(PublishingBodyHandler::new, "PublishingBodyHandler::new");
+
     @DataProvider(name = "variants")
     public Object[][] variants() {
         return new Object[][]{
-                { httpURI_fixed,    false },
-                { httpURI_chunk,    false },
-                { httpsURI_fixed,   false },
-                { httpsURI_chunk,   false },
-                { http2URI_fixed,   false },
-                { http2URI_chunk,   false },
-                { https2URI_fixed,  false,},
-                { https2URI_chunk,  false },
+                { httpURI_fixed,    false, OF_PUBLISHER_API },
+                { httpURI_chunk,    false, OF_PUBLISHER_API },
+                { httpsURI_fixed,   false, OF_PUBLISHER_API },
+                { httpsURI_chunk,   false, OF_PUBLISHER_API },
+                { http2URI_fixed,   false, OF_PUBLISHER_API },
+                { http2URI_chunk,   false, OF_PUBLISHER_API },
+                { https2URI_fixed,  false, OF_PUBLISHER_API },
+                { https2URI_chunk,  false, OF_PUBLISHER_API },
+
+                { httpURI_fixed,    true, OF_PUBLISHER_API },
+                { httpURI_chunk,    true, OF_PUBLISHER_API },
+                { httpsURI_fixed,   true, OF_PUBLISHER_API },
+                { httpsURI_chunk,   true, OF_PUBLISHER_API },
+                { http2URI_fixed,   true, OF_PUBLISHER_API },
+                { http2URI_chunk,   true, OF_PUBLISHER_API },
+                { https2URI_fixed,  true, OF_PUBLISHER_API },
+                { https2URI_chunk,  true, OF_PUBLISHER_API },
 
-                { httpURI_fixed,    true },
-                { httpURI_chunk,    true },
-                { httpsURI_fixed,   true },
-                { httpsURI_chunk,   true },
-                { http2URI_fixed,   true },
-                { http2URI_chunk,   true },
-                { https2URI_fixed,  true,},
-                { https2URI_chunk,  true },
+                { httpURI_fixed,    false, OF_PUBLISHER_TEST },
+                { httpURI_chunk,    false, OF_PUBLISHER_TEST },
+                { httpsURI_fixed,   false, OF_PUBLISHER_TEST },
+                { httpsURI_chunk,   false, OF_PUBLISHER_TEST },
+                { http2URI_fixed,   false, OF_PUBLISHER_TEST },
+                { http2URI_chunk,   false, OF_PUBLISHER_TEST },
+                { https2URI_fixed,  false, OF_PUBLISHER_TEST },
+                { https2URI_chunk,  false, OF_PUBLISHER_TEST },
+
+                { httpURI_fixed,    true, OF_PUBLISHER_TEST },
+                { httpURI_chunk,    true, OF_PUBLISHER_TEST },
+                { httpsURI_fixed,   true, OF_PUBLISHER_TEST },
+                { httpsURI_chunk,   true, OF_PUBLISHER_TEST },
+                { http2URI_fixed,   true, OF_PUBLISHER_TEST },
+                { http2URI_chunk,   true, OF_PUBLISHER_TEST },
+                { https2URI_fixed,  true, OF_PUBLISHER_TEST },
+                { https2URI_chunk,  true, OF_PUBLISHER_TEST },
         };
     }
 
@@ -128,7 +177,7 @@
     }
 
     @Test(dataProvider = "variants")
-    public void testExceptions(String uri, boolean sameClient) throws Exception {
+    public void testExceptions(String uri, boolean sameClient, BHS handlers) throws Exception {
         HttpClient client = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             if (!sameClient || client == null)
@@ -136,7 +185,7 @@
 
             HttpRequest req = HttpRequest.newBuilder(URI.create(uri))
                     .build();
-            BodyHandler<Publisher<List<ByteBuffer>>> handler = new PublishingBodyHandler();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
             HttpResponse<Publisher<List<ByteBuffer>>> response = client.send(req, handler);
             try {
                 response.body().subscribe(null);
@@ -169,7 +218,7 @@
     }
 
     @Test(dataProvider = "variants")
-    public void testNoBody(String uri, boolean sameClient) throws Exception {
+    public void testNoBody(String uri, boolean sameClient, BHS handlers) throws Exception {
         HttpClient client = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             if (!sameClient || client == null)
@@ -177,7 +226,7 @@
 
             HttpRequest req = HttpRequest.newBuilder(URI.create(uri))
                     .build();
-            BodyHandler<Publisher<List<ByteBuffer>>> handler = new PublishingBodyHandler();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
             HttpResponse<Publisher<List<ByteBuffer>>> response = client.send(req, handler);
             // We can reuse our BodySubscribers implementations to subscribe to the
             // Publisher<List<ByteBuffer>>
@@ -192,7 +241,7 @@
     }
 
     @Test(dataProvider = "variants")
-    public void testNoBodyAsync(String uri, boolean sameClient) throws Exception {
+    public void testNoBodyAsync(String uri, boolean sameClient, BHS handlers) throws Exception {
         HttpClient client = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             if (!sameClient || client == null)
@@ -200,7 +249,7 @@
 
             HttpRequest req = HttpRequest.newBuilder(URI.create(uri))
                     .build();
-            BodyHandler<Publisher<List<ByteBuffer>>> handler = new PublishingBodyHandler();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
             // We can reuse our BodySubscribers implementations to subscribe to the
             // Publisher<List<ByteBuffer>>
             BodySubscriber<String> ofString = BodySubscribers.ofString(UTF_8);
@@ -218,7 +267,7 @@
     }
 
     @Test(dataProvider = "variants")
-    public void testAsString(String uri, boolean sameClient) throws Exception {
+    public void testAsString(String uri, boolean sameClient, BHS handlers) throws Exception {
         HttpClient client = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             if (!sameClient || client == null)
@@ -226,7 +275,7 @@
 
             HttpRequest req = HttpRequest.newBuilder(URI.create(uri+"/withBody"))
                     .build();
-            BodyHandler<Publisher<List<ByteBuffer>>> handler = new PublishingBodyHandler();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
             HttpResponse<Publisher<List<ByteBuffer>>> response = client.send(req, handler);
             // We can reuse our BodySubscribers implementations to subscribe to the
             // Publisher<List<ByteBuffer>>
@@ -241,7 +290,7 @@
     }
 
     @Test(dataProvider = "variants")
-    public void testAsStringAsync(String uri, boolean sameClient) throws Exception {
+    public void testAsStringAsync(String uri, boolean sameClient, BHS handlers) throws Exception {
         HttpClient client = null;
         for (int i=0; i< ITERATION_COUNT; i++) {
             if (!sameClient || client == null)
@@ -249,7 +298,7 @@
 
             HttpRequest req = HttpRequest.newBuilder(URI.create(uri+"/withBody"))
                     .build();
-            BodyHandler<Publisher<List<ByteBuffer>>> handler = new PublishingBodyHandler();
+            BodyHandler<Publisher<List<ByteBuffer>>> handler = handlers.get();
             // We can reuse our BodySubscribers implementations to subscribe to the
             // Publisher<List<ByteBuffer>>
             BodySubscriber<String> ofString = BodySubscribers.ofString(UTF_8);