8226303: Examine the HttpRequest.BodyPublishers for exception handling
authorprappo
Tue, 18 Jun 2019 14:12:06 +0100
changeset 55426 4efe251009b4
parent 55309 8081b181bba8
child 55427 e0be41293b41
8226303: Examine the HttpRequest.BodyPublishers for exception handling Reviewed-by: chegar
src/java.net.http/share/classes/jdk/internal/net/http/PullPublisher.java
src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java
test/jdk/java/net/httpclient/RelayingPublishers.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/PullPublisher.java	Mon Jun 10 11:17:57 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/PullPublisher.java	Tue Jun 18 14:12:06 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -103,11 +103,19 @@
                 }
 
                 while (demand.tryDecrement() && !cancelled) {
-                    if (!iter.hasNext()) {
-                        break;
-                    } else {
-                        subscriber.onNext(iter.next());
+                    T next;
+                    try {
+                        if (!iter.hasNext()) {
+                            break;
+                        }
+                        next = iter.next();
+                    } catch (Throwable t1) {
+                        completed = true;
+                        pullScheduler.stop();
+                        subscriber.onError(t1);
+                        return;
                     }
+                    subscriber.onNext(next);
                 }
                 if (!iter.hasNext() && !cancelled) {
                     completed = true;
@@ -123,7 +131,7 @@
                 return;  // no-op
 
             if (n <= 0) {
-                error = new IllegalArgumentException("illegal non-positive request:" + n);
+                error = new IllegalArgumentException("non-positive subscription request: " + n);
             } else {
                 demand.increase(n);
             }
--- a/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java	Mon Jun 10 11:17:57 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java	Tue Jun 18 14:12:06 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -271,12 +271,13 @@
 
         @Override
         public void subscribe(Flow.Subscriber<? super ByteBuffer> subscriber) {
-            InputStream is;
+            InputStream is = null;
+            Throwable t = null;
             if (System.getSecurityManager() == null) {
                 try {
                     is = new FileInputStream(file);
                 } catch (IOException ioe) {
-                    throw new UncheckedIOException(ioe);
+                    t = ioe;
                 }
             } else {
                 try {
@@ -284,11 +285,16 @@
                             () -> new FileInputStream(file);
                     is = AccessController.doPrivileged(pa, null, filePermissions);
                 } catch (PrivilegedActionException pae) {
-                    throw new UncheckedIOException((IOException) pae.getCause());
+                    t = pae.getCause();
                 }
             }
-            PullPublisher<ByteBuffer> publisher =
-                    new PullPublisher<>(() -> new StreamIterator(is));
+            final InputStream fis = is;
+            PullPublisher<ByteBuffer> publisher;
+            if (t == null) {
+                publisher = new PullPublisher<>(() -> new StreamIterator(fis));
+            } else {
+                publisher = new PullPublisher<>(null, t);
+            }
             publisher.subscribe(subscriber);
         }
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/RelayingPublishers.java	Tue Jun 18 14:12:06 2019 +0100
@@ -0,0 +1,113 @@
+/*
+ * Copyright (c) 2019, 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 jdk.test.lib.util.FileUtils;
+import org.testng.annotations.Test;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.net.http.HttpRequest.BodyPublisher;
+import java.net.http.HttpRequest.BodyPublishers;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow;
+
+import static org.testng.Assert.assertEquals;
+
+/*
+ * @test
+ * @summary Verifies that some of the standard BodyPublishers relay exception
+ *          rather than throw it
+ * @bug 8226303
+ * @library /test/lib
+ * @run testng/othervm RelayingPublishers
+ */
+public class RelayingPublishers {
+
+    @Test
+    public void ofFile0() throws IOException {
+        Path directory = Files.createDirectory(Path.of("d"));
+        // Even though the path exists, the publisher should not be able
+        // to read from it, as that path denotes a directory, not a file
+        BodyPublisher pub = BodyPublishers.ofFile(directory);
+        CompletableSubscriber<ByteBuffer> s = new CompletableSubscriber<>();
+        pub.subscribe(s);
+        s.future().join();
+        // Interestingly enough, it's FileNotFoundException if a file
+        // is a directory
+        assertEquals(s.future().join().getClass(), FileNotFoundException.class);
+    }
+
+    @Test
+    public void ofFile1() throws IOException {
+        Path file = Files.createFile(Path.of("f"));
+        BodyPublisher pub = BodyPublishers.ofFile(file);
+        FileUtils.deleteFileWithRetry(file);
+        CompletableSubscriber<ByteBuffer> s = new CompletableSubscriber<>();
+        pub.subscribe(s);
+        assertEquals(s.future().join().getClass(), FileNotFoundException.class);
+    }
+
+    @Test
+    public void ofByteArrays() {
+        List<byte[]> bytes = new ArrayList<>();
+        bytes.add(null);
+        BodyPublisher pub = BodyPublishers.ofByteArrays(bytes);
+        CompletableSubscriber<ByteBuffer> s = new CompletableSubscriber<>();
+        pub.subscribe(s);
+        assertEquals(s.future().join().getClass(), NullPointerException.class);
+    }
+
+    static class CompletableSubscriber<T> implements Flow.Subscriber<T> {
+
+        final CompletableFuture<Throwable> f = new CompletableFuture<>();
+
+        @Override
+        public void onSubscribe(Flow.Subscription subscription) {
+            subscription.request(1);
+        }
+
+        @Override
+        public void onNext(T item) {
+            f.completeExceptionally(new RuntimeException("Unexpected onNext"));
+        }
+
+        @Override
+        public void onError(Throwable throwable) {
+            f.complete(throwable);
+        }
+
+        @Override
+        public void onComplete() {
+            f.completeExceptionally(new RuntimeException("Unexpected onNext"));
+        }
+
+        CompletableFuture<Throwable> future() {
+            return f.copy();
+        }
+    }
+}