http-client-branch: HttpClient uses ProxySelector.getDefault() by default http-client-branch
authordfuchs
Thu, 16 Nov 2017 19:56:44 +0000
branchhttp-client-branch
changeset 55821 fa0fc03c0853
parent 55820 76d033c9908f
child 55822 28dcbbc4148b
http-client-branch: HttpClient uses ProxySelector.getDefault() by default
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClient.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/BuilderImpl.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java
test/jdk/java/net/httpclient/security/0.policy
test/jdk/java/net/httpclient/security/1.policy
test/jdk/java/net/httpclient/security/10.policy
test/jdk/java/net/httpclient/security/11.policy
test/jdk/java/net/httpclient/security/12.policy
test/jdk/java/net/httpclient/security/14.policy
test/jdk/java/net/httpclient/security/15.policy
test/jdk/java/net/httpclient/security/2.policy
test/jdk/java/net/httpclient/security/3.policy
test/jdk/java/net/httpclient/security/4.policy
test/jdk/java/net/httpclient/security/5.policy
test/jdk/java/net/httpclient/security/6.policy
test/jdk/java/net/httpclient/security/7.policy
test/jdk/java/net/httpclient/security/8.policy
test/jdk/java/net/httpclient/security/9.policy
test/jdk/java/net/httpclient/security/filePerms/httpclient.policy
test/jdk/java/net/httpclient/websocket/security/httpclient.policy
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java	Thu Nov 16 19:56:44 2017 +0000
@@ -448,7 +448,7 @@
         } catch (SecurityException e) {
             return e;
         }
-        ProxySelector ps = client.proxy().orElse(null);
+        ProxySelector ps = client.proxySelector();
         if (ps != null) {
             if (!method.equals("CONNECT")) {
                 // a non-tunneling HTTP proxy. Need to check access
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClient.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClient.java	Thu Nov 16 19:56:44 2017 +0000
@@ -29,6 +29,7 @@
 import java.net.Authenticator;
 import java.net.CookieHandler;
 import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.ProxySelector;
 import java.net.URI;
 import java.util.Optional;
@@ -66,9 +67,16 @@
      *
      * The default settings include: the "GET" request method, a preference of
      * {@linkplain HttpClient.Version#HTTP_2 HTTP/2}, a redirection policy of
-     * {@linkplain Redirect#NEVER NEVER}, and the {@linkplain
+     * {@linkplain Redirect#NEVER NEVER}, the {@linkplain
+     * ProxySelector#getDefault() default proxy selector}, and the {@linkplain
      * SSLContext#getDefault() default SSL context}.
      *
+     * @implNote The system-wide default values are retrieved at the time the
+     * {@code HttpClient} instance is constructed. Changing the system-wide
+     * values after an {@code HttpClient} instance has been built, for
+     * instance, by calling {@link ProxySelector#setDefault(ProxySelector)},
+     * has no effect on already built instances.
+     *
      * @return a new HttpClient
      */
     public static HttpClient newHttpClient() {
@@ -98,6 +106,15 @@
     public abstract static class Builder {
 
         /**
+         * A proxy selector that always return {@link Proxy#NO_PROXY} implying
+         * a direct connection.
+         * This is a convenience object that can be passed to {@link #proxy(ProxySelector)}
+         * in order to build an instance of {@link HttpClient} that uses no
+         * proxy.
+         */
+        public static final ProxySelector NO_PROXY = ProxySelector.of(null);
+
+        /**
          * Creates a Builder.
          */
         protected Builder() {}
@@ -209,11 +226,21 @@
         /**
          * Sets a {@link java.net.ProxySelector}.
          *
-         * @implNote {@link ProxySelector#of(InetSocketAddress)}
+         * @apiNote {@link ProxySelector#of(InetSocketAddress)}
          * provides a {@code ProxySelector} which uses a single proxy for all
          * requests. The system-wide proxy selector can be retrieved by
          * {@link ProxySelector#getDefault()}.
          *
+         * @implNote
+         * If this method is not invoked prior to {@linkplain #build()
+         * building}, then newly built clients will use the {@linkplain
+         * ProxySelector#getDefault() default proxy selector}, which
+         * is normally adequate for client applications. This default
+         * behavior can be turned off by supplying an explicit proxy
+         * selector to this method, such as {@link #NO_PROXY} or one
+         * returned by {@link ProxySelector#of(InetSocketAddress)},
+         * before calling {@link #build()}.
+         *
          * @param selector the ProxySelector
          * @return this builder
          */
@@ -256,11 +283,17 @@
     public abstract Redirect followRedirects();
 
     /**
-     * Returns an {@code Optional} containing this client's {@code ProxySelector}.
-     * If no proxy selector was set in this client's builder, then the {@code
-     * Optional} is empty.
+     * Returns an {@code Optional} containing the {@code ProxySelector}
+     * supplied to this client. If no proxy selector was set in this client's
+     * builder, then the {@code Optional} is empty.
      *
-     * @return an {@code Optional} containing this client's proxy selector
+     * <p> Even though this method may return an empty optional, the {@code
+     * HttpClient} may still have an non-exposed {@linkplain
+     * Builder#proxy(ProxySelector) default proxy selector} that is
+     * used for sending HTTP requests.
+     *
+     * @return an {@code Optional} containing the proxy selector supplied
+     *        to this client.
      */
     public abstract Optional<ProxySelector> proxy();
 
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/HttpClientImpl.java	Thu Nov 16 19:56:44 2017 +0000
@@ -44,6 +44,7 @@
 import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.NoSuchAlgorithmException;
+import java.security.PrivilegedAction;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
@@ -54,7 +55,6 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -114,6 +114,7 @@
 
     private final CookieHandler cookieHandler;
     private final Redirect followRedirects;
+    private final Optional<ProxySelector> userProxySelector;
     private final ProxySelector proxySelector;
     private final Authenticator authenticator;
     private final Version version;
@@ -231,7 +232,11 @@
         cookieHandler = builder.cookieHandler;
         followRedirects = builder.followRedirects == null ?
                 Redirect.NEVER : builder.followRedirects;
-        this.proxySelector = builder.proxy;
+        this.userProxySelector = Optional.ofNullable(builder.proxy);
+        this.proxySelector = userProxySelector
+                .orElseGet(HttpClientImpl::getDefaultProxySelector);
+        debug.log(Level.DEBUG, "proxySelector is %s (user-supplied=%s)",
+                this.proxySelector, userProxySelector.isPresent());
         authenticator = builder.authenticator;
         if (builder.version == null) {
             version = HttpClient.Version.HTTP_2;
@@ -279,6 +284,11 @@
         return params;
     }
 
+    private static ProxySelector getDefaultProxySelector() {
+        PrivilegedAction<ProxySelector> action = ProxySelector::getDefault;
+        return AccessController.doPrivileged(action);
+    }
+
     // Returns the facade that was returned to the application code.
     // May be null if that facade is no longer referenced.
     final HttpClientFacade facade() {
@@ -906,7 +916,12 @@
 
     @Override
     public Optional<ProxySelector> proxy() {
-        return Optional.ofNullable(this.proxySelector);
+        return this.userProxySelector;
+    }
+
+    // Return the effective proxy that this client uses.
+    ProxySelector proxySelector() {
+        return proxySelector;
     }
 
     @Override
@@ -917,7 +932,7 @@
         // WebSocket has been created, at which point the pendingOperationCount
         // will have been incremented by the DetachedConnectionChannel
         // (see PlainHttpConnection.detachChannel())
-        return new BuilderImpl(this.facade(), uri, listener);
+        return new BuilderImpl(this.facade(), uri, listener, proxySelector);
     }
 
     @Override
@@ -936,12 +951,6 @@
         return super.toString() + ("(" + id + ")");
     }
 
-    //private final HashMap<String, Boolean> http2NotSupported = new HashMap<>();
-
-//    boolean getHttp2Allowed() {
-//        return version.equals(Version.HTTP_2);
-//    }
-
     private void initFilters() {
         addFilter(AuthenticationFilter.class);
         addFilter(RedirectFilter.class);
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java	Thu Nov 16 19:56:44 2017 +0000
@@ -51,6 +51,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CompletionException;
+import java.util.concurrent.ExecutionException;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Stream;
@@ -104,7 +105,8 @@
     }
 
     public static Throwable getCompletionCause(Throwable x) {
-        if (!(x instanceof CompletionException)) return x;
+        if (!(x instanceof CompletionException)
+                && !(x instanceof ExecutionException)) return x;
         final Throwable cause = x.getCause();
         return cause == null ? x : cause;
     }
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/BuilderImpl.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/BuilderImpl.java	Thu Nov 16 19:56:44 2017 +0000
@@ -31,11 +31,13 @@
 import jdk.incubator.http.WebSocket.Listener;
 import jdk.incubator.http.internal.common.Pair;
 
+import java.net.ProxySelector;
 import java.net.URI;
 import java.time.Duration;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 
 import static java.lang.String.format;
@@ -49,12 +51,17 @@
     private final Listener listener;
     private final Collection<Pair<String, String>> headers = new LinkedList<>();
     private final Collection<String> subprotocols = new LinkedList<>();
+    private final Optional<ProxySelector> proxySelector;
     private Duration timeout;
 
-    public BuilderImpl(HttpClient client, URI uri, Listener listener) {
+    public BuilderImpl(HttpClient client, URI uri, Listener listener, ProxySelector proxySelector) {
         this.client = requireNonNull(client, "client");
         this.uri = checkURI(requireNonNull(uri, "uri"));
         this.listener = requireNonNull(listener, "listener");
+        this.proxySelector = Optional.ofNullable(proxySelector);
+        // if the proxy selector was supplied by the user, it should be present
+        // on the client and should be the same than what we get as argument.
+        assert !client.proxy().isPresent() || client.proxy().get() == proxySelector;
     }
 
     private static IllegalArgumentException newIAE(String message, Object... args) {
@@ -123,4 +130,6 @@
     Collection<String> getSubprotocols() { return subprotocols; }
 
     Duration getConnectTimeout() { return timeout; }
+
+    Optional<ProxySelector> proxySelector() { return proxySelector; }
 }
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java	Thu Nov 16 12:15:55 2017 +0000
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java	Thu Nov 16 19:56:44 2017 +0000
@@ -122,13 +122,12 @@
      * Returns the proxy for the given URI when sent through the given client,
      * or {@code null} if none is required or applicable.
      */
-    private static Proxy proxyFor(HttpClient client, URI uri) {
-        Optional<ProxySelector> optional = client.proxy();
-        if (!optional.isPresent()) {
+    private static Proxy proxyFor(Optional<ProxySelector> selector, URI uri) {
+        if (!selector.isPresent()) {
             return null;
         }
         URI requestURI = OpeningHandshake.createRequestURI(uri);  // based on the HTTP scheme
-        List<Proxy> pl = optional.get().select(requestURI);
+        List<Proxy> pl = selector.get().select(requestURI);
         if (pl.isEmpty()) {
             return null;
         }
@@ -164,7 +163,7 @@
 
     static CompletableFuture<WebSocket> newInstanceAsync(BuilderImpl b) {
         // TODO: a security issue? TOCTOU: two accesses to b.getURI
-        Proxy proxy = proxyFor(b.getClient(), b.getUri());
+        Proxy proxy = proxyFor(b.proxySelector(), b.getUri());
         try {
             checkPermissions(b, proxy);
         } catch (Throwable throwable) {
--- a/test/jdk/java/net/httpclient/security/0.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/0.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/1.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/1.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,6 +43,7 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
 
--- a/test/jdk/java/net/httpclient/security/10.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/10.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -42,5 +42,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/11.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/11.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -44,5 +44,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/12.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/12.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -44,5 +44,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/14.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/14.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/15.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/15.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -46,5 +46,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/2.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/2.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/3.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/3.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/4.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/4.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -44,5 +44,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/5.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/5.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/6.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/6.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/7.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/7.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/8.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/8.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/9.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/9.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -43,5 +43,6 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 };
 
--- a/test/jdk/java/net/httpclient/security/filePerms/httpclient.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/security/filePerms/httpclient.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -36,6 +36,7 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 
     permission java.security.SecurityPermission "createAccessControlContext";
 };
--- a/test/jdk/java/net/httpclient/websocket/security/httpclient.policy	Thu Nov 16 12:15:55 2017 +0000
+++ b/test/jdk/java/net/httpclient/websocket/security/httpclient.policy	Thu Nov 16 19:56:44 2017 +0000
@@ -36,6 +36,7 @@
     // ## these permissions do not appear in the NetPermission spec!!! JDK bug?
     permission java.net.NetPermission "getSSLContext";
     permission java.net.NetPermission "setSSLContext";
+    permission java.net.NetPermission "getProxySelector";
 
     permission java.security.SecurityPermission "createAccessControlContext";
 };