http-client-branch: verify that send/sendAsync will throw IAE and NPE as specified by HttpRequest.Builder http-client-branch
authordfuchs
Thu, 22 Mar 2018 14:45:30 +0000
branchhttp-client-branch
changeset 56342 5c2ea761455b
parent 56341 10fcbe13cd19
child 56343 2aa07dd36870
http-client-branch: verify that send/sendAsync will throw IAE and NPE as specified by HttpRequest.Builder
src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestBuilderImpl.java
src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java
src/java.net.http/share/classes/jdk/internal/net/http/ImmutableHeaders.java
src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java
test/jdk/java/net/httpclient/HeadersTest.java
test/jdk/java/net/httpclient/MethodsTest.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestBuilderImpl.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestBuilderImpl.java	Thu Mar 22 14:45:30 2018 +0000
@@ -37,6 +37,7 @@
 import static java.util.Objects.requireNonNull;
 import static jdk.internal.net.http.common.Utils.isValidName;
 import static jdk.internal.net.http.common.Utils.isValidValue;
+import static jdk.internal.net.http.common.Utils.newIAE;
 
 public class HttpRequestBuilderImpl implements HttpRequest.Builder {
 
@@ -71,11 +72,7 @@
         return this;
     }
 
-    private static IllegalArgumentException newIAE(String message, Object... args) {
-        return new IllegalArgumentException(format(message, args));
-    }
-
-    private static void checkURI(URI uri) {
+    static void checkURI(URI uri) {
         String scheme = uri.getScheme();
         if (scheme == null)
             throw newIAE("URI with undefined scheme");
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java	Thu Mar 22 14:45:30 2018 +0000
@@ -36,6 +36,7 @@
 import java.time.Duration;
 import java.util.List;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.Optional;
 import java.net.http.HttpClient;
 import java.net.http.HttpHeaders;
@@ -101,8 +102,11 @@
                     .replace("\r", "\\r")
                     .replace("\t", "\\t")
                     + "\"");
+        URI requestURI = Objects.requireNonNull(request.uri(),
+                "uri must be non null");
+        Duration timeout = request.timeout().orElse(null);
         this.method = method == null ? "GET" : method;
-        this.userHeaders = ImmutableHeaders.of(request.headers());
+        this.userHeaders = ImmutableHeaders.validate(request.headers());
         if (request instanceof HttpRequestImpl) {
             // all cases exception WebSocket should have a new system headers
             this.isWebSocket = ((HttpRequestImpl) request).isWebSocket;
@@ -112,10 +116,12 @@
                 this.systemHeaders = new HttpHeadersImpl();
             }
         } else {
+            HttpRequestBuilderImpl.checkURI(requestURI);
             this.systemHeaders = new HttpHeadersImpl();
+            checkTimeout(timeout);
         }
         this.systemHeaders.setHeader("User-Agent", USER_AGENT);
-        this.uri = request.uri();
+        this.uri = requestURI;
         if (isWebSocket) {
             // WebSocket determines and sets the proxy itself
             this.proxy = ((HttpRequestImpl) request).proxy;
@@ -128,11 +134,18 @@
         this.expectContinue = request.expectContinue();
         this.secure = uri.getScheme().toLowerCase(Locale.US).equals("https");
         this.requestPublisher = request.bodyPublisher().orElse(null);
-        this.timeout = request.timeout().orElse(null);
+        this.timeout = timeout;
         this.version = request.version();
         this.authority = null;
     }
 
+    private static void checkTimeout(Duration duration) {
+        if (duration != null) {
+            if (duration.isNegative() || Duration.ZERO.equals(duration))
+                throw new IllegalArgumentException("Invalid duration: " + duration);
+        }
+    }
+
     /** Returns a new instance suitable for redirection. */
     public static HttpRequestImpl newInstanceForRedirection(URI uri,
                                                             String method,
--- a/src/java.net.http/share/classes/jdk/internal/net/http/ImmutableHeaders.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/ImmutableHeaders.java	Thu Mar 22 14:45:30 2018 +0000
@@ -28,10 +28,13 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TreeMap;
 import java.util.function.BiPredicate;
 import java.util.function.Predicate;
 import java.net.http.HttpHeaders;
+import jdk.internal.net.http.common.HttpHeadersImpl;
+import jdk.internal.net.http.common.Utils;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableList;
 import static java.util.Collections.unmodifiableMap;
@@ -55,6 +58,17 @@
                 : of(headers.map());
     }
 
+    static ImmutableHeaders validate(HttpHeaders headers) {
+        if (headers instanceof ImmutableHeaders) {
+            return of(headers);
+        }
+        if (headers instanceof HttpHeadersImpl) {
+            return of(headers);
+        }
+        Map<String, List<String>> map = headers.map();
+        return new ImmutableHeaders(map, Utils.VALIDATE_USER_HEADER);
+    }
+
     public static ImmutableHeaders of(Map<String, List<String>> src,
                                       Predicate<? super String> keyAllowed) {
         requireNonNull(src, "src");
@@ -73,16 +87,24 @@
                              BiPredicate<? super String, ? super List<String>> headerAllowed) {
         Map<String, List<String>> m = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
         src.entrySet().stream()
-                .filter(e -> headerAllowed.test(e.getKey(), e.getValue()))
-                .forEach(e ->
-                        {
-                            List<String> values = new ArrayList<>(e.getValue());
-                            m.put(e.getKey(), unmodifiableList(values));
-                        }
-                );
+                .forEach(e -> addIfAllowed(e, headerAllowed, m));
         this.map = unmodifiableMap(m);
     }
 
+    private static void addIfAllowed(Map.Entry<String, List<String>> e,
+                                     BiPredicate<? super String, ? super List<String>> headerAllowed,
+                                     Map<String, List<String>> map) {
+        String key = e.getKey();
+        List<String> values = unmodifiableValues(e.getValue());
+        if (headerAllowed.test(key, values)) {
+            map.put(key, values);
+        }
+    }
+
+    private static List<String> unmodifiableValues(List<String> values) {
+        return unmodifiableList(new ArrayList<>(Objects.requireNonNull(values)));
+    }
+
     private static BiPredicate<String, List<String>> headerAllowed(Predicate<? super String> keyAllowed) {
         return (n,v) -> keyAllowed.test(n);
     }
--- a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java	Thu Mar 22 14:45:30 2018 +0000
@@ -63,6 +63,8 @@
 import javax.net.ssl.SSLSession;
 import javax.net.ssl.ExtendedSSLSession;
 
+import static java.lang.String.format;
+import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.joining;
 
 /**
@@ -71,11 +73,13 @@
 public final class Utils {
 
     public static final boolean ASSERTIONSENABLED;
+
     static {
         boolean enabled = false;
         assert enabled = true;
         ASSERTIONSENABLED = enabled;
     }
+
 //    public static final boolean TESTING;
 //    static {
 //        if (ASSERTIONSENABLED) {
@@ -98,7 +102,7 @@
         String prop = getProperty("jdk.internal.httpclient.disableHostnameVerification");
         if (prop == null)
             return false;
-        return prop.isEmpty() ?  true : Boolean.parseBoolean(prop);
+        return prop.isEmpty() ? true : Boolean.parseBoolean(prop);
     }
 
     /**
@@ -113,6 +117,7 @@
     );
 
     private static final Set<String> DISALLOWED_HEADERS_SET;
+
     static {
         // A case insensitive TreeSet of strings.
         TreeSet<String> treeSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER);
@@ -124,7 +129,26 @@
     }
 
     public static final Predicate<String>
-        ALLOWED_HEADERS = header -> !DISALLOWED_HEADERS_SET.contains(header);
+            ALLOWED_HEADERS = header -> !DISALLOWED_HEADERS_SET.contains(header);
+
+    public static final BiPredicate<String, List<String>> VALIDATE_USER_HEADER =
+            (name, lv) -> {
+                requireNonNull(name, "header name");
+                requireNonNull(lv, "header values");
+                if (!isValidName(name)) {
+                    throw newIAE("invalid header name: \"%s\"", name);
+                }
+                if (!Utils.ALLOWED_HEADERS.test(name)) {
+                    throw newIAE("restricted header name: \"%s\"", name);
+                }
+                for (String value : lv) {
+                    requireNonNull(value, "header value");
+                    if (!isValidValue(value)) {
+                        throw newIAE("invalid header value for %s: \"%s\"", name, value);
+                    }
+                }
+                return true;
+            };
 
     private static final Predicate<String> IS_PROXY_HEADER = (k) ->
             k != null && k.length() > 6 && "proxy-".equalsIgnoreCase(k.substring(0,6));
@@ -201,6 +225,9 @@
                       : ! PROXY_AUTH_DISABLED_SCHEMES.isEmpty();
     }
 
+    public static IllegalArgumentException newIAE(String message, Object... args) {
+        return new IllegalArgumentException(format(message, args));
+    }
     public static ByteBuffer getBuffer() {
         return ByteBuffer.allocate(BUFSIZE);
     }
@@ -377,6 +404,7 @@
         return accepted;
     }
 
+
     public static int getIntegerNetProperty(String name, int defaultValue) {
         return AccessController.doPrivileged((PrivilegedAction<Integer>) () ->
                 NetProperties.getInteger(name, defaultValue));
--- a/test/jdk/java/net/httpclient/HeadersTest.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/test/jdk/java/net/httpclient/HeadersTest.java	Thu Mar 22 14:45:30 2018 +0000
@@ -21,24 +21,298 @@
  * questions.
  */
 
+import java.net.http.HttpClient;
+import java.net.http.HttpHeaders;
 import java.net.http.HttpRequest;
 import java.net.URI;
+import java.net.http.HttpResponse;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import static java.net.http.HttpClient.Builder.NO_PROXY;
 
 /**
  * @test
  * @bug 8087112
- * @summary Basic test for headers
+ * @summary Basic test for headers, uri, and duration
  */
 public class HeadersTest {
 
     static final URI TEST_URI = URI.create("http://www.foo.com/");
+    static final HttpClient client = HttpClient.newBuilder().proxy(NO_PROXY).build();
 
-    static void bad(String name) {
+    static final class HttpHeadersStub extends HttpHeaders {
+        Map<String, List<String>> map;
+        HttpHeadersStub(Map<String, List<String>> map) {
+            this.map = map;
+        }
+        @Override
+        public Map<String, List<String>> map() {
+            return map;
+        }
+    }
+
+    static void bad(String name) throws Exception {
         HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
         try {
             builder.header(name, "foo");
             throw new RuntimeException("Expected IAE for header:" + name);
-        } catch (IllegalArgumentException expected) { }
+        } catch (IllegalArgumentException expected)  {
+            System.out.println("Got expected IAE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = Map.of(name, List.of("foo"));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected IAE for header:" + name);
+        } catch (IllegalArgumentException expected) {
+            System.out.println("Got expected IAE: " + expected);
+        }
+    }
+
+    static void badValue(String value) throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
+        try {
+            builder.header("x-bad", value);
+            throw new RuntimeException("Expected IAE for header x-bad: "
+                    + value.replace("\r", "\\r")
+                    .replace("\n", "\\n"));
+        } catch (IllegalArgumentException expected)  {
+            System.out.println("Got expected IAE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = Map.of("x-bad", List.of(value));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected IAE for header x-bad:"
+                    + value.replace("\r", "\\r")
+                    .replace("\n", "\\n"));
+        } catch (IllegalArgumentException expected) {
+            System.out.println("Got expected IAE: " + expected);
+        }
+    }
+
+    static void nullName() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
+        try {
+            builder.header(null, "foo");
+            throw new RuntimeException("Expected NPE for null header name");
+        } catch (NullPointerException expected)  {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = new HashMap<>();
+                    map.put(null, List.of("foo"));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null header name");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+    }
+
+    static void nullValue() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
+        try {
+            builder.header("x-bar", null);
+            throw new RuntimeException("Expected NPE for null header value");
+        } catch (NullPointerException expected)  {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = new HashMap<>();
+                    map.put("x-bar", null);
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null header values");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    List<String> values = new ArrayList<>();
+                    values.add("foo");
+                    values.add(null);
+                    return new HttpHeadersStub(Map.of("x-bar", values));
+                }
+            };
+            client
+                    .send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null header value");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+    }
+
+    static void nullHeaders() throws Exception {
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    return new HttpHeadersStub(null);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null header name");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return TEST_URI;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    return null;
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null header name");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
     }
 
     static void good(String name) {
@@ -50,7 +324,210 @@
         }
     }
 
-    public static void main(String[] args) {
+    static void badURI() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder();
+        URI uri = URI.create(TEST_URI.toString().replace("http", "ftp"));
+        try {
+            builder.uri(uri);
+            throw new RuntimeException("Expected IAE for uri: " + uri);
+        } catch (IllegalArgumentException expected)  {
+            System.out.println("Got expected IAE: " + expected);
+        }
+        try {
+            HttpRequest.newBuilder(uri);
+            throw new RuntimeException("Expected IAE for uri: " + uri);
+        } catch (IllegalArgumentException expected)  {
+            System.out.println("Got expected IAE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return uri;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = Map.of("x-good", List.of("foo"));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected IAE for uri:" + uri);
+        } catch (IllegalArgumentException expected) {
+            System.out.println("Got expected IAE: " + expected);
+        }
+    }
+
+    static void nullURI() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder();
+        try {
+            builder.uri(null);
+            throw new RuntimeException("Expected NPE for null URI");
+        } catch (NullPointerException expected)  {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest.newBuilder(null);
+            throw new RuntimeException("Expected NPE for null uri");
+        } catch (NullPointerException expected)  {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+                @Override public String method() {
+                    return "GET";
+                }
+                @Override public Optional<Duration> timeout() {
+                    return Optional.empty();
+                }
+                @Override public boolean expectContinue() {
+                    return false;
+                }
+                @Override public URI uri() {
+                    return null;
+                }
+                @Override public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+                @Override public HttpHeaders headers() {
+                    Map<String, List<String>> map = Map.of("x-good", List.of("foo"));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null uri");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+    }
+
+    static void badTimeout() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
+        Duration zero = Duration.ofSeconds(0);
+        Duration negative = Duration.ofSeconds(-10);
+        for (Duration bad : List.of(zero, negative)) {
+            try {
+                builder.timeout(zero);
+                throw new RuntimeException("Expected IAE for timeout: " + bad);
+            } catch (IllegalArgumentException expected) {
+                System.out.println("Got expected IAE: " + expected);
+            }
+            try {
+                HttpRequest req = new HttpRequest() {
+                    @Override
+                    public Optional<BodyPublisher> bodyPublisher() {
+                        return Optional.of(BodyPublishers.noBody());
+                    }
+
+                    @Override
+                    public String method() {
+                        return "GET";
+                    }
+
+                    @Override
+                    public Optional<Duration> timeout() {
+                        return Optional.of(bad);
+                    }
+
+                    @Override
+                    public boolean expectContinue() {
+                        return false;
+                    }
+
+                    @Override
+                    public URI uri() {
+                        return TEST_URI;
+                    }
+
+                    @Override
+                    public Optional<HttpClient.Version> version() {
+                        return Optional.empty();
+                    }
+
+                    @Override
+                    public HttpHeaders headers() {
+                        Map<String, List<String>> map = Map.of("x-good", List.of("foo"));
+                        return new HttpHeadersStub(map);
+                    }
+                };
+                client.send(req, HttpResponse.BodyHandlers.ofString());
+                throw new RuntimeException("Expected IAE for timeout:" + bad);
+            } catch (IllegalArgumentException expected) {
+                System.out.println("Got expected IAE: " + expected);
+            }
+        }
+    }
+
+    static void nullTimeout() throws Exception {
+        HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
+        try {
+            builder.timeout(null);
+            throw new RuntimeException("Expected NPE for null timeout");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+        try {
+            HttpRequest req = new HttpRequest() {
+                @Override
+                public Optional<BodyPublisher> bodyPublisher() {
+                    return Optional.of(BodyPublishers.noBody());
+                }
+
+                @Override
+                public String method() {
+                    return "GET";
+                }
+
+                @Override
+                public Optional<Duration> timeout() {
+                    return null;
+                }
+
+                @Override
+                public boolean expectContinue() {
+                    return false;
+                }
+
+                @Override
+                public URI uri() {
+                    return TEST_URI;
+                }
+
+                @Override
+                public Optional<HttpClient.Version> version() {
+                    return Optional.empty();
+                }
+
+                @Override
+                public HttpHeaders headers() {
+                    Map<String, List<String>> map = Map.of("x-good", List.of("foo"));
+                    return new HttpHeadersStub(map);
+                }
+            };
+            client.send(req, HttpResponse.BodyHandlers.ofString());
+            throw new RuntimeException("Expected NPE for null timeout");
+        } catch (NullPointerException expected) {
+            System.out.println("Got expected NPE: " + expected);
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
         bad("bad:header");
         bad("Foo\n");
         good("X-Foo!");
@@ -60,5 +537,13 @@
         bad("Bar\r\n");
         good("Hello#world");
         good("Qwer#ert");
+        badValue("blah\r\n blah");
+        nullName();
+        nullValue();
+        nullHeaders();
+        badURI();
+        nullURI();
+        badTimeout();
+        nullTimeout();
     }
 }
--- a/test/jdk/java/net/httpclient/MethodsTest.java	Thu Mar 22 09:48:27 2018 +0000
+++ b/test/jdk/java/net/httpclient/MethodsTest.java	Thu Mar 22 14:45:30 2018 +0000
@@ -31,6 +31,8 @@
 import java.net.http.HttpResponse;
 import java.time.Duration;
 import java.util.Optional;
+import static java.net.http.HttpClient.Builder.NO_PROXY;
+
 
 /**
  * @test
@@ -42,6 +44,7 @@
 
     static final URI TEST_URI = URI.create("http://www.foo.com/");
     static final String FORBIDDEN = "()<>@,;:\\\"/[]?={} \t\r\n";
+    static final HttpClient client = HttpClient.newBuilder().proxy(NO_PROXY).build();
 
     static void bad(String name) throws IOException, InterruptedException {
         HttpRequest.Builder builder = HttpRequest.newBuilder(TEST_URI);
@@ -75,7 +78,7 @@
                     return new HttpHeadersImpl();
                 }
             };
-            HttpClient.newHttpClient().send(req, HttpResponse.BodyHandlers.ofString());
+            client.send(req, HttpResponse.BodyHandlers.ofString());
             throw new RuntimeException("Expected IAE for method:" + name);
         } catch (IllegalArgumentException expected) {
             System.out.println("Got expected IAE: " + expected);