Skip to content

Commit

Permalink
Fix zero size reads for Engine Socket.
Browse files Browse the repository at this point in the history
InputStream explicitly allows zero size reads without attempting
IO or blocking.  We do this correctly for the fd-socket but
missed it for the Engine socket.

Not clear whether zero sized read should trigger a TLS handshake,
but it always has on Android so keeping it that way.

Bug manifested as a flaky test hang.  Fixed the test to
explicitly check corner cases and removed un-needed concurrency.
  • Loading branch information
prbprbprb authored and tweksteen committed Nov 24, 2024
1 parent d798f1b commit 4f591b4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 12 deletions.
3 changes: 3 additions & 0 deletions common/src/main/java/org/conscrypt/ConscryptEngineSocket.java
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,9 @@ public int read(byte[] b) throws IOException {
@Override
public int read(byte[] b, int off, int len) throws IOException {
waitForHandshake();
if (len == 0) {
return 0;
}
synchronized (readLock) {
return readUntilDataAvailable(b, off, len);
}
Expand Down
38 changes: 26 additions & 12 deletions openjdk/src/test/java/org/conscrypt/ConscryptSocketTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -731,18 +731,28 @@ public void savedSessionWorksAfterClose() throws Exception {
@Test
public void dataFlows() throws Exception {
final TestConnection connection =
new TestConnection(new X509Certificate[] {cert, ca}, certKey);
new TestConnection(new X509Certificate[]{cert, ca}, certKey);
connection.doHandshakeSuccess();
// Max app data size that will fit in a single TLS record.
int maxDataSize = connection.client.getSession().getApplicationBufferSize();

// Basic data flow assurance. Send random buffers in each direction, each less than 16K
// so should fit in a single TLS packet. 50% chance of sending in each direction on
// each iteration to randomize the flow.
// Zero sized reads and writes. InputStream.read() allows zero size reads
// to succeed even when no data is available.
sendData(connection.client, connection.server, randomBuffer(0));
sendData(connection.server, connection.client, randomBuffer(0));

// Completely full record.
sendData(connection.client, connection.server, randomBuffer(maxDataSize));
sendData(connection.server, connection.client, randomBuffer(maxDataSize));

// Random workout. Send random sized buffers in each direction, 50% chance of sending in
// each direction on each iteration to randomize the flow.
for (int i = 0; i < 50; i++) {
if (random.nextBoolean()) {
sendData(connection.client, connection.server, randomBuffer());
sendData(connection.client, connection.server, randomSizeBuffer(maxDataSize));
}
if (random.nextBoolean()) {
sendData(connection.server, connection.client, randomBuffer());
sendData(connection.server, connection.client, randomSizeBuffer(maxDataSize));
}
}
}
Expand All @@ -751,16 +761,20 @@ private void sendData(SSLSocket source, final SSLSocket destination, byte[] data
throws Exception {
final byte[] received = new byte[data.length];

Future<Integer> readFuture = executor.submit(
() -> destination.getInputStream().read(received));

source.getOutputStream().write(data);
assertEquals(data.length, (int) readFuture.get());
assertEquals(data.length, destination.getInputStream().read(received));
assertArrayEquals(data, received);
}

private byte[] randomBuffer() {
byte[] buffer = new byte[random.nextInt(16 * 1024)];
// Returns a random sized buffer containing random data.
// Zero and maxSize are valid possible sizes for the returned buffer.
private byte[] randomSizeBuffer(int maxSize) {
return randomBuffer(random.nextInt(maxSize + 1));
}

// Returns a buffer of random data of the size requested.
private byte[] randomBuffer(int size) {
byte[] buffer = new byte[size];
random.nextBytes(buffer);
return buffer;
}
Expand Down

0 comments on commit 4f591b4

Please sign in to comment.