From: Brian Carlstrom Date: Mon, 23 Aug 2010 21:06:51 +0000 (-0700) Subject: SSLSocket.read should throw SocketException not NullPointerException X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=5f2e6872311240319509aed64d9f58cd5b64719b;p=android-x86%2Flibcore.git SSLSocket.read should throw SocketException not NullPointerException OpenSSLSocketImpl now uses checkOpen similar to Socket's checkOpenAndCreate to ensure that SocketExceptions are thrown if certain operations are tried after the socket is closed. Also added *_setUseClientMode_afterHandshake tests for SSLSocket and SSLEngine. We properly through IllegalArgument exception in this case, but it wasn't covered by the tests previously. Bug: 2918499 Change-Id: I393ad39bed40a33725d2c0f3f08b9d0b0d3ff85f --- 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 9cb771d0..aa3f75eb 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 @@ -301,10 +301,23 @@ public class OpenSSLSocketImpl } /** + * Checks whether the socket is closed, and throws an exception. + * + * @throws SocketException + * if the socket is closed. + */ + private void checkOpen() throws SocketException { + if (isClosed()) { + throw new SocketException("Socket is closed"); + } + } + + /** * Perform the handshake * @param full If true, disable handshake cutthrough for a fully synchronous handshake */ public synchronized void startHandshake(boolean full) throws IOException { + checkOpen(); synchronized (handshakeLock) { if (!handshakeStarted) { handshakeStarted = true; @@ -631,6 +644,7 @@ public class OpenSSLSocketImpl */ @Override public InputStream getInputStream() throws IOException { + checkOpen(); synchronized (this) { if (is == null) { is = new SSLInputStream(); @@ -650,6 +664,7 @@ public class OpenSSLSocketImpl */ @Override public OutputStream getOutputStream() throws IOException { + checkOpen(); synchronized (this) { if (os == null) { os = new SSLOutputStream(); @@ -702,6 +717,7 @@ public class OpenSSLSocketImpl */ @Override public int read() throws IOException { + checkOpen(); synchronized (readLock) { return NativeCrypto.SSL_read_byte(sslNativePointer, timeout); } @@ -713,6 +729,7 @@ public class OpenSSLSocketImpl */ @Override public int read(byte[] b, int off, int len) throws IOException { + checkOpen(); if (b == null) { throw new NullPointerException("b == null"); } @@ -748,6 +765,7 @@ public class OpenSSLSocketImpl */ @Override public void write(int b) throws IOException { + checkOpen(); synchronized (writeLock) { NativeCrypto.SSL_write_byte(sslNativePointer, b); } @@ -759,6 +777,7 @@ public class OpenSSLSocketImpl */ @Override public void write(byte[] b, int start, int len) throws IOException { + checkOpen(); if (b == null) { throw new NullPointerException("b == null"); } diff --git a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java index b91942a4..94558dbc 100644 --- a/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java +++ b/luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java @@ -231,6 +231,22 @@ public class SSLEngineTest extends TestCase { assertNotConnected(test_SSLEngine_setUseClientMode(false, false)); } + public void test_SSLEngine_setUseClientMode_afterHandshake() throws Exception { + + // can't set after handshake + TestSSLEnginePair pair = TestSSLEnginePair.create(null); + try { + pair.server.setUseClientMode(false); + fail(); + } catch (IllegalArgumentException expected) { + } + try { + pair.client.setUseClientMode(false); + fail(); + } catch (IllegalArgumentException expected) { + } + } + private TestSSLEnginePair test_SSLEngine_setUseClientMode(final boolean clientClientMode, final boolean serverClientMode) throws Exception { 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 5807d6b4..94dc082b 100644 --- a/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java +++ b/luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java @@ -16,11 +16,12 @@ package libcore.javax.net.ssl; +import java.io.InputStream; +import java.io.OutputStream; import java.net.Socket; +import java.net.SocketException; import java.net.SocketTimeoutException; import java.security.Principal; -import libcore.java.security.StandardNames; -import libcore.java.security.TestKeyStore; import java.security.cert.Certificate; import java.util.Arrays; import javax.net.ssl.HandshakeCompletedEvent; @@ -34,6 +35,8 @@ import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSocket; import javax.net.ssl.SSLSocketFactory; import junit.framework.TestCase; +import libcore.java.security.StandardNames; +import libcore.java.security.TestKeyStore; public class SSLSocketTest extends TestCase { @@ -437,6 +440,22 @@ public class SSLSocketTest extends TestCase { } } + public void test_SSLSocket_setUseClientMode_afterHandshake() throws Exception { + + // can't set after handshake + TestSSLEnginePair pair = TestSSLEnginePair.create(null); + try { + pair.server.setUseClientMode(false); + fail(); + } catch (IllegalArgumentException expected) { + } + try { + pair.client.setUseClientMode(false); + fail(); + } catch (IllegalArgumentException expected) { + } + } + private void test_SSLSocket_setUseClientMode(final boolean clientClientMode, final boolean serverClientMode) throws Exception { @@ -668,6 +687,90 @@ public class SSLSocketTest extends TestCase { } } + public void test_SSLSocket_close() throws Exception { + TestSSLSocketPair pair = TestSSLSocketPair.create(); + SSLSocket server = pair.server; + SSLSocket client = pair.client; + assertFalse(server.isClosed()); + assertFalse(client.isClosed()); + InputStream input = client.getInputStream(); + OutputStream output = client.getOutputStream(); + server.close(); + client.close(); + assertTrue(server.isClosed()); + assertTrue(client.isClosed()); + + // close after close is okay... + server.close(); + client.close(); + + // ...so are a lot of other operations... + HandshakeCompletedListener l = new HandshakeCompletedListener () { + public void handshakeCompleted(HandshakeCompletedEvent e) {} + }; + client.addHandshakeCompletedListener(l); + assertNotNull(client.getEnabledCipherSuites()); + assertNotNull(client.getEnabledProtocols()); + client.getEnableSessionCreation(); + client.getNeedClientAuth(); + assertNotNull(client.getSession()); + assertNotNull(client.getSSLParameters()); + assertNotNull(client.getSupportedProtocols()); + client.getUseClientMode(); + client.getWantClientAuth(); + client.removeHandshakeCompletedListener(l); + client.setEnabledCipherSuites(new String[0]); + client.setEnabledProtocols(new String[0]); + client.setEnableSessionCreation(false); + client.setNeedClientAuth(false); + client.setSSLParameters(client.getSSLParameters()); + client.setWantClientAuth(false); + + // ...but some operations are expected to give SocketException... + try { + client.startHandshake(); + fail(); + } catch (SocketException expected) { + } + try { + client.getInputStream(); + fail(); + } catch (SocketException expected) { + } + try { + client.getOutputStream(); + fail(); + } catch (SocketException expected) { + } + try { + input.read(); + fail(); + } catch (SocketException expected) { + } + try { + input.read(null, -1, -1); + fail(); + } catch (SocketException expected) { + } + try { + output.write(-1); + fail(); + } catch (SocketException expected) { + } + try { + output.write(null, -1, -1); + fail(); + } catch (SocketException expected) { + } + + // ... and one gives IllegalArgumentException + try { + client.setUseClientMode(false); + fail(); + } catch (IllegalArgumentException expected) { + } + } + public void test_TestSSLSocketPair_create() { TestSSLSocketPair test = TestSSLSocketPair.create(); assertNotNull(test.c); @@ -675,6 +778,8 @@ public class SSLSocketTest extends TestCase { assertNotNull(test.client); assertTrue(test.server.isConnected()); assertTrue(test.client.isConnected()); + assertFalse(test.server.isClosed()); + assertFalse(test.client.isClosed()); assertNotNull(test.server.getSession()); assertNotNull(test.client.getSession()); assertTrue(test.server.getSession().isValid());