8216498: Confusing and unneeded wrapping of SSLHandshakeException

[httpclient] Avoid wrapping SSLHandshakeException in plain IOException

Reviewed-by: chegar

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

@@ -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
@@ -26,6 +26,8 @@
 package jdk.internal.net.http;
 
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLParameters;
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -561,6 +563,15 @@ final class HttpClientImpl extends HttpClient implements Trackable {
                 ConnectException ce = new ConnectException(msg);
                 ce.initCause(throwable);
                 throw ce;
+            } else if (throwable instanceof SSLHandshakeException) {
+                // special case for SSLHandshakeException
+                SSLHandshakeException he = new SSLHandshakeException(msg);
+                he.initCause(throwable);
+                throw he;
+            } else if (throwable instanceof SSLException) {
+                // any other SSLException is wrapped in a plain
+                // SSLException
+                throw new SSLException(msg, throwable);
             } else if (throwable instanceof IOException) {
                 throw new IOException(msg, throwable);
             } else {

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

@@ -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
@@ -31,11 +31,11 @@ import sun.net.www.HeaderParser;
 
 import javax.net.ssl.ExtendedSSLSession;
 import javax.net.ssl.SSLException;
+import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSession;
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
-import java.io.EOFException;
 import java.io.IOException;
 import java.io.PrintStream;
 import java.io.UncheckedIOException;
@@ -74,7 +74,6 @@ import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.lang.String.format;
-import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.joining;
 import jdk.internal.net.http.HttpRequestImpl;
 
@@ -307,11 +306,16 @@ public final class Utils {
         if (!(t instanceof IOException))
             return t;
 
+        if (t instanceof SSLHandshakeException)
+            return t;  // no need to decorate
+
         String msg = messageSupplier.get();
         if (msg == null)
             return t;
 
         if (t instanceof ConnectionExpiredException) {
+            if (t.getCause() instanceof SSLHandshakeException)
+                return t;  // no need to decorate
             IOException ioe = new IOException(msg, t.getCause());
             t = new ConnectionExpiredException(ioe);
         } else {

test/jdk/java/net/httpclient/ShortResponseBody.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 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
@@ -23,6 +23,7 @@
 
 /*
  * @test
+ * @bug 8216498
  * @summary Tests Exception detail message when too few response bytes are
  *          received before a socket exception or eof.
  * @library /test/lib
@@ -61,6 +62,7 @@ import org.testng.annotations.BeforeTest;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLServerSocketFactory;
 import javax.net.ssl.SSLParameters;
 import javax.net.ssl.SSLSocket;
@@ -224,8 +226,7 @@ public class ShortResponseBody {
                 fail("UNEXPECTED RESPONSE: " + response);
             } catch (IOException ioe) {
                 out.println("Caught expected exception:" + ioe);
-                String msg = ioe.getMessage();
+                assertExpectedMessage(request, ioe, expectedMsg);
-                assertTrue(msg.contains(expectedMsg), "exception msg:[" + msg + "]");
                 // synchronous API must have the send method on the stack
                 assertSendMethodOnStack(ioe);
                 assertNoConnectionExpiredException(ioe);
@@ -252,8 +253,7 @@ public class ShortResponseBody {
                 if (ee.getCause() instanceof IOException) {
                     IOException ioe = (IOException) ee.getCause();
                     out.println("Caught expected exception:" + ioe);
-                    String msg = ioe.getMessage();
+                    assertExpectedMessage(request, ioe, expectedMsg);
-                    assertTrue(msg.contains(expectedMsg), "exception msg:[" + msg + "]");
                     assertNoConnectionExpiredException(ioe);
                 } else {
                     throw ee;
@@ -335,15 +335,13 @@ public class ShortResponseBody {
                 fail("UNEXPECTED RESPONSE: " + response);
             } catch (IOException ioe) {
                 out.println("Caught expected exception:" + ioe);
-                String msg = ioe.getMessage();
 
                 List<String> expectedMessages = new ArrayList<>();
                 expectedMessages.add(expectedMsg);
                 MSGS_ORDER.stream().takeWhile(s -> !s.equals(expectedMsg))
                                    .forEach(expectedMessages::add);
 
-                assertTrue(expectedMessages.stream().anyMatch(s -> msg.indexOf(s) != -1),
+                assertExpectedMessage(request, ioe, expectedMessages);
-                           "exception msg:[" + msg + "], not in [" + expectedMessages);
                 // synchronous API must have the send method on the stack
                 assertSendMethodOnStack(ioe);
                 assertNoConnectionExpiredException(ioe);
@@ -379,8 +377,7 @@ public class ShortResponseBody {
                     MSGS_ORDER.stream().takeWhile(s -> !s.equals(expectedMsg))
                             .forEach(expectedMessages::add);
 
-                    assertTrue(expectedMessages.stream().anyMatch(s -> msg.indexOf(s) != -1),
+                    assertExpectedMessage(request, ioe, expectedMessages);
-                               "exception msg:[" + msg + "], not in [" + expectedMessages);
                     assertNoConnectionExpiredException(ioe);
                 } else {
                     throw ee;
@@ -389,6 +386,31 @@ public class ShortResponseBody {
         }
     }
 
+
+    void assertExpectedMessage(HttpRequest request, Throwable t, String expected) {
+        if (request.uri().getScheme().equalsIgnoreCase("https")
+                && (t instanceof SSLHandshakeException)) {
+            // OK
+            out.println("Skipping expected " + t);
+        } else {
+            String msg = t.getMessage();
+            assertTrue(msg.contains(expected),
+                    "exception msg:[" + msg + "]");
+        }
+    }
+
+    void assertExpectedMessage(HttpRequest request, Throwable t, List<String> expected) {
+        if (request.uri().getScheme().equalsIgnoreCase("https")
+                && (t instanceof SSLHandshakeException)) {
+            // OK
+            out.println("Skipping expected " + t);
+        } else {
+            String msg = t.getMessage();
+            assertTrue(expected.stream().anyMatch(msg::contains),
+                    "exception msg:[" + msg + "] not in " + Arrays.asList(expected));
+        }
+    }
+
     // Asserts that the "send" method appears in the stack of the given
     // exception. The synchronous API must contain the send method on the stack.
     static void assertSendMethodOnStack(IOException ioe) {