This is an automated email from the ASF dual-hosted git repository.
LuciferYang pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new fb14e6cee794 [SPARK-57520][SQL] Fix UTF8String.codePointFrom and
copyUTF8String reading past the end of a truncated trailing UTF-8 sequence
fb14e6cee794 is described below
commit fb14e6cee794191b8a27f3acc51e695492edeaa6
Author: YangJie <[email protected]>
AuthorDate: Mon Jun 22 10:36:04 2026 +0800
[SPARK-57520][SQL] Fix UTF8String.codePointFrom and copyUTF8String reading
past the end of a truncated trailing UTF-8 sequence
### What changes were proposed in this pull request?
`UTF8String.codePointFrom` decodes a code point by reading
`numBytesForFirstByte(leader)` continuation bytes, and `copyUTF8String` copies
`end - start + 1` bytes. Neither bounds the read by the bytes that actually
remain, so when a string ends in a truncated multi-byte sequence (a leader byte
whose declared width exceeds the remaining bytes), both read past the end of
the backing memory. `trimLeft`/`trimRight` build their search character through
`copyUTF8String`, so they over-read too.
This PR:
- `codePointFrom` reads continuation bytes through a small
`continuationByte` helper that returns 0 once the index passes the end of the
string.
- `copyUTF8String` clamps the copy length to `numBytes - start`.
- Once `copyUTF8String` stops over-reading, `trimRight` needs a matching
accounting fix: it advanced `trimEnd` by the leader's declared width, which
overshoots a truncated trailing character, so it now uses the actual (clamped)
byte count, as `trimLeft` already does.
### Why are the changes needed?
`UTF8String` can hold malformed UTF-8 (for example, bytes from binary
coercion or truncated input). For a string ending in an incomplete multi-byte
sequence, these methods read out of bounds and produced wrong results:
`codePointFrom` assembled a code point from adjacent memory, and `trimRight`
could drop valid leading characters. Well-formed UTF-8 is unaffected, since a
complete sequence never exceeds the remaining bytes.
This is a follow-up to SPARK-57507, which fixed the same kind of over-read
in `reverse()`.
### Does this PR introduce _any_ user-facing change?
Yes, it fixes incorrect results on malformed input. String operations that
reach these methods (such as trimming or code-point access) no longer read past
the end of a value that ends in a truncated multi-byte sequence; only
previously-incorrect results change. Well-formed strings behave exactly as
before.
### How was this patch tested?
Added cases to `UTF8StringSuite`:
- `testCodePointFrom`: truncated trailing 2-, 3-, and 4-byte leaders,
including a 4-byte leader with only the last continuation byte missing.
- `copyUTF8StringClampsToRemainingBytes`: an `end` one past the last byte,
with a non-zero start so the clamp must use `numBytes - start`.
- `trimTruncatedTrailingSequence`: trimming a truncated trailing leader
keeps the valid preceding character.
Each uses a sliced backing array with a trailing sentinel byte, so the
previous over-read produces a deterministically wrong value; the cases fail on
the old code and pass with the fix. `build/sbt 'unsafe/testOnly
*UTF8StringSuite'` passes (51 tests).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #56585 from LuciferYang/SPARK-57520-utf8-overread.
Authored-by: YangJie <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
(cherry picked from commit 33ae7f6e5a886dc184f64e31eb149eba0ac859f8)
Signed-off-by: yangjie01 <[email protected]>
---
.../org/apache/spark/unsafe/types/UTF8String.java | 32 +++++++++----
.../apache/spark/unsafe/types/UTF8StringSuite.java | 55 ++++++++++++++++++++++
2 files changed, 79 insertions(+), 8 deletions(-)
diff --git
a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
index 5ece20818296..ac41812bc6d3 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/types/UTF8String.java
@@ -724,7 +724,8 @@ public final class UTF8String implements
Comparable<UTF8String>, Externalizable,
/**
* Returns the code point starting from the byte at position `byteIndex`.
- * If byte index is invalid, throws exception.
+ * If byte index is invalid, throws exception. If the sequence is truncated
(the leader byte
+ * declares more bytes than remain), the missing continuation bytes are
treated as 0.
*/
public int codePointFrom(int byteIndex) {
if (byteIndex < 0 || byteIndex >= numBytes) {
@@ -736,18 +737,28 @@ public final class UTF8String implements
Comparable<UTF8String>, Externalizable,
case 1 ->
b & 0x7F;
case 2 ->
- ((b & 0x1F) << 6) | (getByte(byteIndex + 1) & 0x3F);
+ ((b & 0x1F) << 6) | continuationByte(byteIndex + 1);
case 3 ->
- ((b & 0x0F) << 12) | ((getByte(byteIndex + 1) & 0x3F) << 6) |
- (getByte(byteIndex + 2) & 0x3F);
+ ((b & 0x0F) << 12) | (continuationByte(byteIndex + 1) << 6) |
+ continuationByte(byteIndex + 2);
case 4 ->
- ((b & 0x07) << 18) | ((getByte(byteIndex + 1) & 0x3F) << 12) |
- ((getByte(byteIndex + 2) & 0x3F) << 6) | (getByte(byteIndex + 3) &
0x3F);
+ ((b & 0x07) << 18) | (continuationByte(byteIndex + 1) << 12) |
+ (continuationByte(byteIndex + 2) << 6) | continuationByte(byteIndex +
3);
default ->
throw new IllegalStateException("Error in UTF-8 code point");
};
}
+ /**
+ * Returns the low 6 bits of the UTF-8 continuation byte at `byteIndex`, or
0 when `byteIndex`
+ * is past the end of the string. The bounds check stops a truncated
trailing multi-byte
+ * sequence (a leader byte whose declared width exceeds the bytes that
remain) from reading
+ * past the end of the backing memory.
+ */
+ private int continuationByte(int byteIndex) {
+ return byteIndex < numBytes ? getByte(byteIndex) & 0x3F : 0;
+ }
+
public boolean matchAt(final UTF8String s, int pos) {
if (s.numBytes + pos > numBytes || pos < 0) {
return false;
@@ -944,7 +955,10 @@ public final class UTF8String implements
Comparable<UTF8String>, Externalizable,
* @return a new UTF8String in the position of [start, end] of current
UTF8String bytes.
*/
public UTF8String copyUTF8String(int start, int end) {
- int len = end - start + 1;
+ // Clamp to the bytes that actually remain so an out-of-range `end` (for
example, derived
+ // from a truncated trailing multi-byte sequence) can't copy past the end
of the backing
+ // memory.
+ int len = Math.min(end - start + 1, numBytes - start);
byte[] newBytes = new byte[len];
copyMemory(base, offset + start, newBytes, BYTE_ARRAY_OFFSET, len);
return UTF8String.fromBytes(newBytes);
@@ -1137,7 +1151,9 @@ public final class UTF8String implements
Comparable<UTF8String>, Externalizable,
stringCharPos[numChars - 1],
stringCharPos[numChars - 1] + stringCharLen[numChars - 1] - 1);
if (trimString.find(searchChar, 0) >= 0) {
- trimEnd -= stringCharLen[numChars - 1];
+ // Advance by the bytes the character actually occupies. A truncated
trailing leader is
+ // shorter than the width its leader byte declares, so use the
(clamped) search char.
+ trimEnd -= searchChar.numBytes;
} else {
break;
}
diff --git
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
index a9f365173be6..0374a1672d22 100644
---
a/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
+++
b/common/unsafe/src/test/java/org/apache/spark/unsafe/types/UTF8StringSuite.java
@@ -1228,6 +1228,61 @@ public class UTF8StringSuite {
assertThrows(IndexOutOfBoundsException.class, () -> s.codePointFrom(-1));
assertThrows(IndexOutOfBoundsException.class, () ->
s.codePointFrom(str.length()));
assertThrows(IndexOutOfBoundsException.class, () ->
s.codePointFrom(str.length() + 1));
+
+ // Truncated trailing multi-byte sequence: the leader declares more bytes
than remain.
+ // codePointFrom should decode only the bytes present (missing
continuation bytes count as
+ // 0) and not read past the end. Each backing array has extra trailing
bytes, so an
+ // over-read regression would show up in the result.
+ // 2-byte leader 0xCE with no continuation byte present.
+ assertEquals((0xCE & 0x1F) << 6,
+ fromBytes(new byte[] {(byte) 0xCE, 0x42}, 0, 1).codePointFrom(0));
+ // 3-byte leader 0xE4 with no continuation bytes present.
+ assertEquals((0xE4 & 0x0F) << 12,
+ fromBytes(new byte[] {(byte) 0xE4, 0x42, 0x43}, 0, 1).codePointFrom(0));
+ // 3-byte leader 0xE4 0xB8 with the final continuation byte missing.
+ assertEquals(((0xE4 & 0x0F) << 12) | ((0xB8 & 0x3F) << 6),
+ fromBytes(new byte[] {(byte) 0xE4, (byte) 0xB8, 0x42}, 0,
2).codePointFrom(0));
+ // 4-byte leader 0xF1 with no continuation bytes present.
+ assertEquals((0xF1 & 0x07) << 18,
+ fromBytes(new byte[] {(byte) 0xF1, 0x42, 0x43, 0x44}, 0,
1).codePointFrom(0));
+ // 4-byte leader 0xF1 with two continuation bytes present and only the
last one missing,
+ // so just the final read crosses the end.
+ assertEquals(((0xF1 & 0x07) << 18) | ((0x9F & 0x3F) << 12) | ((0x8F &
0x3F) << 6),
+ fromBytes(new byte[] {(byte) 0xF1, (byte) 0x9F, (byte) 0x8F, 0x42}, 0,
3).codePointFrom(0));
+ }
+
+ @Test
+ public void copyUTF8StringClampsToRemainingBytes() {
+ // Here `end` runs one byte past the string, as it would for a truncated
trailing sequence.
+ // copyUTF8String should clamp to the available bytes; the extra backing
byte would show up
+ // in the result if it over-read.
+ byte[] backing = new byte[] {0x41, 0x42, 0x43};
+ UTF8String s = fromBytes(backing, 0, 2); // views "AB"
+ // `end` (2) is one past the last valid byte index (1); only the two real
bytes are copied.
+ assertEquals(fromString("AB"), s.copyUTF8String(0, 2));
+ // Same with a non-zero start, so the clamp uses `numBytes - start`, not
`numBytes`.
+ assertEquals(fromString("B"), s.copyUTF8String(1, 2));
+ // In-bounds copies are unaffected.
+ assertEquals(fromString("AB"), s.copyUTF8String(0, 1));
+ assertEquals(fromString("B"), s.copyUTF8String(1, 1));
+ }
+
+ @Test
+ public void trimTruncatedTrailingSequence() {
+ // trimLeft/trimRight build the search character through copyUTF8String,
so an over-read would
+ // make it longer than the bytes that remain. The backing arrays carry an
extra trailing byte
+ // to make any over-read deterministic.
+ // A lone truncated 2-byte leader (0xC2): the clamped search char is just
the leader, which
+ // matches the 1-byte trim set and is trimmed away.
+ UTF8String lone = fromBytes(new byte[] {(byte) 0xC2, 0x42}, 0, 1);
+ UTF8String trim2 = fromBytes(new byte[] {(byte) 0xC2});
+ assertEquals(EMPTY_UTF8, lone.trimLeft(trim2));
+ assertEquals(EMPTY_UTF8, lone.trimRight(trim2));
+ // 'A' followed by a truncated 3-byte leader (0xE4). Trimming the leader
from the right must
+ // keep 'A': the trailing character occupies only one real byte, so only
that byte is removed.
+ UTF8String prefixed = fromBytes(new byte[] {0x41, (byte) 0xE4, 0x42}, 0,
2);
+ UTF8String trim3 = fromBytes(new byte[] {(byte) 0xE4});
+ assertEquals(fromString("A"), prefixed.trimRight(trim3));
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
