Skip to content

Commit 63f8fc8

Browse files
clivevergheseXueleiFan
cliveverghese
authored and
XueleiFan
committed
8259662: Don't wrap SocketExceptions into SSLExceptions in SSLSocketImpl
Reviewed-by: xuelei
1 parent cf0019d commit 63f8fc8

7 files changed

Lines changed: 199 additions & 22 deletions

File tree

‎src/java.base/share/classes/sun/security/ssl/SSLSocketImpl.java‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@
7474
public final class SSLSocketImpl
7575
extends BaseSSLSocketImpl implements SSLTransport {
7676

77+
/**
78+
* ERROR HANDLING GUIDELINES
79+
* (which exceptions to throw and catch and which not to throw and catch)
80+
*
81+
* - if there is an IOException (SocketException) when accessing the
82+
* underlying Socket, pass it through
83+
*
84+
* - do not throw IOExceptions, throw SSLExceptions (or a subclass)
85+
*/
86+
7787
final SSLContextImpl sslContext;
7888
final TransportContext conContext;
7989

@@ -446,6 +456,8 @@ private void startHandshake(boolean resumable) throws IOException {
446456
throw conContext.fatal(Alert.HANDSHAKE_FAILURE,
447457
"Couldn't kickstart handshaking", iioe);
448458
}
459+
} catch (SocketException se) {
460+
handleException(se);
449461
} catch (IOException ioe) {
450462
throw conContext.fatal(Alert.HANDSHAKE_FAILURE,
451463
"Couldn't kickstart handshaking", ioe);
@@ -1405,9 +1417,9 @@ private int readHandshakeRecord() throws IOException {
14051417
conContext.isNegotiated) {
14061418
return 0;
14071419
}
1408-
} catch (SSLException | InterruptedIOException ssle) {
1409-
// don't change exception in case of timeouts or interrupts
1410-
throw ssle;
1420+
} catch (SSLException | InterruptedIOException | SocketException se) {
1421+
// don't change exception in case of timeouts or interrupts or SocketException
1422+
throw se;
14111423
} catch (IOException ioe) {
14121424
throw new SSLException("readHandshakeRecord", ioe);
14131425
}
@@ -1468,9 +1480,9 @@ private ByteBuffer readApplicationRecord(
14681480
buffer.position() > 0) {
14691481
return buffer;
14701482
}
1471-
} catch (SSLException | InterruptedIOException ssle) {
1472-
// don't change exception in case of timeouts or interrupts
1473-
throw ssle;
1483+
} catch (SSLException | InterruptedIOException | SocketException se) {
1484+
// don't change exception in case of timeouts or interrupts or SocketException.
1485+
throw se;
14741486
} catch (IOException ioe) {
14751487
throw new SSLException("readApplicationRecord", ioe);
14761488
}
@@ -1678,6 +1690,16 @@ private void handleException(Exception cause) throws IOException {
16781690
}
16791691
}
16801692

1693+
if (cause instanceof SocketException) {
1694+
try {
1695+
conContext.fatal(alert, cause);
1696+
} catch (Exception e) {
1697+
// Just delivering the fatal alert, re-throw the socket exception instead.
1698+
}
1699+
1700+
throw (SocketException)cause;
1701+
}
1702+
16811703
throw conContext.fatal(alert, cause);
16821704
}
16831705

‎src/java.base/share/classes/sun/security/ssl/SSLTransport.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.io.EOFException;
2929
import java.io.IOException;
3030
import java.io.InterruptedIOException;
31+
import java.net.SocketException;
3132
import java.nio.ByteBuffer;
3233
import javax.crypto.AEADBadTagException;
3334
import javax.crypto.BadPaddingException;
@@ -137,9 +138,9 @@ static Plaintext decode(TransportContext context,
137138
} catch (EOFException eofe) {
138139
// rethrow EOFException, the call will handle it if neede.
139140
throw eofe;
140-
} catch (InterruptedIOException iioe) {
141-
// don't close the Socket in case of timeouts or interrupts.
142-
throw iioe;
141+
} catch (InterruptedIOException | SocketException se) {
142+
// don't close the Socket in case of timeouts or interrupts or SocketException.
143+
throw se;
143144
} catch (IOException ioe) {
144145
throw context.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
145146
}

‎test/jdk/java/net/httpclient/InvalidSSLContextTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -37,6 +37,7 @@
3737
import java.net.URI;
3838
import java.util.concurrent.CompletableFuture;
3939
import java.util.concurrent.CompletionException;
40+
import java.net.SocketException;
4041
import javax.net.ssl.SSLContext;
4142
import javax.net.ssl.SSLException;
4243
import javax.net.ssl.SSLHandshakeException;
@@ -173,8 +174,8 @@ public void run() {
173174
s.startHandshake();
174175
s.close();
175176
Assert.fail("SERVER: UNEXPECTED ");
176-
} catch (SSLException he) {
177-
System.out.println("SERVER: caught expected " + he);
177+
} catch (SSLException | SocketException se) {
178+
System.out.println("SERVER: caught expected " + se);
178179
} catch (IOException e) {
179180
System.out.println("SERVER: caught: " + e);
180181
if (!sslServerSocket.isClosed()) {

‎test/jdk/javax/net/ssl/SSLSession/TestEnabledProtocols.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,10 @@ protected void runServerApplication(SSLSocket socket) throws Exception {
8989
se.printStackTrace(System.out);
9090
} catch (InterruptedIOException ioe) {
9191
// must have been interrupted, no harm
92-
} catch (SSLException ssle) {
92+
} catch (SSLException | SocketException se) {
9393
// The client side may have closed the socket.
9494
System.out.println("Server SSLException:");
95-
ssle.printStackTrace(System.out);
95+
se.printStackTrace(System.out);
9696
} catch (Exception e) {
9797
System.out.println("Server exception:");
9898
e.printStackTrace(System.out);

‎test/jdk/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -131,9 +131,9 @@ protected void runServerApplication(SSLSocket socket) throws Exception {
131131
sslIS.read();
132132
sslOS.write('A');
133133
sslOS.flush();
134-
} catch (SSLException ssle) {
134+
} catch (SSLException | SocketException se) {
135135
if (!expectFail) {
136-
throw ssle;
136+
throw se;
137137
} // Otherwise, ignore.
138138
}
139139
}

‎test/jdk/sun/security/ssl/SSLSocketImpl/SSLSocketShouldThrowSocketException.java‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
/*
2+
* Copyright (c) 2021, Amazon and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8214339 8259662
27+
* @summary When a SocketException is thrown by the underlying layer, It
28+
* should be thrown as is and not be transformed to an SSLException.
29+
* @library /javax/net/ssl/templates
30+
* @run main/othervm SSLSocketShouldThrowSocketException
31+
*/
32+
33+
import java.io.*;
34+
import java.net.*;
35+
import java.util.*;
36+
import java.security.*;
37+
import javax.net.ssl.*;
38+
39+
import java.util.concurrent.CountDownLatch;
40+
import java.util.concurrent.TimeUnit;
41+
42+
public class SSLSocketShouldThrowSocketException extends SSLSocketTemplate {
43+
44+
boolean handshake;
45+
46+
private final CountDownLatch clientTerminatedCondition = new CountDownLatch(1);
47+
48+
SSLSocketShouldThrowSocketException(boolean handshake) {
49+
this.handshake = handshake;
50+
}
51+
52+
@Override
53+
protected boolean isCustomizedClientConnection() {
54+
return true;
55+
}
56+
57+
@Override
58+
protected void runServerApplication(SSLSocket socket) throws Exception {
59+
clientTerminatedCondition.await(30L, TimeUnit.SECONDS);
60+
}
61+
62+
@Override
63+
protected void runClientApplication(int serverPort) throws Exception {
64+
Socket baseSocket = new Socket("localhost", serverPort);
65+
66+
SSLSocketFactory sslsf =
67+
(SSLSocketFactory) SSLSocketFactory.getDefault();
68+
SSLSocket sslSocket = (SSLSocket)
69+
sslsf.createSocket(baseSocket, "localhost", serverPort, false);
70+
71+
if (this.handshake) {
72+
testHandshakeClose(baseSocket, sslSocket);
73+
} else {
74+
testDataClose(baseSocket, sslSocket);
75+
}
76+
77+
clientTerminatedCondition.countDown();
78+
79+
}
80+
81+
private void testHandshakeClose(Socket baseSocket, SSLSocket sslSocket) throws Exception {
82+
Thread aborter = new Thread() {
83+
@Override
84+
public void run() {
85+
86+
try {
87+
Thread.sleep(10);
88+
System.err.println("Closing the client socket : " + System.nanoTime());
89+
baseSocket.close();
90+
} catch (Exception ieo) {
91+
ieo.printStackTrace();
92+
}
93+
}
94+
};
95+
96+
aborter.start();
97+
98+
try {
99+
// handshaking
100+
System.err.println("Client starting handshake: " + System.nanoTime());
101+
sslSocket.startHandshake();
102+
throw new Exception("Start handshake did not throw an exception");
103+
} catch (SocketException se) {
104+
System.err.println("Caught Expected SocketException");
105+
}
106+
107+
aborter.join();
108+
}
109+
110+
private void testDataClose(Socket baseSocket, SSLSocket sslSocket) throws Exception{
111+
112+
CountDownLatch handshakeCondition = new CountDownLatch(1);
113+
114+
Thread aborter = new Thread() {
115+
@Override
116+
public void run() {
117+
118+
try {
119+
handshakeCondition.await(10L, TimeUnit.SECONDS);
120+
System.err.println("Closing the client socket : " + System.nanoTime());
121+
baseSocket.close();
122+
} catch (Exception ieo) {
123+
ieo.printStackTrace();
124+
}
125+
}
126+
};
127+
128+
aborter.start();
129+
130+
try {
131+
// handshaking
132+
System.err.println("Client starting handshake: " + System.nanoTime());
133+
sslSocket.startHandshake();
134+
handshakeCondition.countDown();
135+
System.err.println("Reading data from server");
136+
BufferedReader is = new BufferedReader(
137+
new InputStreamReader(sslSocket.getInputStream()));
138+
String data = is.readLine();
139+
throw new Exception("Start handshake did not throw an exception");
140+
} catch (SocketException se) {
141+
System.err.println("Caught Expected SocketException");
142+
}
143+
144+
aborter.join();
145+
}
146+
147+
public static void main(String[] args) throws Exception {
148+
// SocketException should be throws during a handshake phase.
149+
(new SSLSocketShouldThrowSocketException(true)).run();
150+
// SocketException should be throw during the application data phase.
151+
(new SSLSocketShouldThrowSocketException(false)).run();
152+
}
153+
}

test/jdk/sun/security/ssl/SSLSocketImpl/SSLExceptionForIOIssue.java renamed to test/jdk/sun/security/ssl/SSLSocketImpl/SocketExceptionForSocketIssues.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,18 @@
3131
* @bug 8214339
3232
* @summary SSLSocketImpl erroneously wraps SocketException
3333
* @library /javax/net/ssl/templates
34-
* @run main/othervm SSLExceptionForIOIssue
34+
* @run main/othervm SocketExceptionForSocketIssues
3535
*/
3636

3737
import javax.net.ssl.*;
3838
import java.io.*;
3939
import java.net.*;
4040

41-
public class SSLExceptionForIOIssue implements SSLContextTemplate {
41+
public class SocketExceptionForSocketIssues implements SSLContextTemplate {
4242

4343
public static void main(String[] args) throws Exception {
4444
System.err.println("===================================");
45-
new SSLExceptionForIOIssue().test();
45+
new SocketExceptionForSocketIssues().test();
4646
}
4747

4848
private void test() throws Exception {
@@ -79,9 +79,9 @@ private void test() throws Exception {
7979
os.flush();
8080
} catch (SSLProtocolException | SSLHandshakeException sslhe) {
8181
throw sslhe;
82-
} catch (SSLException ssle) {
82+
} catch (SocketException se) {
8383
// the expected exception, ignore it
84-
System.err.println("server exception: " + ssle);
84+
System.err.println("server exception: " + se);
8585
} finally {
8686
if (listenSocket != null) {
8787
listenSocket.close();

0 commit comments

Comments
 (0)