# HG changeset patch # User dfuchs # Date 1521729930 0 # Node ID 5c2ea761455b9c32e53f51aa8215181aae7a19d4 # Parent 10fcbe13cd195ff02064e8a4ca141d2e7c381d52 http-client-branch: verify that send/sendAsync will throw IAE and NPE as specified by HttpRequest.Builder diff -r 10fcbe13cd19 -r 5c2ea761455b src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestBuilderImpl.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"); diff -r 10fcbe13cd19 -r 5c2ea761455b src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java --- 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, diff -r 10fcbe13cd19 -r 5c2ea761455b src/java.net.http/share/classes/jdk/internal/net/http/ImmutableHeaders.java --- 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> map = headers.map(); + return new ImmutableHeaders(map, Utils.VALIDATE_USER_HEADER); + } + public static ImmutableHeaders of(Map> src, Predicate keyAllowed) { requireNonNull(src, "src"); @@ -73,16 +87,24 @@ BiPredicate> headerAllowed) { Map> m = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); src.entrySet().stream() - .filter(e -> headerAllowed.test(e.getKey(), e.getValue())) - .forEach(e -> - { - List 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> e, + BiPredicate> headerAllowed, + Map> map) { + String key = e.getKey(); + List values = unmodifiableValues(e.getValue()); + if (headerAllowed.test(key, values)) { + map.put(key, values); + } + } + + private static List unmodifiableValues(List values) { + return unmodifiableList(new ArrayList<>(Objects.requireNonNull(values))); + } + private static BiPredicate> headerAllowed(Predicate keyAllowed) { return (n,v) -> keyAllowed.test(n); } diff -r 10fcbe13cd19 -r 5c2ea761455b src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java --- 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 DISALLOWED_HEADERS_SET; + static { // A case insensitive TreeSet of strings. TreeSet treeSet = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); @@ -124,7 +129,26 @@ } public static final Predicate - ALLOWED_HEADERS = header -> !DISALLOWED_HEADERS_SET.contains(header); + ALLOWED_HEADERS = header -> !DISALLOWED_HEADERS_SET.contains(header); + + public static final BiPredicate> 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 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) () -> NetProperties.getInteger(name, defaultValue)); diff -r 10fcbe13cd19 -r 5c2ea761455b test/jdk/java/net/httpclient/HeadersTest.java --- 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> map; + HttpHeadersStub(Map> map) { + this.map = map; + } + @Override + public Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + List 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return TEST_URI; + } + @Override public Optional 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return uri; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + @Override public String method() { + return "GET"; + } + @Override public Optional timeout() { + return Optional.empty(); + } + @Override public boolean expectContinue() { + return false; + } + @Override public URI uri() { + return null; + } + @Override public Optional version() { + return Optional.empty(); + } + @Override public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + + @Override + public String method() { + return "GET"; + } + + @Override + public Optional timeout() { + return Optional.of(bad); + } + + @Override + public boolean expectContinue() { + return false; + } + + @Override + public URI uri() { + return TEST_URI; + } + + @Override + public Optional version() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + Map> 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() { + return Optional.of(BodyPublishers.noBody()); + } + + @Override + public String method() { + return "GET"; + } + + @Override + public Optional timeout() { + return null; + } + + @Override + public boolean expectContinue() { + return false; + } + + @Override + public URI uri() { + return TEST_URI; + } + + @Override + public Optional version() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + Map> 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(); } } diff -r 10fcbe13cd19 -r 5c2ea761455b test/jdk/java/net/httpclient/MethodsTest.java --- 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);