8193698: Null handling in BodyPublisher, BodyHandler, and BodySubscriber convenience static factory methods
authorchegar
Mon, 18 Dec 2017 10:21:38 +0000
changeset 48379 5382baab8371
parent 48378 e8e8db4f8194
child 48380 597f69e5f1e3
8193698: Null handling in BodyPublisher, BodyHandler, and BodySubscriber convenience static factory methods Reviewed-by: dfuchs
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java
test/jdk/java/net/httpclient/RequestProcessorExceptions.java
test/jdk/java/net/httpclient/SubscriberPublisherAPIExceptions.java
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java	Tue Dec 19 08:51:11 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java	Mon Dec 18 10:21:38 2017 +0000
@@ -80,7 +80,7 @@
 
     BufferingSubscriber(HttpResponse.BodySubscriber<T> downstreamSubscriber,
                         int bufferSize) {
-        this.downstreamSubscriber = downstreamSubscriber;
+        this.downstreamSubscriber = Objects.requireNonNull(downstreamSubscriber);
         this.bufferSize = bufferSize;
         synchronized (buffersLock) {
             internalBuffers = new ArrayList<>();
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java	Tue Dec 19 08:51:11 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java	Mon Dec 18 10:21:38 2017 +0000
@@ -344,24 +344,14 @@
          * Returns a {@code BodyHandler<String>} that returns a
          * {@link BodySubscriber BodySubscriber}{@code <String>} obtained from
          * {@link BodySubscriber#asString(Charset) BodySubscriber.asString(Charset)}.
-         * If a charset is provided, the body is decoded using it. If charset is
-         * {@code null} then the handler tries to determine the character set
-         * from the {@code Content-encoding} header. If that charset is not
-         * supported then {@link java.nio.charset.StandardCharsets#UTF_8 UTF_8}
-         * is used.
+         * The body is decoded using the given character set.
          *
-         * @param charset The name of the charset to interpret the body as. If
-         *                {@code null} then the charset is determined from the
-         *                <i>Content-encoding</i> header.
+         * @param charset the character set to convert the body with
          * @return a response body handler
          */
         public static BodyHandler<String> asString(Charset charset) {
-            return (status, headers) -> {
-                if (charset != null) {
-                    return BodySubscriber.asString(charset);
-                }
-                return BodySubscriber.asString(charsetFrom(headers));
-            };
+            Objects.requireNonNull(charset);
+            return (status, headers) -> BodySubscriber.asString(charset);
         }
 
         /**
@@ -386,11 +376,11 @@
          */
         public static BodyHandler<Path> asFile(Path file, OpenOption... openOptions) {
             Objects.requireNonNull(file);
+            List<OpenOption> opts = List.of(openOptions);
             SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
                 String fn = pathForSecurityCheck(file);
                 sm.checkWrite(fn);
-                List<OpenOption> opts = Arrays.asList(openOptions);
                 if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE))
                     sm.checkDelete(fn);
                 if (opts.contains(StandardOpenOption.READ))
@@ -450,11 +440,11 @@
         public static BodyHandler<Path> asFileDownload(Path directory,
                                                        OpenOption... openOptions) {
             Objects.requireNonNull(directory);
+            List<OpenOption> opts = List.of(openOptions);
             SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
                 String fn = pathForSecurityCheck(directory);
                 sm.checkWrite(fn);
-                List<OpenOption> opts = Arrays.asList(openOptions);
                 if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE))
                     sm.checkDelete(fn);
                 if (opts.contains(StandardOpenOption.READ))
@@ -494,6 +484,7 @@
          * @return a response body handler
          */
         public static BodyHandler<Void> asByteArrayConsumer(Consumer<Optional<byte[]>> consumer) {
+            Objects.requireNonNull(consumer);
             return (status, headers) -> BodySubscriber.asByteArrayConsumer(consumer);
         }
 
@@ -547,6 +538,7 @@
          */
          public static <T> BodyHandler<T> buffering(BodyHandler<T> downstreamHandler,
                                                     int bufferSize) {
+             Objects.requireNonNull(downstreamHandler);
              if (bufferSize <= 0)
                  throw new IllegalArgumentException("must be greater than 0");
              return (status, headers) -> BodySubscriber
@@ -613,6 +605,7 @@
          * @return a body subscriber
          */
         public static BodySubscriber<String> asString(Charset charset) {
+            Objects.requireNonNull(charset);
             return new ResponseSubscribers.ByteArraySubscriber<>(
                     bytes -> new String(bytes, charset)
             );
@@ -662,11 +655,11 @@
          */
         public static BodySubscriber<Path> asFile(Path file, OpenOption... openOptions) {
             Objects.requireNonNull(file);
+            List<OpenOption> opts = List.of(openOptions);
             SecurityManager sm = System.getSecurityManager();
             if (sm != null) {
                 String fn = pathForSecurityCheck(file);
                 sm.checkWrite(fn);
-                List<OpenOption> opts = Arrays.asList(openOptions);
                 if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE))
                     sm.checkDelete(fn);
                 if (opts.contains(StandardOpenOption.READ))
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java	Tue Dec 19 08:51:11 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java	Mon Dec 18 10:21:38 2017 +0000
@@ -43,6 +43,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.NoSuchElementException;
+import java.util.Objects;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.Flow;
 import java.util.function.Supplier;
@@ -109,7 +110,7 @@
         private volatile long contentLength;
 
         IterablePublisher(Iterable<byte[]> content) {
-            this.content = content;
+            this.content = Objects.requireNonNull(content);
         }
 
         // The ByteBufferIterator will iterate over the byte[] arrays in
@@ -323,7 +324,7 @@
         private final Supplier<? extends InputStream> streamSupplier;
 
         InputStreamPublisher(Supplier<? extends InputStream> streamSupplier) {
-            this.streamSupplier = streamSupplier;
+            this.streamSupplier = Objects.requireNonNull(streamSupplier);
         }
 
         @Override
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java	Tue Dec 19 08:51:11 2017 -0800
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java	Mon Dec 18 10:21:38 2017 +0000
@@ -62,7 +62,7 @@
         private final AtomicBoolean subscribed = new AtomicBoolean();
 
         ConsumerSubscriber(Consumer<Optional<byte[]>> consumer) {
-            this.consumer = consumer;
+            this.consumer = Objects.requireNonNull(consumer);
         }
 
         @Override
--- a/test/jdk/java/net/httpclient/RequestProcessorExceptions.java	Tue Dec 19 08:51:11 2017 -0800
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,91 +0,0 @@
-/*
- * Copyright (c) 2017, 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
- * @run testng RequestProcessorExceptions
- */
-
-import java.io.FileNotFoundException;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.nio.file.Paths;
-import java.util.List;
-import org.testng.annotations.DataProvider;
-import org.testng.annotations.Test;
-import static jdk.incubator.http.HttpRequest.BodyPublisher.fromByteArray;
-import static jdk.incubator.http.HttpRequest.BodyPublisher.fromFile;
-
-public class RequestProcessorExceptions {
-
-    @DataProvider(name = "byteArrayOOBs")
-    public Object[][] byteArrayOOBs() {
-        return new Object[][] {
-                { new byte[100],    1,  100 },
-                { new byte[100],   -1,   10 },
-                { new byte[100],   99,    2 },
-                { new byte[1],   -100,    1 } };
-    }
-
-    @Test(dataProvider = "byteArrayOOBs", expectedExceptions = IndexOutOfBoundsException.class)
-    public void fromByteArrayCheck(byte[] buf, int offset, int length) {
-        fromByteArray(buf, offset, length);
-    }
-
-    @DataProvider(name = "nonExistentFiles")
-    public Object[][] nonExistentFiles() {
-        List<Path> paths = List.of(Paths.get("doesNotExist"),
-                                   Paths.get("tsixEtoNseod"),
-                                   Paths.get("doesNotExist2"));
-        paths.forEach(p -> {
-            if (Files.exists(p))
-                throw new AssertionError("Unexpected " + p);
-        });
-
-        return paths.stream().map(p -> new Object[] { p }).toArray(Object[][]::new);
-    }
-
-    @Test(dataProvider = "nonExistentFiles", expectedExceptions = FileNotFoundException.class)
-    public void fromFileCheck(Path path) throws Exception {
-        fromFile(path);
-    }
-
-    // ---
-
-    /* Main entry point for standalone testing of the main functional test. */
-    public static void main(String... args) throws Exception {
-        RequestProcessorExceptions t = new RequestProcessorExceptions();
-        for (Object[] objs : t.byteArrayOOBs()) {
-            try {
-                t.fromByteArrayCheck((byte[]) objs[0], (int) objs[1], (int) objs[2]);
-                throw new RuntimeException("fromByteArrayCheck failed");
-            } catch (IndexOutOfBoundsException expected) { /* Ok */ }
-        }
-        for (Object[] objs : t.nonExistentFiles()) {
-            try {
-                t.fromFileCheck((Path) objs[0]);
-                throw new RuntimeException("fromFileCheck failed");
-            } catch (FileNotFoundException expected) { /* Ok */ }
-        }
-    }
-}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/SubscriberPublisherAPIExceptions.java	Mon Dec 18 10:21:38 2017 +0000
@@ -0,0 +1,146 @@
+/*
+ * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.FileNotFoundException;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.OpenOption;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow;
+import jdk.incubator.http.HttpHeaders;
+import jdk.incubator.http.HttpRequest.BodyPublisher;
+import jdk.incubator.http.HttpResponse.BodyHandler;
+import jdk.incubator.http.HttpResponse.BodySubscriber;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.nio.file.StandardOpenOption.CREATE;
+import static java.nio.file.StandardOpenOption.WRITE;
+import static org.testng.Assert.assertThrows;
+
+/*
+ * @test
+ * @summary Basic tests for API specified exceptions from Publisher, Handler,
+ *          and Subscriber convenience static factory methods.
+ * @run testng SubscriberPublisherAPIExceptions
+ */
+
+public class SubscriberPublisherAPIExceptions {
+
+    static final Class<NullPointerException> NPE = NullPointerException.class;
+    static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class;
+    static final Class<IndexOutOfBoundsException> IOB = IndexOutOfBoundsException.class;
+
+    @Test
+    public void publisherAPIExceptions() {
+        assertThrows(NPE, () -> BodyPublisher.fromByteArray(null));
+        assertThrows(NPE, () -> BodyPublisher.fromByteArray(null, 0, 1));
+        assertThrows(IOB, () -> BodyPublisher.fromByteArray(new byte[100],    0, 101));
+        assertThrows(IOB, () -> BodyPublisher.fromByteArray(new byte[100],    1, 100));
+        assertThrows(IOB, () -> BodyPublisher.fromByteArray(new byte[100],   -1,  10));
+        assertThrows(IOB, () -> BodyPublisher.fromByteArray(new byte[100],   99,   2));
+        assertThrows(IOB, () -> BodyPublisher.fromByteArray(new byte[1],   -100,   1));
+        assertThrows(NPE, () -> BodyPublisher.fromByteArrays(null));
+        assertThrows(NPE, () -> BodyPublisher.fromFile(null));
+        assertThrows(NPE, () -> BodyPublisher.fromInputStream(null));
+        assertThrows(NPE, () -> BodyPublisher.fromString(null));
+        assertThrows(NPE, () -> BodyPublisher.fromString("A", null));
+        assertThrows(NPE, () -> BodyPublisher.fromString(null, UTF_8));
+        assertThrows(NPE, () -> BodyPublisher.fromString(null, null));
+    }
+
+    @DataProvider(name = "nonExistentFiles")
+    public Object[][] nonExistentFiles() {
+        List<Path> paths = List.of(Paths.get("doesNotExist"),
+                                   Paths.get("tsixEtoNseod"),
+                                   Paths.get("doesNotExist2"));
+        paths.forEach(p -> {
+            if (Files.exists(p))
+                throw new AssertionError("Unexpected " + p);
+        });
+
+        return paths.stream().map(p -> new Object[] { p }).toArray(Object[][]::new);
+    }
+
+    @Test(dataProvider = "nonExistentFiles", expectedExceptions = FileNotFoundException.class)
+    public void fromFileCheck(Path path) throws Exception {
+        BodyPublisher.fromFile(path);
+    }
+
+    @Test
+    public void handlerAPIExceptions() {
+        Path path = Paths.get(".").resolve("tt");
+        assertThrows(NPE, () -> BodyHandler.asByteArrayConsumer(null));
+        assertThrows(NPE, () -> BodyHandler.asFile(null));
+        assertThrows(NPE, () -> BodyHandler.asFile(null, CREATE, WRITE));
+        assertThrows(NPE, () -> BodyHandler.asFile(path, (OpenOption[])null));
+        assertThrows(NPE, () -> BodyHandler.asFile(path, new OpenOption[] {null}));
+        assertThrows(NPE, () -> BodyHandler.asFile(path, new OpenOption[] {CREATE, null}));
+        assertThrows(NPE, () -> BodyHandler.asFile(path, new OpenOption[] {null, CREATE}));
+        assertThrows(NPE, () -> BodyHandler.asFile(null, (OpenOption[])null));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(null, CREATE, WRITE));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(path, (OpenOption[])null));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(path, new OpenOption[] {null}));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(path, new OpenOption[] {CREATE, null}));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(path, new OpenOption[] {null, CREATE}));
+        assertThrows(NPE, () -> BodyHandler.asFileDownload(null, (OpenOption[])null));
+        assertThrows(NPE, () -> BodyHandler.asString(null));
+        assertThrows(NPE, () -> BodyHandler.buffering(null, 1));
+        assertThrows(IAE, () -> BodyHandler.buffering(new NoOpHandler(), 0));
+        assertThrows(IAE, () -> BodyHandler.buffering(new NoOpHandler(), -1));
+        assertThrows(IAE, () -> BodyHandler.buffering(new NoOpHandler(), Integer.MIN_VALUE));
+    }
+
+    @Test
+    public void subscriberAPIExceptions() {
+        Path path = Paths.get(".").resolve("tt");
+        assertThrows(NPE, () -> BodySubscriber.asByteArrayConsumer(null));
+        assertThrows(NPE, () -> BodySubscriber.asFile(null));
+        assertThrows(NPE, () -> BodySubscriber.asFile(null, CREATE, WRITE));
+        assertThrows(NPE, () -> BodySubscriber.asFile(path, (OpenOption[])null));
+        assertThrows(NPE, () -> BodySubscriber.asFile(path, new OpenOption[] {null}));
+        assertThrows(NPE, () -> BodySubscriber.asFile(path, new OpenOption[] {CREATE, null}));
+        assertThrows(NPE, () -> BodySubscriber.asFile(path, new OpenOption[] {null, CREATE}));
+        assertThrows(NPE, () -> BodySubscriber.asFile(null, (OpenOption[])null));
+        assertThrows(NPE, () -> BodySubscriber.asString(null));
+        assertThrows(NPE, () -> BodySubscriber.buffering(null, 1));
+        assertThrows(IAE, () -> BodySubscriber.buffering(new NoOpSubscriber(), 0));
+        assertThrows(IAE, () -> BodySubscriber.buffering(new NoOpSubscriber(), -1));
+        assertThrows(IAE, () -> BodySubscriber.buffering(new NoOpSubscriber(), Integer.MIN_VALUE));
+    }
+
+    static class NoOpHandler implements BodyHandler<Void> {
+        @Override public BodySubscriber<Void> apply(int code, HttpHeaders hrds) { return null; }
+    }
+
+    static class NoOpSubscriber implements BodySubscriber<Void> {
+        @Override public void onSubscribe(Flow.Subscription subscription) { }
+        @Override public void onNext(List<ByteBuffer> item) { }
+        @Override public void onError(Throwable throwable) { }
+        @Override public void onComplete() { }
+        @Override public CompletableFuture<Void> getBody() { return null; }
+    }
+}