# 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 { + void stop(T service) throws Exception; + } + + static T stop(T service, Stoppable stop) { + try { + if (service != null) stop.stop(service); + } catch (Throwable x) { + } + return null; + } + + static class HttpProxySelector extends ProxySelector { + private static final List NO_PROXY = List.of(Proxy.NO_PROXY); + private final List proxyList; + + HttpProxySelector(InetSocketAddress proxyAddress) { + proxyList = List.of(new Proxy(Proxy.Type.HTTP, proxyAddress)); + } + + @Override + public List 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 uris) { + assert uris.size() == URI_COUNT; + barrier.reset(); + System.out.println("Client opening connection to: " + uris.toString()); + + List>> 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 uris) throws Exception { + System.out.println("Server listening at " + uris.toString()); + doClient(uris); + } +}