# HG changeset patch
# User jboes
# Date 1573211716 0
# Node ID 3786a0962570d8fd7bb8e30671a34b49a442c2a5
# Parent  4dbdb7a8fa758c210467d8524d50c9c48cfb9fb5
8232853: AuthenticationFilter.Cache::remove may throw ConcurrentModificationException
Summary: Change implementation to use iterator instead of plain LinkedList
Reviewed-by: dfuchs, vtewari

diff -r 4dbdb7a8fa75 -r 3786a0962570 src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java
--- a/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java	Tue Nov 12 12:19:24 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/AuthenticationFilter.java	Fri Nov 08 11:15:16 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -32,7 +32,6 @@
 import java.net.InetSocketAddress;
 import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.charset.Charset;
 import java.util.Base64;
 import java.util.LinkedList;
 import java.util.List;
@@ -380,10 +379,18 @@
             return null;
         }
 
+        private static boolean equalsIgnoreCase(String s1, String s2) {
+            return s1 == s2 || (s1 != null && s1.equalsIgnoreCase(s2));
+        }
+
         synchronized void remove(String authscheme, URI domain, boolean proxy) {
-            for (CacheEntry entry : entries) {
-                if (entry.equalsKey(domain, proxy)) {
-                    entries.remove(entry);
+            var iterator = entries.iterator();
+            while (iterator.hasNext()) {
+                var entry = iterator.next();
+                if (equalsIgnoreCase(entry.scheme, authscheme)) {
+                    if (entry.equalsKey(domain, proxy)) {
+                        iterator.remove();
+                    }
                 }
             }
         }
diff -r 4dbdb7a8fa75 -r 3786a0962570 test/jdk/java/net/httpclient/AuthFilterCacheTest.java
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/net/httpclient/AuthFilterCacheTest.java	Fri Nov 08 11:15:16 2019 +0000
@@ -0,0 +1,298 @@
+/*
+ * Copyright (c) 2019, 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 java.io.IOException;
+import java.net.*;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicLong;
+
+import com.sun.net.httpserver.HttpServer;
+import com.sun.net.httpserver.HttpsConfigurator;
+import com.sun.net.httpserver.HttpsServer;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import javax.net.ssl.SSLContext;
+
+/**
+ * @test
+ * @bug 8232853
+ * @summary AuthenticationFilter.Cache::remove may throw ConcurrentModificationException
+ * @library /test/lib http2/server
+ * @build jdk.test.lib.net.SimpleSSLContext HttpServerAdapters DigestEchoServer HttpRedirectTest
+ * @modules java.net.http/jdk.internal.net.http.common
+ * java.net.http/jdk.internal.net.http.frame
+ * java.net.http/jdk.internal.net.http.hpack
+ * java.logging
+ * java.base/sun.net.www.http
+ * java.base/sun.net.www
+ * java.base/sun.net
+ * @run testng/othervm -Dtest.requiresHost=true
+ * -Djdk.httpclient.HttpClient.log=headers
+ * -Djdk.internal.httpclient.debug=false
+ * AuthFilterCacheTest
+ */
+
+public class AuthFilterCacheTest implements HttpServerAdapters {
+
+    static final String RESPONSE_BODY = "Hello World!";
+    static final int REQUEST_COUNT = 5;
+    static final int URI_COUNT = 6;
+    static final CyclicBarrier barrier = new CyclicBarrier(REQUEST_COUNT * URI_COUNT);
+    static final SSLContext context;
+
+    static {
+        try {
+            context = new jdk.test.lib.net.SimpleSSLContext().get();
+            SSLContext.setDefault(context);
+        } catch (Exception x) {
+            throw new ExceptionInInitializerError(x);
+        }
+    }
+
+    HttpTestServer http1Server;
+    HttpTestServer http2Server;
+    HttpTestServer https1Server;
+    HttpTestServer https2Server;
+    DigestEchoServer.TunnelingProxy proxy;
+    URI http1URI;
+    URI https1URI;
+    URI http2URI;
+    URI https2URI;
+    InetSocketAddress proxyAddress;
+    ProxySelector proxySelector;
+    MyAuthenticator auth;
+    HttpClient client;
+    Executor executor = Executors.newCachedThreadPool();
+
+    @DataProvider(name = "uris")
+    Object[][] testURIs() {
+        Object[][] uris = new Object[][]{
+                {List.of(http1URI.resolve("direct/orig/"),
+                        https1URI.resolve("direct/orig/"),
+                        https1URI.resolve("proxy/orig/"),
+                        http2URI.resolve("direct/orig/"),
+                        https2URI.resolve("direct/orig/"),
+                        https2URI.resolve("proxy/orig/"))}
+        };
+        return uris;
+    }
+
+    public HttpClient newHttpClient(ProxySelector ps, Authenticator auth) {
+        HttpClient.Builder builder = HttpClient
+                .newBuilder()
+                .sslContext(context)
+                .authenticator(auth)
+                .proxy(ps);
+        return builder.build();
+    }
+
+    @BeforeClass
+    public void setUp() throws Exception {
+        try {
+            InetSocketAddress sa =
+                    new InetSocketAddress(InetAddress.getLoopbackAddress(), 0);
+            auth = new MyAuthenticator();
+
+            // HTTP/1.1
+            HttpServer server1 = HttpServer.create(sa, 0);
+            server1.setExecutor(executor);
+            http1Server = HttpTestServer.of(server1);
+            http1Server.addHandler(new TestHandler(), "/AuthFilterCacheTest/http1/");
+            http1Server.start();
+            http1URI = new URI("http://" + http1Server.serverAuthority()
+                    + "/AuthFilterCacheTest/http1/");
+
+            // HTTPS/1.1
+            HttpsServer sserver1 = HttpsServer.create(sa, 100);
+            sserver1.setExecutor(executor);
+            sserver1.setHttpsConfigurator(new HttpsConfigurator(context));
+            https1Server = HttpTestServer.of(sserver1);
+            https1Server.addHandler(new TestHandler(), "/AuthFilterCacheTest/https1/");
+            https1Server.start();
+            https1URI = new URI("https://" + https1Server.serverAuthority()
+                    + "/AuthFilterCacheTest/https1/");
+
+            // HTTP/2.0
+            http2Server = HttpTestServer.of(
+                    new Http2TestServer("localhost", false, 0));
+            http2Server.addHandler(new TestHandler(), "/AuthFilterCacheTest/http2/");
+            http2Server.start();
+            http2URI = new URI("http://" + http2Server.serverAuthority()
+                    + "/AuthFilterCacheTest/http2/");
+
+            // HTTPS/2.0
+            https2Server = HttpTestServer.of(
+                    new Http2TestServer("localhost", true, 0));
+            https2Server.addHandler(new TestHandler(), "/AuthFilterCacheTest/https2/");
+            https2Server.start();
+            https2URI = new URI("https://" + https2Server.serverAuthority()
+                    + "/AuthFilterCacheTest/https2/");
+
+            proxy = DigestEchoServer.createHttpsProxyTunnel(
+                    DigestEchoServer.HttpAuthSchemeType.NONE);
+            proxyAddress = proxy.getProxyAddress();
+            proxySelector = new HttpProxySelector(proxyAddress);
+            client = newHttpClient(proxySelector, auth);
+
+            System.out.println("Setup: done");
+        } catch (Exception x) {
+            tearDown();
+            throw x;
+        } catch (Error e) {
+            tearDown();
+            throw e;
+        }
+    }
+
+    @AfterClass
+    public void tearDown() {
+        proxy = stop(proxy, DigestEchoServer.TunnelingProxy::stop);
+        http1Server = stop(http1Server, HttpTestServer::stop);
+        https1Server = stop(https1Server, HttpTestServer::stop);
+        http2Server = stop(http2Server, HttpTestServer::stop);
+        https2Server = stop(https2Server, HttpTestServer::stop);
+        client = null;
+
+        System.out.println("Teardown: done");
+    }
+
+    private interface Stoppable<T> {
+        void stop(T service) throws Exception;
+    }
+
+    static <T> T stop(T service, Stoppable<T> stop) {
+        try {
+            if (service != null) stop.stop(service);
+        } catch (Throwable x) {
+        }
+        return null;
+    }
+
+    static class HttpProxySelector extends ProxySelector {
+        private static final List<Proxy> NO_PROXY = List.of(Proxy.NO_PROXY);
+        private final List<Proxy> proxyList;
+
+        HttpProxySelector(InetSocketAddress proxyAddress) {
+            proxyList = List.of(new Proxy(Proxy.Type.HTTP, proxyAddress));
+        }
+
+        @Override
+        public List<Proxy> select(URI uri) {
+            // Our proxy only supports tunneling
+            if (uri.getScheme().equalsIgnoreCase("https")) {
+                if (uri.getPath().contains("/proxy/")) {
+                    return proxyList;
+                }
+            }
+            return NO_PROXY;
+        }
+
+        @Override
+        public void connectFailed(URI uri, SocketAddress sa, IOException ioe) {
+            System.err.println("Connection to proxy failed: " + ioe);
+            System.err.println("Proxy: " + sa);
+            System.err.println("\tURI: " + uri);
+            ioe.printStackTrace();
+        }
+    }
+
+    public static class TestHandler implements HttpTestHandler {
+        static final AtomicLong respCounter = new AtomicLong();
+
+        @Override
+        public void handle(HttpTestExchange t) throws IOException {
+            var count = respCounter.incrementAndGet();
+            System.out.println("Responses handled: " + count);
+            t.getRequestBody().readAllBytes();
+
+            if (t.getRequestMethod().equalsIgnoreCase("GET")) {
+                if (!t.getRequestHeaders().containsKey("Authorization")) {
+                    t.getResponseHeaders()
+                            .addHeader("WWW-Authenticate", "Basic realm=\"Earth\"");
+                    t.sendResponseHeaders(401, 0);
+                } else {
+                    byte[] resp = RESPONSE_BODY.getBytes(StandardCharsets.UTF_8);
+                    t.sendResponseHeaders(200, resp.length);
+                    try {
+                        barrier.await();
+                    } catch (Exception e) {
+                        throw new IOException(e);
+                    }
+                    t.getResponseBody().write(resp);
+                }
+            }
+            t.close();
+        }
+    }
+
+    void doClient(List<URI> uris) {
+        assert uris.size() == URI_COUNT;
+        barrier.reset();
+        System.out.println("Client opening connection to: " + uris.toString());
+
+        List<CompletableFuture<HttpResponse<String>>> cfs = new ArrayList<>();
+
+        for (int i = 0; i < REQUEST_COUNT; i++) {
+            for (URI uri : uris) {
+                HttpRequest req = HttpRequest.newBuilder()
+                        .uri(uri)
+                        .build();
+                cfs.add(client.sendAsync(req, HttpResponse.BodyHandlers.ofString()));
+            }
+        }
+        CompletableFuture.allOf(cfs.toArray(new CompletableFuture[0])).join();
+    }
+
+    static class MyAuthenticator extends Authenticator {
+        private int count = 0;
+
+        MyAuthenticator() {
+            super();
+        }
+
+        public PasswordAuthentication getPasswordAuthentication() {
+            System.out.println("Authenticator called: " + ++count);
+            return (new PasswordAuthentication("user" + count,
+                    ("passwordNotCheckedAnyway" + count).toCharArray()));
+        }
+
+        public int getCount() {
+            return count;
+        }
+    }
+
+    @Test(dataProvider = "uris")
+    public void test(List<URI> uris) throws Exception {
+        System.out.println("Server listening at " + uris.toString());
+        doClient(uris);
+    }
+}