From a4a95792af235d4bf3256eab3208f74fae8ec262 Mon Sep 17 00:00:00 2001 From: Brian Carlstrom Date: Sat, 18 Sep 2010 21:16:01 -0700 Subject: [PATCH] SSLSocket should respect timeout of a wrapped Socket Change to using getSoTimeout in OpenSSLSocketImpl instead of directly using the timeout field. This means the proper timeout will be used for instances of the OpenSSLSocketImplWrapper subclass, which is used when an SSLSocket is wrapped around an existing connected non-SSL Socket. The code still maintains the local timeout field, now renamed timeoutMilliseconds, which is now accesed via OpenSSLSocketImpl.getSoTimeout. Doing so prevents a getsockopt syscall that otherwise would be necessary if the super.getSoTimeout() was used. Added two unit tests for testing timeouts with SSLSockets wrapped around Socket. One is simply for getters/setters. The second makes sure the timeout is functioning when set on the underlying socket. Bug: 2973305 Change-Id: Idac52853f5d777fae5060a840eefbfe85d448e4c --- .../xnet/provider/jsse/OpenSSLSocketImpl.java | 50 ++++++++++++++-------- .../java/libcore/javax/net/ssl/SSLSocketTest.java | 45 +++++++++++++++++++ 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java index 28cfffab..f37282e7 100644 --- a/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java +++ b/luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java @@ -82,9 +82,17 @@ public class OpenSSLSocketImpl private boolean handshakeCompleted = false; private ArrayList listeners; - private int timeout = 0; + + /** + * Local cache of timeout to avoid getsockopt on every read and + * write for non-wrapped sockets. Note that + * OpenSSLSocketImplWrapper overrides setSoTimeout and + * getSoTimeout to delegate to the wrapped socket. + */ + private int timeoutMilliseconds = 0; + // BEGIN android-added - private int handshakeTimeout = -1; // -1 = same as timeout; 0 = infinite + private int handshakeTimeoutMilliseconds = -1; // -1 = same as timeout; 0 = infinite // END android-added private String wrappedHost; private int wrappedPort; @@ -189,11 +197,14 @@ public class OpenSSLSocketImpl boolean autoClose, SSLParametersImpl sslParameters) throws IOException { super(); this.socket = socket; - this.timeout = socket.getSoTimeout(); this.wrappedHost = host; this.wrappedPort = port; this.autoClose = autoClose; init(sslParameters); + + // this.timeout is not set intentionally. + // OpenSSLSocketImplWrapper.getSoTimeout will delegate timeout + // to wrapped socket } /** @@ -435,9 +446,9 @@ public class OpenSSLSocketImpl // BEGIN android-added // Temporarily use a different timeout for the handshake process - int savedTimeout = timeout; - if (handshakeTimeout >= 0) { - setSoTimeout(handshakeTimeout); + int savedTimeoutMilliseconds = getSoTimeout(); + if (handshakeTimeoutMilliseconds >= 0) { + setSoTimeout(handshakeTimeoutMilliseconds); } // END android-added @@ -445,8 +456,8 @@ public class OpenSSLSocketImpl Socket socket = this.socket != null ? this.socket : this; int sslSessionNativePointer; try { - sslSessionNativePointer - = NativeCrypto.SSL_do_handshake(sslNativePointer, socket, this, timeout, client); + sslSessionNativePointer = NativeCrypto.SSL_do_handshake(sslNativePointer, socket, + this, getSoTimeout(), client); } catch (CertificateException e) { throw new SSLPeerUnverifiedException(e.getMessage()); } @@ -487,8 +498,8 @@ public class OpenSSLSocketImpl // BEGIN android-added // Restore the original timeout now that the handshake is complete - if (handshakeTimeout >= 0) { - setSoTimeout(savedTimeout); + if (handshakeTimeoutMilliseconds >= 0) { + setSoTimeout(savedTimeoutMilliseconds); } // END android-added @@ -734,7 +745,7 @@ public class OpenSSLSocketImpl checkOpen(); BlockGuard.getThreadPolicy().onNetwork(); synchronized (readLock) { - return NativeCrypto.SSL_read_byte(sslNativePointer, timeout); + return NativeCrypto.SSL_read_byte(sslNativePointer, getSoTimeout()); } } @@ -756,7 +767,7 @@ public class OpenSSLSocketImpl return 0; } synchronized (readLock) { - return NativeCrypto.SSL_read(sslNativePointer, b, off, len, timeout); + return NativeCrypto.SSL_read(sslNativePointer, b, off, len, getSoTimeout()); } } } @@ -1136,9 +1147,14 @@ public class OpenSSLSocketImpl * @throws SocketException if an error occurs setting the option */ @Override - public void setSoTimeout(int timeout) throws SocketException { - super.setSoTimeout(timeout); - this.timeout = timeout; + public void setSoTimeout(int timeoutMilliseconds) throws SocketException { + super.setSoTimeout(timeoutMilliseconds); + this.timeoutMilliseconds = timeoutMilliseconds; + } + + @Override + public int getSoTimeout() throws SocketException { + return timeoutMilliseconds; } // BEGIN android-added @@ -1148,8 +1164,8 @@ public class OpenSSLSocketImpl * * @param timeout the handshake timeout value */ - public void setHandshakeTimeout(int timeout) throws SocketException { - this.handshakeTimeout = timeout; + public void setHandshakeTimeout(int timeoutMilliseconds) throws SocketException { + this.handshakeTimeoutMilliseconds = timeoutMilliseconds; } // END android-added diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java index 94dc082b..dbd9a2a5 100644 --- a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java +++ b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java @@ -18,6 +18,7 @@ package libcore.javax.net.ssl; import java.io.InputStream; import java.io.OutputStream; +import java.net.ServerSocket; import java.net.Socket; import java.net.SocketException; import java.net.SocketTimeoutException; @@ -771,6 +772,50 @@ public class SSLSocketTest extends TestCase { } } + public void test_SSLSocket_setSoTimeout_basic() throws Exception { + ServerSocket listening = new ServerSocket(0); + + Socket underlying = new Socket(listening.getInetAddress(), listening.getLocalPort()); + assertEquals(0, underlying.getSoTimeout()); + + SSLSocketFactory sf = (SSLSocketFactory) SSLSocketFactory.getDefault(); + Socket wrapping = sf.createSocket(underlying, null, -1, false); + assertEquals(0, wrapping.getSoTimeout()); + + // setting wrapper sets underlying and ... + wrapping.setSoTimeout(10); + assertEquals(10, wrapping.getSoTimeout()); + assertEquals(10, underlying.getSoTimeout()); + + // ... getting wrapper inspects underlying + underlying.setSoTimeout(0); + assertEquals(0, wrapping.getSoTimeout()); + assertEquals(0, underlying.getSoTimeout()); + } + + public void test_SSLSocket_setSoTimeout_wrapper() throws Exception { + ServerSocket listening = new ServerSocket(0); + + // setSoTimeout applies to read, not connect, so connect first + Socket underlying = new Socket(listening.getInetAddress(), listening.getLocalPort()); + Socket server = listening.accept(); + + SSLSocketFactory sf = (SSLSocketFactory) SSLSocketFactory.getDefault(); + Socket clientWrapping = sf.createSocket(underlying, null, -1, false); + + underlying.setSoTimeout(1); + try { + clientWrapping.getInputStream().read(); + fail(); + } catch (SocketTimeoutException expected) { + } + + clientWrapping.close(); + server.close(); + underlying.close(); + listening.close(); + } + public void test_TestSSLSocketPair_create() { TestSSLSocketPair test = TestSSLSocketPair.create(); assertNotNull(test.c); -- 2.11.0