Revert "Throw SocketException on ERR_SSL_SYSCALL. (#430)" (#432)
We've had reports from people that depended on "normal" connection
termination (such as in HTTP where the peer sends data and then closes
the TCP connection) returning -1 from read(), and in some cases this
change causes read() to throw a SocketException in that circumstance.
Rather than causing such a fundamental change in behavior, probably
better to just adjust the test to allow for a -1 return.
This reverts commit d84f6f57b216ab53dc3a71d30fd2252ce2208c13.
diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
index d295540..053315e 100644
--- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc
+++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
@@ -7886,12 +7886,10 @@
// A problem occurred during a system call, but this is not
// necessarily an error.
case SSL_ERROR_SYSCALL: {
- // Connection closed without proper shutdown. Throw a SocketException to
- // indicate that the socket is closed.
+ // Connection closed without proper shutdown. Tell caller we
+ // have reached end-of-stream.
if (result == 0) {
- conscrypt::jniutil::throwException(env, "java/net/SocketException",
- "Socket closed");
- return THROWN_EXCEPTION;
+ return -1;
}
// System call has been interrupted. Simply retry.
diff --git a/openjdk-integ-tests/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java b/openjdk-integ-tests/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java
index f224a0a..930f4ae 100644
--- a/openjdk-integ-tests/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java
+++ b/openjdk-integ-tests/src/test/java/org/conscrypt/javax/net/ssl/SSLSocketTest.java
@@ -1955,8 +1955,16 @@
// Read from the socket.
try {
toRead.setSoTimeout(readingTimeoutMillis);
- toRead.getInputStream().read();
- fail();
+ int read = toRead.getInputStream().read();
+ // In the case of reading the wrapper and closing the underlying socket,
+ // there is a race condition between the reading thread being woken and
+ // reading the socket again and the closing thread marking the file descriptor
+ // as invalid. If the latter happens first, a SocketException is thrown,
+ // but if the former happens first it just looks like the peer closed the
+ // connection and a -1 return is acceptable.
+ if (read != -1 || readUnderlying || !closeUnderlying) {
+ fail();
+ }
} catch (SocketTimeoutException e) {
throw e;
} catch (IOException expected) {
