Skip to content

GitLab

Commit 5f2e6872 authored by Brian Carlstrom's avatar Brian Carlstrom
Browse files

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
parent 7023c8ff
Hide whitespace changes
Inline Side-by-side
Showing with 142 additions and 2 deletions
luni/src/main/java/org/apache/harmony/xnet/provider/jsse/OpenSSLSocketImpl.java
View file @ 5f2e6872
......@@ -300,11 +300,24 @@ public class OpenSSLSocketImpl
startHandshake(true);
}
/**
* 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");
}
......
luni/src/test/java/libcore/javax/net/ssl/SSLEngineTest.java
View file @ 5f2e6872
......@@ -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 {
......
luni/src/test/java/libcore/javax/net/ssl/SSLSocketTest.java
View file @ 5f2e6872
......@@ -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());
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment