Skip to content

Commit 051aea7

Browse files
authored
Fix edge cases with UTF-8 strings in ChecksumResultSet (#13441)
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 resets the buffer's write limit back to its full capacity for the next iteration, instead of shrinking it to the size of the previous write. 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 cfc6ba7 commit 051aea7

2 files changed

Lines changed: 42 additions & 3 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,8 @@ private void putString(String stringValue) {
330330
if (buffer == null || (buffer.capacity() < MAX_BUFFER_SIZE && buffer.capacity() < length)) {
331331
// Create a ByteBuffer with a maximum buffer size.
332332
// This buffer is re-used for all string values in the result set.
333-
buffer = ByteBuffer.allocate(Math.min(MAX_BUFFER_SIZE, length));
333+
// UTF-8 can require 4 bytes to represent, so we need at least that size buffer.
334+
buffer = ByteBuffer.allocate(Math.max(4, Math.min(MAX_BUFFER_SIZE, length)));
334335
} else {
335336
buffer.clear();
336337
}
@@ -354,8 +355,8 @@ private void putString(String stringValue) {
354355
buffer.flip();
355356
// Put the bytes from the buffer into the digest.
356357
digest.update(buffer);
357-
// Flip the buffer again, so we can repeat and write to the start of the buffer again.
358-
buffer.flip();
358+
// Clear the buffer, so we can repeat and write to the start of the buffer again.
359+
buffer.clear();
359360
}
360361
}
361362
}

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)