This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new cac30c2d58 fix: Only require valid UTF-8 within the visible span of
string arrays (#11323)
cac30c2d58 is described below
commit cac30c2d586bde9b3c13a3f1166ddba4fbf87cd1
Author: Neil Conway <[email protected]>
AuthorDate: Sun Oct 4 03:41:18 2026 +0000
fix: Only require valid UTF-8 within the visible span of string arrays
(#11323)
# Which issue does this PR close?
- N/A
# Rationale for this change
Arrow does not require bytes outside the visible span of the values
buffer to be valid UTF-8. This rule was not consistently applied:
* `ArrayData` checked for UTF-8 validity over the visible span, but
* `StringArray::try_new` and `try_from_binary` checked for UTF-8
validity of the entire values buffer, which is both inefficient for
sliced arrays and also rejects otherwise valid inputs
* `substring` assumed that the entire values buffer contained valid
UTF-8 bytes, which was sound for inputs from the `StringArray` path but
not for the `ArrayData` path
* Binary to Utf8 casts (in `safe: true`) reserved enough space for the
whole values buffer, not the visible span
# What changes are included in this PR?
* Both validators now check only the bytes between the first and last
offset
* `substring` reads only those bytes as a `&str`.
* The `safe: true` fallback of the Binary to Utf8 cast only reserves
space for the visible span, not the whole values buffer.
* Improve some docs
* Add unit tests
# Are these changes tested?
Yes; existing tests pass, new tests added.
# Are there any user-facing changes?
No.
# AI usage
Developed with Claude Code (Opus 5.5), reviewed with Codex (Astra 6). I
revised and understand the resulting code.
---
arrow-array/src/array/byte_array.rs | 7 ++++++-
arrow-array/src/array/string_array.rs | 23 +++++++++++++++++++++++
arrow-array/src/types.rs | 25 ++++++++++++++-----------
arrow-cast/src/cast/mod.rs | 28 ++++++++++++++++++++++++++++
arrow-cast/src/cast/string.rs | 8 +++++---
arrow-data/src/data.rs | 33 ++++++++++++++++++++++++++-------
arrow-string/src/substring.rs | 20 ++++++++++++++++----
7 files changed, 118 insertions(+), 26 deletions(-)
diff --git a/arrow-array/src/array/byte_array.rs
b/arrow-array/src/array/byte_array.rs
index cd0b9f6358..049d1f47cf 100644
--- a/arrow-array/src/array/byte_array.rs
+++ b/arrow-array/src/array/byte_array.rs
@@ -282,13 +282,18 @@ impl<T: ByteArrayType> GenericByteArray<T> {
/// Returns the values of this array
///
/// Unlike [`Self::value_data`] this returns the [`Buffer`]
- /// allowing for zero-copy cloning
+ /// allowing for zero-copy cloning. Like [`Self::value_data`], it can
+ /// include bytes that are not part of any value.
#[inline]
pub fn values(&self) -> &Buffer {
&self.value_data
}
/// Returns the raw value data
+ ///
+ /// This can include bytes before the first offset or after the last one,
+ /// for example when the array is a slice of a larger array. Those bytes
are
+ /// not part of any value, and in a string array they need not be valid
UTF-8.
pub fn value_data(&self) -> &[u8] {
self.value_data.as_slice()
}
diff --git a/arrow-array/src/array/string_array.rs
b/arrow-array/src/array/string_array.rs
index c98b2a1b2a..39427181d2 100644
--- a/arrow-array/src/array/string_array.rs
+++ b/arrow-array/src/array/string_array.rs
@@ -504,6 +504,29 @@ mod tests {
let _ = StringArray::from(list);
}
+ #[test]
+ fn test_string_array_invalid_bytes_outside_offsets() {
+ // Only the bytes that the offsets span need to be valid UTF-8, as in a
+ // slice of a larger array
+ let values = Buffer::from_slice_ref(b"\xFFa\xC3\xA9\xFF");
+ let offsets = OffsetBuffer::new(vec![1, 2, 4].into());
+ let string = StringArray::try_new(offsets, values.clone(),
None).unwrap();
+ assert_eq!(string, StringArray::from(vec!["a", "é"]));
+
+ let offsets = OffsetBuffer::new(vec![1, 3, 4].into());
+ let err = StringArray::try_new(offsets, values, None).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Invalid argument error: Split UTF-8 codepoint at offset 3"
+ );
+
+ // An empty value whose offset is inside a character, in a values
buffer
+ // that is valid UTF-8 as a whole. ArrayData validation must agree.
+ let offsets = OffsetBuffer::new(vec![1, 1].into());
+ let string = StringArray::try_new(offsets,
Buffer::from_slice_ref("é"), None).unwrap();
+ string.to_data().validate_full().unwrap();
+ }
+
#[test]
fn test_empty_offsets() {
let string = StringArray::from(
diff --git a/arrow-array/src/types.rs b/arrow-array/src/types.rs
index ba0d88ae64..c7f2d51989 100644
--- a/arrow-array/src/types.rs
+++ b/arrow-array/src/types.rs
@@ -1686,23 +1686,26 @@ impl<O: OffsetSizeTrait> ByteArrayType for
GenericStringType<O> {
};
fn validate(offsets: &OffsetBuffer<Self::Offset>, values: &Buffer) ->
Result<(), ArrowError> {
- // Verify that the slice as a whole is valid UTF-8
- let validated = std::str::from_utf8(values).map_err(|e| {
+ let start = offsets.first().as_usize();
+ let end = offsets.last().as_usize();
+ if end > values.len() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Offset of {end} exceeds length of values {}",
+ values.len()
+ )));
+ }
+
+ // Verify that the bytes the offsets span are valid UTF-8
+ let validated = std::str::from_utf8(&values[start..end]).map_err(|e| {
ArrowError::InvalidArgumentError(format!("Encountered non UTF-8
data: {e}"))
})?;
- // Verify each offset is at a valid character boundary in this UTF-8
array
+ // Verify each offset is at a valid character boundary
for offset in offsets.iter() {
let o = offset.as_usize();
- if !validated.is_char_boundary(o) {
- if o < validated.len() {
- return Err(ArrowError::InvalidArgumentError(format!(
- "Split UTF-8 codepoint at offset {o}"
- )));
- }
+ if !validated.is_char_boundary(o - start) {
return Err(ArrowError::InvalidArgumentError(format!(
- "Offset of {o} exceeds length of values {}",
- validated.len()
+ "Split UTF-8 codepoint at offset {o}"
)));
}
}
diff --git a/arrow-cast/src/cast/mod.rs b/arrow-cast/src/cast/mod.rs
index cd858859ea..3d7fb88c79 100644
--- a/arrow-cast/src/cast/mod.rs
+++ b/arrow-cast/src/cast/mod.rs
@@ -12465,6 +12465,34 @@ mod tests {
assert_eq!(a.value(1), "\x00 Foo");
}
+ #[test]
+ fn test_cast_sliced_binary_with_hidden_invalid_utf8() {
+ // The invalid first value is outside the slice, so it must not fail
the cast
+ let values: Vec<&[u8]> = vec![b"\xFF", b"a", b"b", b"c", b"d", b"e",
b"f"];
+ let arrays: [ArrayRef; 2] = [
+ Arc::new(BinaryArray::from(values.clone()).slice(1, 6)),
+ Arc::new(LargeBinaryArray::from(values).slice(1, 6)),
+ ];
+ let options = CastOptions {
+ safe: false,
+ format_options: FormatOptions::default(),
+ };
+ let expected = StringArray::from(vec!["a", "b", "c", "d", "e", "f"]);
+ for array in &arrays {
+ for to_type in [DataType::Utf8, DataType::LargeUtf8,
DataType::Utf8View] {
+ let result = cast_with_options(array, &to_type,
&options).unwrap();
+ assert_eq!(result.as_ref(), cast(&expected,
&to_type).unwrap().as_ref());
+ }
+
+ // A dictionary with fewer keys than half its values takes a
separate
+ // path to Utf8View
+ let keys = Int32Array::from(vec![5, 0]);
+ let dict = DictionaryArray::try_new(keys,
Arc::clone(array)).unwrap();
+ let result = cast_with_options(&dict, &DataType::Utf8View,
&options).unwrap();
+ assert_eq!(result.as_ref(), &StringViewArray::from(vec!["f",
"a"]));
+ }
+ }
+
#[test]
fn test_cast_utf8_to_timestamptz() {
let valid = StringArray::from(vec!["2023-01-01"]);
diff --git a/arrow-cast/src/cast/string.rs b/arrow-cast/src/cast/string.rs
index 8d6255f151..c193eacc6e 100644
--- a/arrow-cast/src/cast/string.rs
+++ b/arrow-cast/src/cast/string.rs
@@ -369,9 +369,11 @@ pub(crate) fn cast_binary_to_string<O: OffsetSizeTrait>(
Ok(a) => Ok(Arc::new(a)),
Err(e) => match cast_options.safe {
true => {
- // Fallback to slow method to convert invalid sequences to
nulls
- let mut builder =
- GenericStringBuilder::<O>::with_capacity(array.len(),
array.value_data().len());
+ // Fallback to slow method to convert invalid sequences to
nulls.
+ // Reserve only the bytes the offsets span, not the whole
values buffer.
+ let offsets = array.offsets();
+ let data_len = (offsets.last() - offsets.first()).as_usize();
+ let mut builder =
GenericStringBuilder::<O>::with_capacity(array.len(), data_len);
extend_valid_utf8(&mut builder, array.iter());
Ok(Arc::new(builder.finish()))
diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs
index 7404808a16..c2f32ef9d0 100644
--- a/arrow-data/src/data.rs
+++ b/arrow-data/src/data.rs
@@ -272,6 +272,19 @@ fn checked_len_plus_offset(
})
}
+/// Returns the first and last of `offsets`, and the bytes of `values` between
+/// them as a `str`. Returns `None` if the offsets are not valid indexes into
+/// `values` or those bytes are not valid UTF-8.
+fn utf8_span<'a, T: ArrowNativeType>(
+ offsets: &[T],
+ values: &'a [u8],
+) -> Option<(usize, usize, &'a str)> {
+ let first = offsets.first()?.to_usize()?;
+ let last = offsets.last()?.to_usize()?;
+ let values_str = std::str::from_utf8(values.get(first..last)?).ok()?;
+ Some((first, last, values_str))
+}
+
impl ArrayData {
/// Create a new ArrayData instance;
///
@@ -1696,18 +1709,24 @@ impl ArrayData {
}
/// Ensures that all strings formed by the offsets in `buffers[0]`
- /// into `buffers[1]` are valid utf8 sequences
+ /// into `buffers[1]` are valid utf8 sequences. Bytes of `buffers[1]`
+ /// outside the range of the offsets are not checked.
fn validate_utf8<T>(&self) -> Result<(), ArrowError>
where
T: ArrowNativeType + TryInto<usize> + num_traits::Num +
std::fmt::Display,
{
let values_buffer = &self.buffer_at(1)?.as_slice();
- if let Ok(values_str) = std::str::from_utf8(values_buffer) {
- // Validate Offsets are correct
+ let offsets = self.typed_offsets::<T>()?;
+ if let Some((first, last, values_str)) = utf8_span(offsets,
values_buffer) {
self.validate_each_offset::<T, _>(values_buffer.len(),
|string_index, range| {
- if !values_str.is_char_boundary(range.start)
- || !values_str.is_char_boundary(range.end)
- {
+ // `values_str` ends at the last offset. Checking each pair of
offsets doesn't
+ // show that `range.end <= last`: a later offset can still be
smaller, which
+ // `validate_each_offset` reports when it gets there.
+ if range.end > last {
+ return Ok(());
+ }
+ let (start, end) = (range.start - first, range.end - first);
+ if !values_str.is_char_boundary(start) ||
!values_str.is_char_boundary(end) {
return Err(ArrowError::InvalidArgumentError(format!(
"incomplete utf-8 byte sequence from index
{string_index}"
)));
@@ -1715,7 +1734,7 @@ impl ArrayData {
Ok(())
})
} else {
- // find specific offset that failed utf8 validation
+ // Find specific offset that failed utf8 validation
self.validate_each_offset::<T, _>(values_buffer.len(),
|string_index, range| {
std::str::from_utf8(&values_buffer[range.clone()]).map_err(|e|
{
ArrowError::InvalidArgumentError(format!(
diff --git a/arrow-string/src/substring.rs b/arrow-string/src/substring.rs
index 82171b5a1d..dfc10e099c 100644
--- a/arrow-string/src/substring.rs
+++ b/arrow-string/src/substring.rs
@@ -350,14 +350,17 @@ where
// When array is [Large]StringArray, we will check whether `offset` is at
a valid char boundary.
let check_char_boundary = {
- |offset: T::Offset| {
+ let first = array.offsets().first().as_usize();
+ let last = array.offsets().last().as_usize();
+ move |offset: T::Offset| {
if !matches!(T::DATA_TYPE, DataType::Utf8 | DataType::LargeUtf8) {
return Ok(offset);
}
- // Safety: a StringArray must contain valid UTF8 data
- let data_str = unsafe { std::str::from_utf8_unchecked(data) };
+ // Safety: in a StringArray, the bytes between the first and last
offsets
+ // are valid UTF-8. The bytes outside that range need not be.
+ let data_str = unsafe {
std::str::from_utf8_unchecked(&data[first..last]) };
let offset_usize = offset.as_usize();
- if data_str.is_char_boundary(offset_usize) {
+ if data_str.is_char_boundary(offset_usize - first) {
Ok(offset)
} else {
Err(ArrowError::ComputeError(format!(
@@ -1129,6 +1132,15 @@ mod tests {
assert!(err.contains("invalid utf-8 boundary"));
}
+ #[test]
+ fn check_invalid_bytes_outside_offsets() {
+ // The values buffer holds invalid UTF-8 before and after the visible
values
+ let values = Buffer::from_slice_ref(b"\xFFab\x80");
+ let array = StringArray::new(OffsetBuffer::new(vec![1, 3].into()),
values, None);
+ let result = substring(&array, 0, Some(5)).unwrap();
+ assert_eq!(result.as_string::<i32>(), &StringArray::from(vec!["ab"]));
+ }
+
#[test]
fn non_utf8_bytes() {
// non-utf8 bytes
