8216498: Confusing and unneeded wrapping of SSLHandshakeException
Summary: [httpclient] Avoid wrapping SSLHandshakeException in plain IOException
Reviewed-by: chegar
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java Fri Jan 11 23:32:52 2019 +0900
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java Fri Jan 11 14:48:19 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
@@ -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 @@
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 {
--- a/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java Fri Jan 11 23:32:52 2019 +0900
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java Fri Jan 11 14:48:19 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
@@ -31,11 +31,11 @@
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.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 @@
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 {
--- a/test/jdk/java/net/httpclient/ShortResponseBody.java Fri Jan 11 23:32:52 2019 +0900
+++ b/test/jdk/java/net/httpclient/ShortResponseBody.java Fri Jan 11 14:48:19 2019 +0000
@@ -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.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 @@
fail("UNEXPECTED RESPONSE: " + response);
} catch (IOException ioe) {
out.println("Caught expected exception:" + ioe);
- String msg = ioe.getMessage();
- assertTrue(msg.contains(expectedMsg), "exception msg:[" + msg + "]");
+ assertExpectedMessage(request, ioe, expectedMsg);
// synchronous API must have the send method on the stack
assertSendMethodOnStack(ioe);
assertNoConnectionExpiredException(ioe);
@@ -252,8 +253,7 @@
if (ee.getCause() instanceof IOException) {
IOException ioe = (IOException) ee.getCause();
out.println("Caught expected exception:" + ioe);
- String msg = ioe.getMessage();
- assertTrue(msg.contains(expectedMsg), "exception msg:[" + msg + "]");
+ assertExpectedMessage(request, ioe, expectedMsg);
assertNoConnectionExpiredException(ioe);
} else {
throw ee;
@@ -335,15 +335,13 @@
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),
- "exception msg:[" + msg + "], not in [" + expectedMessages);
+ assertExpectedMessage(request, ioe, expectedMessages);
// synchronous API must have the send method on the stack
assertSendMethodOnStack(ioe);
assertNoConnectionExpiredException(ioe);
@@ -379,8 +377,7 @@
MSGS_ORDER.stream().takeWhile(s -> !s.equals(expectedMsg))
.forEach(expectedMessages::add);
- assertTrue(expectedMessages.stream().anyMatch(s -> msg.indexOf(s) != -1),
- "exception msg:[" + msg + "], not in [" + expectedMessages);
+ assertExpectedMessage(request, ioe, expectedMessages);
assertNoConnectionExpiredException(ioe);
} else {
throw ee;
@@ -389,6 +386,31 @@
}
}
+
+ 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) {