Skip to content

Commit 1a1b88e

Browse files
committed
fix(java-spanner): Fix edge cases with UTF-8 strings in ChecksumResultSet
1. Ensure a minimum capacity of 4 bytes when allocating the buffer, this is the max size of a UTF-8 character. However, the java length representation is being used in this code for the byte buffer allocation, which may be too small for a single utf-8 character. 2. Use buffer.clear() instead of the second buffer.flip(). This is for mixed multi-byte utf-8 characters and single-byte characters. A test was added to show this passing, and it fails with flip() vs clear(). Fixes #13440
1 parent c6532ac commit 1a1b88e

2 files changed

Lines changed: 41 additions & 3 deletions

File tree

java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ChecksumResultSet.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ private void putString(String stringValue) {
329329
if (buffer == null || (buffer.capacity() < MAX_BUFFER_SIZE && buffer.capacity() < length)) {
330330
// Create a ByteBuffer with a maximum buffer size.
331331
// This buffer is re-used for all string values in the result set.
332-
buffer = ByteBuffer.allocate(Math.min(MAX_BUFFER_SIZE, length));
332+
buffer = ByteBuffer.allocate(Math.max(4, Math.min(MAX_BUFFER_SIZE, length)));
333333
} else {
334334
buffer.clear();
335335
}
@@ -349,8 +349,8 @@ private void putString(String stringValue) {
349349
buffer.flip();
350350
// Put the bytes from the buffer into the digest.
351351
digest.update(buffer);
352-
// Flip the buffer again, so we can repeat and write to the start of the buffer again.
353-
buffer.flip();
352+
// Clear the buffer, so we can repeat and write to the start of the buffer again.
353+
buffer.clear();
354354
}
355355
}
356356
}

java-spanner/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ChecksumResultSetTest.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,4 +448,42 @@ public void testRetry() {
448448
() -> resultSet.retry(abortedException));
449449
}
450450
}
451+
452+
@Test
453+
public void testEmptyString() {
454+
ChecksumResultSet resultSet = createStringValChecksumResultSet("");
455+
assertTrue(resultSet.next());
456+
}
457+
458+
@Test
459+
public void testSingleCharMultiByteString() {
460+
ChecksumResultSet resultSet = createStringValChecksumResultSet("ä");
461+
assertTrue(resultSet.next());
462+
}
463+
464+
@Test
465+
public void testLongMixedUtf8String() {
466+
ChecksumResultSet resultSet = createStringValChecksumResultSet("aaa\uD841\uDF0E");
467+
assertTrue(resultSet.next());
468+
}
469+
470+
private ChecksumResultSet createStringValChecksumResultSet(String value) {
471+
Type type = Type.struct(StructField.of("stringVal", Type.string()));
472+
Struct row = Struct.newBuilder().set("stringVal").to(value).build();
473+
474+
ParsedStatement parsedStatement = mock(ParsedStatement.class);
475+
Statement statement = Statement.of("select * from foo");
476+
when(parsedStatement.getStatement()).thenReturn(statement);
477+
ReadWriteTransaction transaction = mock(ReadWriteTransaction.class);
478+
when(transaction.runWithRetry(any(Callable.class)))
479+
.thenAnswer(invocationOnMock -> ((Callable<?>) invocationOnMock.getArgument(0)).call());
480+
when(transaction.getStatementExecutor()).thenReturn(mock(StatementExecutor.class));
481+
482+
ResultSet queryResult = ResultSets.forRows(type, ImmutableList.of(row));
483+
return new ChecksumResultSet(
484+
transaction,
485+
DirectExecuteResultSet.ofResultSet(queryResult),
486+
parsedStatement,
487+
AnalyzeMode.NONE);
488+
}
451489
}

0 commit comments

Comments
 (0)