http-client-branch: common URLPermission paths http-client-branch
authorprappo
Mon, 13 Nov 2017 16:13:11 +0300
branchhttp-client-branch
changeset 55805 371ed971281e
parent 55804 b36de693e838
child 55806 e8bc8370f528
http-client-branch: common URLPermission paths
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.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/OpeningHandshake.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java	Mon Nov 13 14:43:09 2017 +0300
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Exchange.java	Mon Nov 13 16:13:11 2017 +0300
@@ -35,13 +35,14 @@
 import java.security.AccessControlContext;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.Executor;
 import jdk.incubator.http.internal.common.MinimalFuture;
 import jdk.incubator.http.internal.common.Utils;
 import jdk.incubator.http.internal.common.Log;
 
+import static jdk.incubator.http.internal.common.Utils.permissionForProxy;
+
 /**
  * One request/response exchange (handles 100/101 intermediate response also).
  * depth field used to track number of times a new request is being sent
@@ -414,55 +415,13 @@
      * If method is CONNECT, then uri must be of form "scheme://host:port"
      */
     private static URLPermission permissionForServer(URI uri,
-                                                    String method,
-                                                    Map<String, List<String>> headers) {
-        StringBuilder sb = new StringBuilder();
-
-        String urlstring, actionstring;
-
+                                                     String method,
+                                                     Map<String, List<String>> headers) {
         if (method.equals("CONNECT")) {
-            urlstring = uri.toString();
-            actionstring = "CONNECT";
+            return new URLPermission(uri.toString(), "CONNECT");
         } else {
-            sb.append(uri.getScheme()).append("://")
-              .append(uri.getAuthority())
-              .append(uri.getPath());
-            urlstring = sb.toString();
-
-            sb = new StringBuilder();
-            sb.append(method);
-            if (headers != null && !headers.isEmpty()) {
-                sb.append(':');
-                Set<String> keys = headers.keySet();
-                boolean first = true;
-                for (String key : keys) {
-                    if (!first) {
-                        sb.append(',');
-                    }
-                    sb.append(key);
-                    first = false;
-                }
-            }
-            actionstring = sb.toString();
+            return Utils.permissionForServer(uri, method, headers.keySet().stream());
         }
-        return new URLPermission(urlstring, actionstring);
-    }
-
-    /**
-     * Returns the security permissions required to connect to the proxy, or
-     * null if none is required or applicable.
-     */
-    static URLPermission permissionForProxy(HttpRequestImpl request) {
-        InetSocketAddress proxyAddress = request.proxy();
-        if (proxyAddress == null)
-            return null;
-
-        StringBuilder sb = new StringBuilder();
-        sb.append("socket://")
-          .append(proxyAddress.getHostString()).append(":")
-          .append(proxyAddress.getPort());
-        String urlString = sb.toString();
-        return new URLPermission(urlString, "CONNECT");
     }
 
     /**
@@ -493,7 +452,7 @@
         if (ps != null) {
             if (!method.equals("CONNECT")) {
                 // a non-tunneling HTTP proxy. Need to check access
-                URLPermission proxyPerm = permissionForProxy(request);
+                URLPermission proxyPerm = permissionForProxy(request.proxy());
                 if (proxyPerm != null) {
                     try {
                         sm.checkPermission(proxyPerm, acc);
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java	Mon Nov 13 14:43:09 2017 +0300
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/Utils.java	Mon Nov 13 16:13:11 2017 +0300
@@ -25,6 +25,7 @@
 
 package jdk.incubator.http.internal.common;
 
+import jdk.incubator.http.HttpHeaders;
 import sun.net.NetProperties;
 import sun.net.util.IPAddressUtil;
 
@@ -32,12 +33,14 @@
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.IOException;
+import java.io.PrintStream;
 import java.io.UncheckedIOException;
-import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
 import java.lang.System.Logger;
 import java.lang.System.Logger.Level;
 import java.net.InetSocketAddress;
+import java.net.URI;
+import java.net.URLPermission;
 import java.nio.ByteBuffer;
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
@@ -45,14 +48,14 @@
 import java.security.PrivilegedAction;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
-import java.util.LinkedList;
 import java.util.Set;
 import java.util.concurrent.CompletionException;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
-import jdk.incubator.http.HttpHeaders;
+import java.util.stream.Stream;
+
+import static java.util.stream.Collectors.joining;
 
 /**
  * Miscellaneous utilities
@@ -119,6 +122,42 @@
 
     private Utils() { }
 
+    /**
+     * Returns the security permissions required to connect to the proxy, or
+     * {@code null} if none is required or applicable.
+     */
+    public static URLPermission permissionForProxy(InetSocketAddress proxyAddress) {
+        if (proxyAddress == null)
+            return null;
+
+        StringBuilder sb = new StringBuilder();
+        sb.append("socket://")
+          .append(proxyAddress.getHostString()).append(":")
+          .append(proxyAddress.getPort());
+        String urlString = sb.toString();
+        return new URLPermission(urlString, "CONNECT");
+    }
+
+    /**
+     * Returns the security permission required for the given details.
+     */
+    public static URLPermission permissionForServer(URI uri,
+                                                    String method,
+                                                    Stream<String> headers) {
+        String urlString = new StringBuilder()
+                .append(uri.getScheme()).append("://")
+                .append(uri.getAuthority())
+                .append(uri.getPath()).toString();
+
+        StringBuilder actionStringBuilder = new StringBuilder(method);
+        String collected = headers.collect(joining(","));
+        if (!collected.isEmpty()) {
+            actionStringBuilder.append(":").append(collected);
+        }
+        return new URLPermission(urlString, actionStringBuilder.toString());
+    }
+
+
     // ABNF primitives defined in RFC 7230
     private static final boolean[] tchar      = new boolean[256];
     private static final boolean[] fieldvchar = new boolean[256];
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/OpeningHandshake.java	Mon Nov 13 14:43:09 2017 +0300
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/OpeningHandshake.java	Mon Nov 13 16:13:11 2017 +0300
@@ -175,7 +175,7 @@
                            null); // No fragment
         } catch (URISyntaxException e) {
             // Shouldn't happen: URI invariant
-            throw new InternalError(e);
+            throw new InternalError(e); // TODO: should actually report on this instead of throwing in builder (rev. 47704:34d7cc00f87a4b18b6c30c122fc3d55456833ae0)
         }
     }
 
@@ -251,6 +251,7 @@
         String expected = Base64.getEncoder().encodeToString(this.sha1.digest());
         String actual = requireSingle(headers, HEADER_ACCEPT);
         if (!actual.trim().equals(expected)) {
+            // TODO: why do we need the value here?
             throw checkFailed("Bad " + HEADER_ACCEPT + ", expected:["
                               + expected + "] ,got:[" + actual.trim() + "]");
         }
--- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java	Mon Nov 13 14:43:09 2017 +0300
+++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/websocket/WebSocketImpl.java	Mon Nov 13 16:13:11 2017 +0300
@@ -65,6 +65,7 @@
 import static java.util.concurrent.CompletableFuture.failedFuture;
 import static java.util.stream.Collectors.joining;
 import static jdk.incubator.http.internal.common.Pair.pair;
+import static jdk.incubator.http.internal.common.Utils.permissionForProxy;
 import static jdk.incubator.http.internal.websocket.StatusCodes.CLOSED_ABNORMALLY;
 import static jdk.incubator.http.internal.websocket.StatusCodes.NO_STATUS_CODE;
 import static jdk.incubator.http.internal.websocket.StatusCodes.isLegalToSendFromClient;
@@ -117,58 +118,24 @@
     private final CompletableFuture<?> closeReceived = new MinimalFuture<>();
     private final CompletableFuture<?> closeSent = new MinimalFuture<>();
 
-    /** Returns the security permission required for the given details. */
-    static URLPermission permissionForServer(URI uri,
-                                             Collection<Pair<String, String>> headers) {
-        StringBuilder sb = new StringBuilder();
-        sb.append(uri.getScheme()).append("://")
-          .append(uri.getAuthority())
-          .append(uri.getPath());
-        String urlstring = sb.toString();
-
-        String actionstring = headers.stream()
-                .map(p -> p.first)
-                .distinct()
-                .collect(joining(","));
-        if (actionstring != null && !actionstring.equals(""))
-            actionstring = ":" + actionstring;     // Note: no method in the action string
-
-        return new URLPermission(urlstring, actionstring);
-    }
-
-    /**
-     * Returns the security permissions required to connect to the proxy, or
-     * null if none is required or applicable.
-     */
-    static URLPermission permissionForProxy(Proxy proxy) {
-        InetSocketAddress proxyAddress = (InetSocketAddress)proxy.address();
-
-        StringBuilder sb = new StringBuilder();
-        sb.append("socket://")
-          .append(proxyAddress.getHostString()).append(":")
-          .append(proxyAddress.getPort());
-        String urlString = sb.toString();
-        return new URLPermission(urlString, "CONNECT");
-    }
-
     /**
      * Returns the proxy for the given URI when sent through the given client,
-     * or null if none is required or applicable.
+     * or {@code null} if none is required or applicable.
      */
-    static Proxy proxyFor(HttpClient client, URI uri) {
+    private static Proxy proxyFor(HttpClient client, URI uri) {
         Optional<ProxySelector> optional = client.proxy();
-        if (!optional.isPresent())
+        if (!optional.isPresent()) {
             return null;
-
-        uri = OpeningHandshake.createRequestURI(uri);  // based on the HTTP scheme
-        List<Proxy> pl = optional.get().select(uri);
-        if (pl.size() < 1)
+        }
+        URI requestURI = OpeningHandshake.createRequestURI(uri);  // based on the HTTP scheme
+        List<Proxy> pl = optional.get().select(requestURI);
+        if (pl.isEmpty()) {
             return null;
-
+        }
         Proxy proxy = pl.get(0);
-        if (!proxy.type().equals(Proxy.Type.HTTP))
+        if (proxy.type() != Proxy.Type.HTTP) {
             return null;
-
+        }
         return proxy;
     }
 
@@ -180,17 +147,23 @@
      */
     static void checkPermissions(BuilderImpl b, Proxy proxy) {
         SecurityManager sm = System.getSecurityManager();
-        if (sm != null) {
-            sm.checkPermission(permissionForServer(b.getUri(), b.getHeaders()));
-            if (proxy != null) {
-                URLPermission perm = permissionForProxy(proxy);
-                if (perm != null)
-                    sm.checkPermission(perm);
-            }
+        if (sm == null) {
+            return;
+        }
+        URLPermission perm1 = Utils.permissionForServer(
+                b.getUri(), "", b.getHeaders().stream().map(p -> p.first).distinct());
+        sm.checkPermission(perm1);
+        if (proxy == null) {
+            return;
+        }
+        URLPermission perm2 = permissionForProxy((InetSocketAddress) proxy.address());
+        if (perm2 != null) {
+            sm.checkPermission(perm2);
         }
     }
 
     static CompletableFuture<WebSocket> newInstanceAsync(BuilderImpl b) {
+        // TODO: a security issue? TOCTOU: two accesses to b.getURI
         Proxy proxy = proxyFor(b.getClient(), b.getUri());
         try {
             checkPermissions(b, proxy);