# HG changeset patch # User chegar # Date 1517320360 0 # Node ID a8423a38386e180de8fd25926bc11cee85c76474 # Parent 08e8e41841cf8f01197bfcfe56cbf4941f97e5f0 http-client-branch: fix issue in SSL flow delegate when handshake error occurs during unwrap diff -r 08e8e41841cf -r a8423a38386e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java Mon Jan 29 18:12:36 2018 +0000 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java Tue Jan 30 13:52:40 2018 +0000 @@ -45,6 +45,7 @@ import java.util.function.Function; import java.util.function.Supplier; import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; import jdk.incubator.http.HttpConnection.HttpPublisher; import jdk.incubator.http.internal.common.FlowTube; import jdk.incubator.http.internal.common.FlowTube.TubeSubscriber; @@ -395,7 +396,15 @@ return cf; }; - return aconn.getALPN().thenCompose(checkAlpnCF); + return aconn.getALPN() + .whenComplete((r,t) -> { + if (t != null && t instanceof SSLException) { + // something went wrong during the initial handshake + // close the connection + aconn.close(); + } + }) + .thenCompose(checkAlpnCF); } synchronized boolean singleStream() { diff -r 08e8e41841cf -r a8423a38386e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SSLFlowDelegate.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SSLFlowDelegate.java Mon Jan 29 18:12:36 2018 +0000 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SSLFlowDelegate.java Tue Jan 30 13:52:40 2018 +0000 @@ -578,6 +578,7 @@ writer.addData(HS_TRIGGER); } } catch (Throwable ex) { + errorCommon(ex); handleError(ex); } } @@ -707,23 +708,23 @@ private void executeTasks(List tasks) { exec.execute(() -> { - handshakeState.getAndUpdate((current) -> current | DOING_TASKS); - List nextTasks = tasks; - do { - try { - nextTasks.forEach((r) -> { - r.run(); - }); + try { + handshakeState.getAndUpdate((current) -> current | DOING_TASKS); + List nextTasks = tasks; + do { + nextTasks.forEach(Runnable::run); if (engine.getHandshakeStatus() == HandshakeStatus.NEED_TASK) { nextTasks = obtainTasks(); - } else break; - } catch (Throwable t) { - handleError(t); - } - } while(true); - handshakeState.getAndUpdate((current) -> current & ~DOING_TASKS); - writer.addData(HS_TRIGGER); - resumeActivity(); + } else { + break; + } + } while (true); + handshakeState.getAndUpdate((current) -> current & ~DOING_TASKS); + writer.addData(HS_TRIGGER); + resumeActivity(); + } catch (Throwable t) { + handleError(t); + } }); } diff -r 08e8e41841cf -r a8423a38386e src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SubscriberWrapper.java --- a/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SubscriberWrapper.java Mon Jan 29 18:12:36 2018 +0000 +++ b/src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SubscriberWrapper.java Tue Jan 30 13:52:40 2018 +0000 @@ -367,7 +367,8 @@ } protected boolean errorCommon(Throwable throwable) { - assert throwable != null; + assert throwable != null || + (throwable = new AssertionError("null throwable")) != null; if (errorRef.compareAndSet(null, throwable)) { logger.log(Level.DEBUG, "error", throwable); pushScheduler.runOrSchedule(); diff -r 08e8e41841cf -r a8423a38386e test/jdk/java/net/httpclient/InvalidSSLContextTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/net/httpclient/InvalidSSLContextTest.java Tue Jan 30 13:52:40 2018 +0000 @@ -0,0 +1,182 @@ +/* + * Copyright (c) 2018, 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. + */ + +/* + * @test + * @summary Test to ensure the HTTP client throws an appropriate SSL exception + * when SSL context is not valid. + * @library /lib/testlibrary + * @build jdk.testlibrary.SimpleSSLContext + * @run testng/othervm -Djdk.internal.httpclient.debug=true InvalidSSLContextTest + */ + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; +import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; +import jdk.incubator.http.HttpClient; +import jdk.incubator.http.HttpClient.Version; +import jdk.incubator.http.HttpRequest; +import jdk.incubator.http.HttpResponse; +import jdk.incubator.http.HttpResponse.BodyHandler; +import jdk.testlibrary.SimpleSSLContext; +import org.testng.Assert; +import org.testng.annotations.AfterTest; +import org.testng.annotations.BeforeTest; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import static jdk.incubator.http.HttpClient.Version.HTTP_1_1; +import static jdk.incubator.http.HttpClient.Version.HTTP_2; + + +public class InvalidSSLContextTest { + + SSLContext sslContext; + volatile SSLServerSocket sslServerSocket; + volatile String uri; + + @DataProvider(name = "versions") + public Object[][] versions() { + return new Object[][]{ + { HTTP_1_1 }, + { HTTP_2 } + }; + } + + @Test(dataProvider = "versions") + public void testSync(Version version) throws Exception { + // client-side uses a different context to that of the server-side + HttpClient client = HttpClient.newBuilder() + .sslContext(SSLContext.getDefault()) + .build(); + + HttpRequest request = HttpRequest.newBuilder(URI.create(uri)) + .version(version) + .build(); + + try { + HttpResponse response = client.send(request, BodyHandler.discard("")); + Assert.fail("UNEXPECTED response" + response); + } catch (SSLException sslex) { + System.out.println("Caught expected: " + sslex); + } + } + + @Test(dataProvider = "versions") + public void testAsync(Version version) throws Exception { + // client-side uses a different context to that of the server-side + HttpClient client = HttpClient.newBuilder() + .sslContext(SSLContext.getDefault()) + .build(); + + HttpRequest request = HttpRequest.newBuilder(URI.create(uri)) + .version(version) + .build(); + + assertExceptionally(SSLException.class, + client.sendAsync(request, BodyHandler.discard(""))); + } + + static void assertExceptionally(Class clazz, + CompletableFuture stage) { + stage.handle((result, error) -> { + if (result != null) { + Assert.fail("UNEXPECTED result: " + result); + return null; + } + if (error instanceof CompletionException) { + Throwable cause = error.getCause(); + if (cause == null) { + Assert.fail("Unexpected null cause: " + error); + } + assertException(clazz, cause); + } else { + assertException(clazz, error); + } + return null; + }).join(); + } + + static void assertException(Class clazz, Throwable t) { + if (t == null) { + Assert.fail("Expected " + clazz + ", caught nothing"); + } + if (!clazz.isInstance(t)) { + Assert.fail("Expected " + clazz + ", caught " + t); + } + } + + @BeforeTest + public void setup() throws Exception { + sslContext = new SimpleSSLContext().get(); + if (sslContext == null) + throw new AssertionError("Unexpected null sslContext"); + + // server-side uses a different context to that of the client-side + sslServerSocket = (SSLServerSocket)sslContext + .getServerSocketFactory() + .createServerSocket(0); + uri = "https://localhost:" + sslServerSocket.getLocalPort() + "/"; + + Thread t = new Thread("SSL-Server-Side") { + @Override + public void run() { + while (true) { + try { + SSLSocket s = (SSLSocket) sslServerSocket.accept(); + System.out.println("SERVER: accepted: " + s); + // artificially slow down the handshake reply to mimic + // a slow(ish) network, and hopefully delay the + // SequentialScheduler on in the client. + Thread.sleep(500); + s.startHandshake(); + s.close(); + Assert.fail("SERVER: UNEXPECTED "); + } catch (SSLHandshakeException he) { + System.out.println("SERVER: caught expected " + he); + } catch (IOException e) { + System.out.println("SERVER: caught: " + e); + if (!sslServerSocket.isClosed()) { + throw new UncheckedIOException(e); + } + break; + } catch (InterruptedException ie) { + throw new RuntimeException(ie); + } + } + } + }; + t.start(); + } + + @AfterTest + public void teardown() throws Exception { + sslServerSocket.close(); + } +}