http-client-branch: fix issue in SSL flow delegate when handshake error occurs during unwrap http-client-branch
authorchegar
Tue, 30 Jan 2018 13:52:40 +0000
branchhttp-client-branch
changeset 56044 a8423a38386e
parent 56043 08e8e41841cf
child 56045 5c6e3b76d2ad
http-client-branch: fix issue in SSL flow delegate when handshake error occurs during unwrap
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/Http2Connection.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SSLFlowDelegate.java
src/jdk.incubator.httpclient/share/classes/jdk/incubator/http/internal/common/SubscriberWrapper.java
test/jdk/java/net/httpclient/InvalidSSLContextTest.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() {
--- 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<Runnable> tasks) {
         exec.execute(() -> {
-            handshakeState.getAndUpdate((current) -> current | DOING_TASKS);
-            List<Runnable> nextTasks = tasks;
-            do {
-                try {
-                    nextTasks.forEach((r) -> {
-                        r.run();
-                    });
+            try {
+                handshakeState.getAndUpdate((current) -> current | DOING_TASKS);
+                List<Runnable> 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);
+            }
         });
     }
 
--- 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();
--- /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<? extends Throwable> 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<? extends Throwable> 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();
+    }
+}