8263442: Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED

Reviewed-by: dfuchs

src/java.net.http/share/classes/jdk/internal/net/http/AsyncSSLTunnelConnection.java

@@ -33,6 +33,7 @@ import java.util.function.Function;
 import jdk.internal.net.http.common.MinimalFuture;
 import jdk.internal.net.http.common.SSLTube;
 import jdk.internal.net.http.common.Utils;
+import static jdk.internal.net.http.common.Utils.ProxyHeaders;
 
 /**
  * An SSL tunnel built on a Plain (CONNECT) TCP tunnel.
@@ -47,7 +48,7 @@ class AsyncSSLTunnelConnection extends AbstractAsyncSSLConnection {
                              HttpClientImpl client,
                              String[] alpn,
                              InetSocketAddress proxy,
-                             HttpHeaders proxyHeaders)
+                             ProxyHeaders proxyHeaders)
     {
         super(addr, client, Utils.getServerName(addr), addr.getPort(), alpn);
         this.plainConnection = new PlainTunnelingConnection(addr, proxy, client, proxyHeaders);

src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java

@@ -52,6 +52,7 @@ import jdk.internal.net.http.common.SequentialScheduler.DeferredCompleter;
 import jdk.internal.net.http.common.Log;
 import jdk.internal.net.http.common.Utils;
 import static java.net.http.HttpClient.Version.HTTP_2;
+import static jdk.internal.net.http.common.Utils.ProxyHeaders;
 
 /**
  * Wraps socket channel layer and takes care of SSL also.
@@ -351,14 +352,10 @@ abstract class HttpConnection implements Closeable {
     // Composes a new immutable HttpHeaders that combines the
     // user and system header but only keeps those headers that
     // start with "proxy-"
-    private static HttpHeaders proxyTunnelHeaders(HttpRequestImpl request) {
-        Map<String, List<String>> combined = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
-        combined.putAll(request.getSystemHeadersBuilder().map());
-        combined.putAll(request.headers().map()); // let user override system
-
-        // keep only proxy-* - and also strip authorization headers
-        // for disabled schemes
-        return HttpHeaders.of(combined, Utils.PROXY_TUNNEL_FILTER);
+    private static ProxyHeaders proxyTunnelHeaders(HttpRequestImpl request) {
+        HttpHeaders userHeaders = HttpHeaders.of(request.headers().map(), Utils.PROXY_TUNNEL_FILTER);
+        HttpHeaders systemHeaders = HttpHeaders.of(request.getSystemHeadersBuilder().map(), Utils.PROXY_TUNNEL_FILTER);
+        return new ProxyHeaders(userHeaders, systemHeaders);
     }
 
     /* Returns either a plain HTTP connection or a plain tunnelling connection

src/java.net.http/share/classes/jdk/internal/net/http/HttpRequestImpl.java

@@ -48,6 +48,7 @@ import jdk.internal.net.http.websocket.OpeningHandshake;
 import jdk.internal.net.http.websocket.WebSocketRequest;
 
 import static jdk.internal.net.http.common.Utils.ALLOWED_HEADERS;
+import static jdk.internal.net.http.common.Utils.ProxyHeaders;
 
 public class HttpRequestImpl extends HttpRequest implements WebSocketRequest {
 
@@ -203,14 +204,15 @@ public class HttpRequestImpl extends HttpRequest implements WebSocketRequest {
     }
 
     /* used for creating CONNECT requests  */
-    HttpRequestImpl(String method, InetSocketAddress authority, HttpHeaders headers) {
+    HttpRequestImpl(String method, InetSocketAddress authority, ProxyHeaders headers) {
         // TODO: isWebSocket flag is not specified, but the assumption is that
         // such a request will never be made on a connection that will be returned
         // to the connection pool (we might need to revisit this constructor later)
         assert "CONNECT".equalsIgnoreCase(method);
         this.method = method;
         this.systemHeadersBuilder = new HttpHeadersBuilder();
-        this.userHeaders = headers;
+        this.systemHeadersBuilder.map().putAll(headers.systemHeaders().map());
+        this.userHeaders = headers.userHeaders();
         this.uri = URI.create("socket://" + authority.getHostString() + ":"
                               + Integer.toString(authority.getPort()) + "/");
         this.proxy = null;

src/java.net.http/share/classes/jdk/internal/net/http/PlainTunnelingConnection.java

@@ -38,6 +38,7 @@ import java.net.http.HttpHeaders;
 import jdk.internal.net.http.common.FlowTube;
 import jdk.internal.net.http.common.MinimalFuture;
 import static java.net.http.HttpResponse.BodyHandlers.discarding;
+import static jdk.internal.net.http.common.Utils.ProxyHeaders;
 
 /**
  * A plain text socket tunnel through a proxy. Uses "CONNECT" but does not
@@ -47,14 +48,14 @@ import static java.net.http.HttpResponse.BodyHandlers.discarding;
 final class PlainTunnelingConnection extends HttpConnection {
 
     final PlainHttpConnection delegate;
-    final HttpHeaders proxyHeaders;
+    final ProxyHeaders proxyHeaders;
     final InetSocketAddress proxyAddr;
     private volatile boolean connected;
 
     protected PlainTunnelingConnection(InetSocketAddress addr,
                                        InetSocketAddress proxy,
                                        HttpClientImpl client,
-                                       HttpHeaders proxyHeaders) {
+                                       ProxyHeaders proxyHeaders) {
         super(addr, client);
         this.proxyAddr = proxy;
         this.proxyHeaders = proxyHeaders;

src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java

@@ -175,10 +175,13 @@ public final class Utils {
     // used by caller.
 
     public static final BiPredicate<String, String> CONTEXT_RESTRICTED(HttpClient client) {
-        return (k, v) -> client.authenticator() == null ||
-                ! (k.equalsIgnoreCase("Authorization")
-                        && k.equalsIgnoreCase("Proxy-Authorization"));
+        return (k, v) -> !client.authenticator().isPresent() ||
+                (!k.equalsIgnoreCase("Authorization")
+                        && !k.equalsIgnoreCase("Proxy-Authorization"));
     }
+
+    public record ProxyHeaders(HttpHeaders userHeaders, HttpHeaders systemHeaders) {}
+
     private static final BiPredicate<String, String> HOST_RESTRICTED = (k,v) -> !"host".equalsIgnoreCase(k);
     public static final BiPredicate<String, String> PROXY_TUNNEL_RESTRICTED(HttpClient client)  {
         return CONTEXT_RESTRICTED(client).and(HOST_RESTRICTED);

test/jdk/java/net/httpclient/AuthFilter.java

@@ -0,0 +1,188 @@
+/*
+ * Copyright (c) 2021, 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 com.sun.net.httpserver.Headers;
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+
+import java.io.*;
+import java.net.Authenticator;
+import java.net.InetSocketAddress;
+import java.net.ProxySelector;
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.channels.*;
+import java.nio.charset.StandardCharsets;
+
+import jdk.test.lib.net.IPSupport;
+
+/**
+ * @test
+ * @bug 8263442
+ * @summary Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED
+ * @library /test/lib
+ * @run main/othervm AuthFilter
+ */
+
+public class AuthFilter {
+    static class Auth extends Authenticator {
+    }
+
+    static HttpServer createServer() throws IOException {
+        HttpServer server = HttpServer.create(new InetSocketAddress(0), 5);
+        HttpHandler handler = (HttpExchange e) -> {
+            InputStream is = e.getRequestBody();
+            is.readAllBytes();
+            is.close();
+            Headers reqh = e.getRequestHeaders();
+            if (reqh.containsKey("authorization")) {
+                e.sendResponseHeaders(500, -1);
+            } else {
+                e.sendResponseHeaders(200, -1);
+            }
+        };
+        server.createContext("/", handler);
+        return server;
+    }
+
+    public static void main(String[] args) throws Exception {
+        test(false);
+        test(true);
+    }
+
+    /**
+     *  Fake proxy. Just looks for Proxy-Authorization header
+     *  and returns error if seen. Returns 200 OK if not.
+     *  Does not actually forward the request
+     */
+    static class ProxyServer extends Thread {
+
+        final ServerSocketChannel server;
+        final int port;
+        volatile SocketChannel c;
+
+        ProxyServer() throws IOException {
+            server = ServerSocketChannel.open();
+            server.bind(new InetSocketAddress(0));
+            if (server.getLocalAddress() instanceof InetSocketAddress isa) {
+                port = isa.getPort();
+            } else {
+                port = -1;
+            }
+        }
+
+        int getPort() {
+            return port;
+        }
+
+        static String ok = "HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n";
+        static String notok1 = "HTTP/1.1 500 Internal Server Error\r\nContent-Length: 0\r\n\r\n";
+        static String notok2 = "HTTP/1.1 501 Not Implemented\r\nContent-Length: 0\r\n\r\n";
+
+        static void reply(String msg, Writer writer) throws IOException {
+            writer.write(msg);
+            writer.flush();
+        }
+
+        public void run() {
+            try {
+                c = server.accept();
+                var cs = StandardCharsets.US_ASCII;
+                LineNumberReader reader = new LineNumberReader(Channels.newReader(c, cs));
+                Writer writer = Channels.newWriter(c, cs);
+
+                String line;
+                while ((line=reader.readLine()) != null) {
+                    if (line.indexOf("Proxy-Authorization") != -1) {
+                        reply(notok1, writer);
+                        return;
+                    }
+                    if (line.equals("")) {
+                        // end of headers
+                        reply(ok, writer);
+                        return;
+                    }
+                }
+                reply(notok2, writer);
+            } catch (IOException e) {
+            }
+            try {
+                server.close();
+                c.close();
+            } catch (IOException ee) {}
+        }
+    }
+
+    private static InetSocketAddress getLoopback(int port) throws IOException {
+        if (IPSupport.hasIPv4()) {
+            return new InetSocketAddress("127.0.0.1", port);
+        } else {
+            return new InetSocketAddress("::1", port);
+        }
+    }
+
+    public static void test(boolean useProxy) throws Exception {
+        HttpServer server = createServer();
+        int port = server.getAddress().getPort();
+        ProxyServer proxy;
+
+        InetSocketAddress proxyAddr;
+        String authHdr;
+        if (useProxy) {
+            proxy = new ProxyServer();
+            proxyAddr = getLoopback(proxy.getPort());
+            proxy.start();
+            authHdr = "Proxy-Authorization";
+        } else {
+            authHdr = "Authorization";
+            proxyAddr = null;
+        }
+
+        server.start();
+
+        // proxyAddr == null => proxying disabled
+        HttpClient client = HttpClient
+                .newBuilder()
+                .authenticator(new Auth())
+                .proxy(ProxySelector.of(proxyAddr))
+                .build();
+
+
+        URI uri = new URI("http://127.0.0.1:" + Integer.toString(port));
+
+        HttpRequest request = HttpRequest.newBuilder(uri)
+                .header(authHdr, "nonsense")
+                .GET()
+                .build();
+
+        HttpResponse<String> response = client.send(request, HttpResponse.BodyHandlers.ofString());
+        int r = response.statusCode();
+        System.out.println(r);
+        server.stop(0);
+        if (r != 200)
+            throw new RuntimeException("Test failed : " + r);
+    }
+}