# HG changeset patch # User chegar # Date 1513592498 0 # Mon Dec 18 10:21:38 2017 +0000 # Branch http-client-branch # Node ID 480572471bbce7cefdf4130b35c1a87ad200b6d6 # Parent 087a6b6d4955bcac8084959171f0a127b1c1ba80 [mq]: PubSubNPE diff --git a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/BufferingSubscriber.java @@ -80,7 +80,7 @@ BufferingSubscriber(HttpResponse.BodySubscriber downstreamSubscriber, int bufferSize) { - this.downstreamSubscriber = downstreamSubscriber; + this.downstreamSubscriber = Objects.requireNonNull(downstreamSubscriber); this.bufferSize = bufferSize; synchronized (buffersLock) { internalBuffers = new ArrayList<>(); diff --git a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpResponse.java @@ -344,11 +344,7 @@ * Returns a {@code BodyHandler} that returns a * {@link BodySubscriber BodySubscriber}{@code } 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 @@ -356,12 +352,8 @@ * @return a response body handler */ public static BodyHandler 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 +378,12 @@ */ public static BodyHandler asFile(Path file, OpenOption... openOptions) { Objects.requireNonNull(file); + Objects.requireNonNull(openOptions); + List opts = List.of(openOptions); SecurityManager sm = System.getSecurityManager(); if (sm != null) { String fn = pathForSecurityCheck(file); sm.checkWrite(fn); - List opts = Arrays.asList(openOptions); if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE)) sm.checkDelete(fn); if (opts.contains(StandardOpenOption.READ)) @@ -450,11 +443,12 @@ public static BodyHandler asFileDownload(Path directory, OpenOption... openOptions) { Objects.requireNonNull(directory); + Objects.requireNonNull(openOptions); + List opts = List.of(openOptions); SecurityManager sm = System.getSecurityManager(); if (sm != null) { String fn = pathForSecurityCheck(directory); sm.checkWrite(fn); - List opts = Arrays.asList(openOptions); if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE)) sm.checkDelete(fn); if (opts.contains(StandardOpenOption.READ)) @@ -494,6 +488,7 @@ * @return a response body handler */ public static BodyHandler asByteArrayConsumer(Consumer> consumer) { + Objects.requireNonNull(consumer); return (status, headers) -> BodySubscriber.asByteArrayConsumer(consumer); } @@ -547,6 +542,7 @@ */ public static BodyHandler buffering(BodyHandler downstreamHandler, int bufferSize) { + Objects.requireNonNull(downstreamHandler); if (bufferSize <= 0) throw new IllegalArgumentException("must be greater than 0"); return (status, headers) -> BodySubscriber @@ -613,6 +609,7 @@ * @return a body subscriber */ public static BodySubscriber asString(Charset charset) { + Objects.requireNonNull(charset); return new ResponseSubscribers.ByteArraySubscriber<>( bytes -> new String(bytes, charset) ); @@ -662,11 +659,12 @@ */ public static BodySubscriber asFile(Path file, OpenOption... openOptions) { Objects.requireNonNull(file); + Objects.requireNonNull(openOptions); + List opts = List.of(openOptions); SecurityManager sm = System.getSecurityManager(); if (sm != null) { String fn = pathForSecurityCheck(file); sm.checkWrite(fn); - List opts = Arrays.asList(openOptions); if (opts.contains(StandardOpenOption.DELETE_ON_CLOSE)) sm.checkDelete(fn); if (opts.contains(StandardOpenOption.READ)) diff --git a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/RequestPublishers.java @@ -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 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 streamSupplier; InputStreamPublisher(Supplier streamSupplier) { - this.streamSupplier = streamSupplier; + this.streamSupplier = Objects.requireNonNull(streamSupplier); } @Override diff --git a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/ResponseSubscribers.java @@ -62,7 +62,7 @@ private final AtomicBoolean subscribed = new AtomicBoolean(); ConsumerSubscriber(Consumer> consumer) { - this.consumer = consumer; + this.consumer = Objects.requireNonNull(consumer); } @Override diff --git a/test/jdk/java/net/httpclient/RequestProcessorExceptions.java b/test/jdk/java/net/httpclient/RequestProcessorExceptions.java deleted file mode 100644 --- a/test/jdk/java/net/httpclient/RequestProcessorExceptions.java +++ /dev/null @@ -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 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 */ } - } - } -} diff --git a/test/jdk/java/net/httpclient/SubscriberPublisherAPIExceptions.java b/test/jdk/java/net/httpclient/SubscriberPublisherAPIExceptions.java new file mode 100644 --- /dev/null +++ b/test/jdk/java/net/httpclient/SubscriberPublisherAPIExceptions.java @@ -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 NPE = NullPointerException.class; + static final Class IAE = IllegalArgumentException.class; + static final Class 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 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 { + @Override public BodySubscriber apply(int code, HttpHeaders hrds) { return null; } + } + + static class NoOpSubscriber implements BodySubscriber { + @Override public void onSubscribe(Flow.Subscription subscription) { } + @Override public void onNext(List item) { } + @Override public void onError(Throwable throwable) { } + @Override public void onComplete() { } + @Override public CompletableFuture getBody() { return null; } + } +}