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 9cb771d..aa3f75e 100644 (file)
--- 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 b91942a..94558db 100644 (file)
--- 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 5807d6b..94dc082 100644 (file)
--- 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());