8193698: Null handling in BodyPublisher, BodyHandler, and BodySubscriber convenience static factory methods
Reviewed-by: dfuchs
--- 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; }
+ }
+}