From e8c4865c02c5cd3fa9903d4f63e7b7ef91fcf98a Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 15:28:53 -0400 Subject: [PATCH 1/5] feat: Validate UTF-8 at NANOARROW_VALIDATION_LEVEL_FULL NANOARROW_VALIDATION_LEVEL_FULL is meant for untrusted input, but it did not check that string values are valid UTF-8, which Arrow C++'s ValidateFull() does. FULL validation now checks that every non-null string and large_string value is valid UTF-8 (RFC 3629: no overlong encodings, surrogates, code points above U+10FFFF, stray continuation bytes or truncated sequences). As in Arrow C++, values are checked one at a time (a code point split across two values is invalid) and null values are not checked. ASCII is skipped eight bytes at a time, and the referenced bytes are first checked as one contiguous range plus a code point boundary check at each offset, so that short values are not validated one call at a time. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/nanoarrow/common/array.c | 158 +++++++++++++++++++++++++++ src/nanoarrow/common/array_test.cc | 164 ++++++++++++++++++++++++++++ src/nanoarrow/common/inline_types.h | 2 + 3 files changed, 324 insertions(+) diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index ee308ea61..642c7d45d 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -2123,6 +2123,155 @@ static int ArrowAssertInt8In(struct ArrowBufferView view, const int8_t* values, return NANOARROW_OK; } +// Returns the length of the longest prefix of data that is valid UTF-8 (RFC 3629), so +// data is valid UTF-8 if and only if the result is size_bytes. Overlong encodings, +// surrogates (U+D800 to U+DFFF), code points above U+10FFFF, stray continuation bytes +// and truncated sequences are all invalid. +static int64_t ArrowUtf8ValidPrefix(const uint8_t* data, int64_t size_bytes) { + int64_t i = 0; + while (i < size_bytes) { + const uint8_t lead = data[i]; + + if (lead < 0x80) { + // After an ASCII byte, skip any further ASCII eight bytes at a time + i++; + while (size_bytes - i >= 8) { + uint64_t word; + memcpy(&word, data + i, sizeof(word)); + if ((word & 0x8080808080808080ULL) != 0) { + break; + } + i += 8; + } + continue; + } + + if (lead < 0xE0) { + // Two bytes (U+0080 to U+07FF); 0xC0 and 0xC1 would be overlong + if (lead < 0xC2 || size_bytes - i < 2 || (data[i + 1] & 0xC0) != 0x80) { + return i; + } + i += 2; + continue; + } + + if (lead < 0xF0) { + // Three bytes (U+0800 to U+FFFF), excluding overlong encodings (0xE0 followed + // by less than 0xA0) and surrogates (0xED followed by 0xA0 or more) + if (size_bytes - i < 3) { + return i; + } + const uint8_t second = data[i + 1]; + const uint8_t second_min = lead == 0xE0 ? 0xA0 : 0x80; + const uint8_t second_max = lead == 0xED ? 0x9F : 0xBF; + if (second < second_min || second > second_max || (data[i + 2] & 0xC0) != 0x80) { + return i; + } + i += 3; + continue; + } + + // Four bytes (U+10000 to U+10FFFF), excluding overlong encodings (0xF0 followed by + // less than 0x90) and code points above U+10FFFF (0xF4 followed by 0x90 or more, or + // a lead byte above 0xF4) + if (lead > 0xF4 || size_bytes - i < 4) { + return i; + } + const uint8_t second = data[i + 1]; + const uint8_t second_min = lead == 0xF0 ? 0x90 : 0x80; + const uint8_t second_max = lead == 0xF4 ? 0x8F : 0xBF; + if (second < second_min || second > second_max || (data[i + 2] & 0xC0) != 0x80 || + (data[i + 3] & 0xC0) != 0x80) { + return i; + } + i += 4; + } + + return size_bytes; +} + +static int64_t ArrowArrayViewGetOffsetUnsafe(const struct ArrowArrayView* array_view, + int64_t i) { + if (array_view->layout.element_size_bits[1] == 32) { + return ArrowBufferViewGetInt32Unsafe(&array_view->buffer_views[1], i); + } else { + return ArrowBufferViewGetInt64Unsafe(&array_view->buffer_views[1], i); + } +} + +static int ArrowArrayViewValidateUtf8Element(const struct ArrowArrayView* array_view, + int64_t i, const uint8_t* value, + int64_t size_bytes, + struct ArrowError* error) { + if (size_bytes == 0) { + return NANOARROW_OK; + } + + int64_t valid_bytes = ArrowUtf8ValidPrefix(value, size_bytes); + if (valid_bytes != size_bytes) { + ArrowErrorSet(error, + "[%" PRId64 + "] Expected valid UTF-8 %s value but found invalid " + "sequence at byte %" PRId64, + i, ArrowTypeString(array_view->storage_type), valid_bytes); + return EINVAL; + } + + return NANOARROW_OK; +} + +// Check that every non-null value of a string or large string array is valid UTF-8. +// Like Arrow C++, values are checked one at a time (a code point split across two +// values is invalid) and the bytes of null values are not checked. +static int ArrowArrayViewValidateUtf8(const struct ArrowArrayView* array_view, + struct ArrowError* error) { + // The offsets buffer of a zero-length array may be empty + if (array_view->length == 0) { + return NANOARROW_OK; + } + + const uint8_t* data = array_view->buffer_views[2].data.as_uint8; + + // The values (including those of null slots) are the contiguous range of bytes + // between the first and last offsets. If that whole range is valid and every value + // begins on a code point boundary (i.e., not with a continuation byte), every value is + // valid, which avoids validating short values one at a time. Otherwise, check the + // non-null values one at a time to find (and report) the first invalid one. + { + int64_t first = ArrowArrayViewGetOffsetUnsafe(array_view, array_view->offset); + int64_t last = ArrowArrayViewGetOffsetUnsafe(array_view, + array_view->offset + array_view->length); + if (first == last) { + return NANOARROW_OK; + } + + int ok = ArrowUtf8ValidPrefix(data + first, last - first) == last - first; + for (int64_t i = 1; ok && i < array_view->length; i++) { + int64_t start = ArrowArrayViewGetOffsetUnsafe(array_view, array_view->offset + i); + ok = start == last || (data[start] & 0xC0) != 0x80; + } + + if (ok) { + return NANOARROW_OK; + } + } + + for (int64_t i = 0; i < array_view->length; i++) { + if (ArrowArrayViewIsNull(array_view, i)) { + continue; + } + + int64_t start = ArrowArrayViewGetOffsetUnsafe(array_view, array_view->offset + i); + int64_t end = ArrowArrayViewGetOffsetUnsafe(array_view, array_view->offset + i + 1); + if (end > start) { + NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateUtf8Element( + array_view, i, data + start, end - start, error)); + } + } + + return NANOARROW_OK; +} + static int ArrowArrayViewValidateFull(struct ArrowArrayView* array_view, struct ArrowError* error) { for (int i = 0; i < NANOARROW_MAX_FIXED_BUFFERS; i++) { @@ -2152,6 +2301,15 @@ static int ArrowArrayViewValidateFull(struct ArrowArrayView* array_view, } } + switch (array_view->storage_type) { + case NANOARROW_TYPE_STRING: + case NANOARROW_TYPE_LARGE_STRING: + NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateUtf8(array_view, error)); + break; + default: + break; + } + if (array_view->storage_type == NANOARROW_TYPE_DENSE_UNION || array_view->storage_type == NANOARROW_TYPE_SPARSE_UNION) { struct ArrowBufferView sliced_type_ids; diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc index 31f442708..55b597fd4 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -20,7 +20,10 @@ #include #include #include +#include #include +#include +#include #if defined(NANOARROW_BUILD_TESTS_WITH_ARROW) #include @@ -3018,6 +3021,167 @@ TEST(ArrayTest, ArrayViewTestValidateUnalignedOffsets) { } } +// A value of a string or binary test array, or a null (kNullValue) +struct TestStringValue { + TestStringValue(const char* value) : is_null(false), value(value) {} + TestStringValue(std::string value) : is_null(false), value(std::move(value)) {} + TestStringValue() : is_null(true) {} + + bool is_null; + std::string value; +}; + +static const TestStringValue kNullValue; + +// Build an array of type from values, slice it, and validate it at level. On failure, +// *message holds the error message. +static ArrowErrorCode ValidateStringValues(enum ArrowType type, + const std::vector& values, + enum ArrowValidationLevel level, + std::string* message, int64_t offset = 0, + int64_t length = -1) { + nanoarrow::UniqueArray array; + nanoarrow::UniqueArrayView array_view; + struct ArrowError error; + error.message[0] = '\0'; + + NANOARROW_RETURN_NOT_OK(ArrowArrayInitFromType(array.get(), type)); + NANOARROW_RETURN_NOT_OK(ArrowArrayStartAppending(array.get())); + for (const auto& value : values) { + if (value.is_null) { + NANOARROW_RETURN_NOT_OK(ArrowArrayAppendNull(array.get(), 1)); + } else { + struct ArrowStringView sv = {value.value.data(), + static_cast(value.value.size())}; + NANOARROW_RETURN_NOT_OK(ArrowArrayAppendString(array.get(), sv)); + } + } + NANOARROW_RETURN_NOT_OK( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_NONE, &error)); + + array->offset = offset; + array->length = length == -1 ? array->length - offset : length; + + ArrowArrayViewInitFromType(array_view.get(), type); + NANOARROW_RETURN_NOT_OK(ArrowArrayViewSetArray(array_view.get(), array.get(), &error)); + ArrowErrorCode result = ArrowArrayViewValidate(array_view.get(), level, &error); + *message = error.message; + return result; +} + +TEST(ArrayTest, ArrayViewTestValidateUtf8) { + std::string message; + const std::string long_ascii(40, 'a'); + + for (const auto type : {NANOARROW_TYPE_STRING, NANOARROW_TYPE_LARGE_STRING}) { + SCOPED_TRACE(ArrowTypeString(type)); + const std::string type_name = ArrowTypeString(type); + auto invalid_message = [&](int64_t i, int64_t byte) { + return "[" + std::to_string(i) + "] Expected valid UTF-8 " + type_name + + " value but found invalid sequence at byte " + std::to_string(byte); + }; + + // Valid ASCII and two-, three- and four-byte sequences + EXPECT_EQ( + ValidateStringValues(type, + {"", "abc", "caf\xc3\xa9", "\xe2\x82\xac", + "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", "\xef\xbf\xbf", + long_ascii + "\xc3\xa9" + long_ascii, kNullValue}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + NANOARROW_OK) + << message; + + // Each kind of invalid sequence + const std::vector> invalid = { + {"\x80", 0}, // Stray continuation byte + {"ab\xc3", 2}, // Truncated two-byte sequence + {"\xe2\x82", 0}, // Truncated three-byte sequence + {"\xf0\x9f\x98", 0}, // Truncated four-byte sequence + {"\xc3\x28", 0}, // Invalid continuation byte + {"\xc0\x80", 0}, // Overlong two-byte encoding + {"\xc1\xbf", 0}, // Overlong two-byte encoding + {"\xe0\x80\x80", 0}, // Overlong three-byte encoding + {"\xf0\x80\x80\x80", 0}, // Overlong four-byte encoding + {"\xed\xa0\x80", 0}, // Surrogate (U+D800) + {"\xed\xbf\xbf", 0}, // Surrogate (U+DFFF) + {"\xf4\x90\x80\x80", 0}, // Above U+10FFFF + {"\xf5\x80\x80\x80", 0}, // Above U+10FFFF + {"\xff", 0}, // Never valid + {"abcdefg\xffhijklmno", 7}, // Within an eight-byte word + {long_ascii + "\xff", 40}, // After the ASCII fast path + {long_ascii + "\xc3\xa9\xa9", 42}}; // After a valid multi-byte sequence + for (const auto& item : invalid) { + SCOPED_TRACE(item.second); + EXPECT_EQ(ValidateStringValues(type, {"abc", item.first}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + EINVAL); + EXPECT_EQ(message, invalid_message(1, item.second)); + + // ...and with a null in the array + EXPECT_EQ(ValidateStringValues(type, {kNullValue, item.first}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + EINVAL); + EXPECT_EQ(message, invalid_message(1, item.second)); + + // Invalid UTF-8 is still valid below NANOARROW_VALIDATION_LEVEL_FULL + EXPECT_EQ(ValidateStringValues(type, {"abc", item.first}, + NANOARROW_VALIDATION_LEVEL_DEFAULT, &message), + NANOARROW_OK); + } + + // Values are checked one at a time: a code point split across two values is + // invalid even though their concatenation is valid + EXPECT_EQ(ValidateStringValues(type, {"a\xc3", "\xa9"}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + EINVAL); + EXPECT_EQ(message, invalid_message(0, 1)); + + // The bytes of a null value are not checked + EXPECT_EQ(ValidateStringValues(type, {"abc", "\xff", long_ascii + "\xff", "def"}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + EINVAL); + nanoarrow::UniqueArray with_nulls; + ASSERT_EQ(ArrowArrayInitFromType(with_nulls.get(), type), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(with_nulls.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(with_nulls.get(), "abc"_asv), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(with_nulls.get(), "\xff"_asv), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(with_nulls.get(), + {long_ascii.data(), (int64_t)long_ascii.size()}), + NANOARROW_OK); + ASSERT_EQ(ArrowArrayFinishBuildingDefault(with_nulls.get(), nullptr), NANOARROW_OK); + // Invalidate the third value and mark the second and third values null + ASSERT_EQ(ArrowBitmapReserve(ArrowArrayValidityBitmap(with_nulls.get()), 3), + NANOARROW_OK); + ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(with_nulls.get()), 1, 1); + ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(with_nulls.get()), 0, 2); + with_nulls->null_count = 2; + ArrowArrayBuffer(with_nulls.get(), 2)->data[4] = 0xff; + struct ArrowError error; + EXPECT_EQ(ArrowArrayFinishBuilding(with_nulls.get(), NANOARROW_VALIDATION_LEVEL_FULL, + &error), + NANOARROW_OK) + << error.message; + + // Only the values within a slice are checked, and indices are relative to it + EXPECT_EQ(ValidateStringValues(type, {"\xff", "abc", "def", "\xff"}, + NANOARROW_VALIDATION_LEVEL_FULL, &message, 1, 2), + NANOARROW_OK) + << message; + EXPECT_EQ(ValidateStringValues(type, {"\xff", "abc", "\xff", "def"}, + NANOARROW_VALIDATION_LEVEL_FULL, &message, 1, 2), + EINVAL); + EXPECT_EQ(message, invalid_message(1, 0)); + } + + // Binary values are not checked + for (const auto type : {NANOARROW_TYPE_BINARY, NANOARROW_TYPE_LARGE_BINARY}) { + EXPECT_EQ(ValidateStringValues(type, {"\xff", long_ascii + "\xff"}, + NANOARROW_VALIDATION_LEVEL_FULL, &message), + NANOARROW_OK) + << message; + } +} + TEST(ArrayTest, ArrayViewTestLargeString) { struct ArrowArrayView array_view; struct ArrowError error; diff --git a/src/nanoarrow/common/inline_types.h b/src/nanoarrow/common/inline_types.h index 5737d6012..5b1eb1e55 100644 --- a/src/nanoarrow/common/inline_types.h +++ b/src/nanoarrow/common/inline_types.h @@ -617,6 +617,8 @@ enum ArrowValidationLevel { /// \brief Validate all buffer sizes and all buffer content. This is useful in the /// context of untrusted input or input that may have been corrupted in transit. + /// This includes checking that every non-null string and large string value is + /// valid UTF-8. NANOARROW_VALIDATION_LEVEL_FULL = 3 }; From 89edbf031a459f0293771656ac031ff67cf6f0db Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 23:42:29 -0400 Subject: [PATCH 2/5] test: Avoid a codespell false positive in the UTF-8 validation test Co-Authored-By: Claude Opus 5.5 (1M context) --- src/nanoarrow/common/array_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc index 55b597fd4..180abf400 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -3084,7 +3084,7 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { // Valid ASCII and two-, three- and four-byte sequences EXPECT_EQ( ValidateStringValues(type, - {"", "abc", "caf\xc3\xa9", "\xe2\x82\xac", + {"", "abc", "\xc3\xa9t\xc3\xa9", "\xe2\x82\xac", "\xf0\x9f\x98\x80", "\xf4\x8f\xbf\xbf", "\xef\xbf\xbf", long_ascii + "\xc3\xa9" + long_ascii, kNullValue}, NANOARROW_VALIDATION_LEVEL_FULL, &message), From 1b4e1a65ef4c7f4780e22e6d9d5247a9aa20e6ad Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 26 Oct 2026 12:00:29 -0400 Subject: [PATCH 3/5] docs: Clarify UTF-8 validation source and null handling --- src/nanoarrow/common/array.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index 642c7d45d..ad5edc1e8 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -2123,7 +2123,9 @@ static int ArrowAssertInt8In(struct ArrowBufferView view, const int8_t* values, return NANOARROW_OK; } -// Returns the length of the longest prefix of data that is valid UTF-8 (RFC 3629), so +// The byte ranges below implement the grammar in RFC 3629, Section 4: +// https://www.rfc-editor.org/rfc/rfc3629.html#section-4 +// Returns the length of the longest prefix of data that is valid UTF-8, so // data is valid UTF-8 if and only if the result is size_bytes. Overlong encodings, // surrogates (U+D800 to U+DFFF), code points above U+10FFFF, stray continuation bytes // and truncated sequences are all invalid. @@ -2221,8 +2223,9 @@ static int ArrowArrayViewValidateUtf8Element(const struct ArrowArrayView* array_ } // Check that every non-null value of a string or large string array is valid UTF-8. -// Like Arrow C++, values are checked one at a time (a code point split across two -// values is invalid) and the bytes of null values are not checked. +// A code point split across two values is invalid. The fast path also checks bytes +// in null slots, but invalid UTF-8 in a null slot does not fail validation: the +// fallback checks non-null values individually. static int ArrowArrayViewValidateUtf8(const struct ArrowArrayView* array_view, struct ArrowError* error) { // The offsets buffer of a zero-length array may be empty From cf92c66627825db982e6611eef3eb482fc815583 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 15:32:00 -0400 Subject: [PATCH 4/5] feat: Validate binary view and string view values at NANOARROW_VALIDATION_LEVEL_FULL FULL validation never checked that a binary view or string view refers to bytes that exist, so an out-of-line value from untrusted input could point outside its variadic buffer (or at a buffer that doesn't exist). Arrow C++'s ValidateFull() checks this. FULL validation now checks, for every non-null view of a binary_view or string_view array, that its size is non-negative and that an out-of-line view's buffer index and byte range are within its variadic buffer. For string_view, it then checks that the value is valid UTF-8, as it already does for string and large_string. As in Arrow C++, null views are not checked. ArrowArrayFinishBuilding() validates through an internal ArrowArrayView that did not point at the variadic buffers or their sizes, so these checks would have rejected every built view array with an out-of-line value. It now points at them, as ArrowArrayViewSetArray() does. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/nanoarrow/common/array.c | 86 ++++++++++++++++++++ src/nanoarrow/common/array_test.cc | 119 ++++++++++++++++++++++++++-- src/nanoarrow/common/inline_types.h | 5 +- 3 files changed, 203 insertions(+), 7 deletions(-) diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c index ad5edc1e8..11979c696 100644 --- a/src/nanoarrow/common/array.c +++ b/src/nanoarrow/common/array.c @@ -943,6 +943,15 @@ static ArrowErrorCode ArrowArrayViewInitFromArray(struct ArrowArrayView* array_v array_view->buffer_views[2].data.as_uint8 = private_data->buffers[1].data; array_view->buffer_views[2].size_bytes = private_data->buffers[1].size_bytes; + // As in ArrowArrayViewSetArray(), point at the variadic buffers and their sizes so + // that the values of a binary view or string view array can be validated + if (private_data->storage_type == NANOARROW_TYPE_STRING_VIEW || + private_data->storage_type == NANOARROW_TYPE_BINARY_VIEW) { + array_view->n_variadic_buffers = private_data->n_variadic_buffers; + array_view->variadic_buffers = array->buffers + NANOARROW_BINARY_VIEW_FIXED_BUFFERS; + array_view->variadic_buffer_sizes = (int64_t*)array->buffers[array->n_buffers - 1]; + } + int result = ArrowArrayViewAllocateChildren(array_view, array->n_children); if (result != NANOARROW_OK) { ArrowArrayViewReset(array_view); @@ -2275,6 +2284,79 @@ static int ArrowArrayViewValidateUtf8(const struct ArrowArrayView* array_view, return NANOARROW_OK; } +// Check that every non-null view of a binary view or string view array refers to bytes +// that exist (and, for a string view, that they are valid UTF-8). The bytes of an +// out-of-line value can't be checked without this, and Arrow C++ performs the same +// checks. Null views are not checked. +static int ArrowArrayViewValidateBinaryViews(const struct ArrowArrayView* array_view, + struct ArrowError* error) { + const int is_string = array_view->storage_type == NANOARROW_TYPE_STRING_VIEW; + const int64_t n_variadic_buffers = array_view->n_variadic_buffers; + + for (int64_t i = 0; i < array_view->length; i++) { + if (ArrowArrayViewIsNull(array_view, i)) { + continue; + } + + union ArrowBinaryView view; + memcpy(&view, + array_view->buffer_views[1].data.as_uint8 + + (array_view->offset + i) * (int64_t)sizeof(union ArrowBinaryView), + sizeof(view)); + + const int64_t size_bytes = view.inlined.size; + if (size_bytes < 0) { + ArrowErrorSet(error, "[%" PRId64 "] Expected view size >= 0 but found %" PRId64, i, + size_bytes); + return EINVAL; + } + + if (size_bytes <= NANOARROW_BINARY_VIEW_INLINE_SIZE) { + if (is_string) { + NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateUtf8Element( + array_view, i, view.inlined.data, size_bytes, error)); + } + continue; + } + + const int64_t buffer_index = view.ref.buffer_index; + if (buffer_index < 0 || buffer_index >= n_variadic_buffers) { + ArrowErrorSet(error, + "[%" PRId64 "] Expected view buffer index >= 0 and < %" PRId64 + " but found %" PRId64, + i, n_variadic_buffers, buffer_index); + return EINVAL; + } + + if (array_view->variadic_buffer_sizes == NULL) { + ArrowErrorSet(error, "Expected variadic buffer sizes for %" PRId64 " buffer(s)", + n_variadic_buffers); + return EINVAL; + } + + int64_t buffer_size; + memcpy(&buffer_size, array_view->variadic_buffer_sizes + buffer_index, + sizeof(buffer_size)); + + const int64_t offset = view.ref.offset; + if (offset < 0 || offset + size_bytes > buffer_size) { + ArrowErrorSet(error, + "[%" PRId64 "] Expected view range %" PRId64 "-%" PRId64 + " to be within variadic buffer %" PRId64 " of size %" PRId64, + i, offset, offset + size_bytes, buffer_index, buffer_size); + return EINVAL; + } + + if (is_string) { + const uint8_t* buffer = (const uint8_t*)array_view->variadic_buffers[buffer_index]; + NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateUtf8Element( + array_view, i, buffer + offset, size_bytes, error)); + } + } + + return NANOARROW_OK; +} + static int ArrowArrayViewValidateFull(struct ArrowArrayView* array_view, struct ArrowError* error) { for (int i = 0; i < NANOARROW_MAX_FIXED_BUFFERS; i++) { @@ -2309,6 +2391,10 @@ static int ArrowArrayViewValidateFull(struct ArrowArrayView* array_view, case NANOARROW_TYPE_LARGE_STRING: NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateUtf8(array_view, error)); break; + case NANOARROW_TYPE_BINARY_VIEW: + case NANOARROW_TYPE_STRING_VIEW: + NANOARROW_RETURN_NOT_OK(ArrowArrayViewValidateBinaryViews(array_view, error)); + break; default: break; } diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc index 180abf400..3331e6724 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -3073,7 +3073,8 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { std::string message; const std::string long_ascii(40, 'a'); - for (const auto type : {NANOARROW_TYPE_STRING, NANOARROW_TYPE_LARGE_STRING}) { + for (const auto type : + {NANOARROW_TYPE_STRING, NANOARROW_TYPE_LARGE_STRING, NANOARROW_TYPE_STRING_VIEW}) { SCOPED_TRACE(ArrowTypeString(type)); const std::string type_name = ArrowTypeString(type); auto invalid_message = [&](int64_t i, int64_t byte) { @@ -3081,7 +3082,8 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { " value but found invalid sequence at byte " + std::to_string(byte); }; - // Valid ASCII and two-, three- and four-byte sequences + // Valid ASCII and two-, three- and four-byte sequences, both short (inlined in a + // view) and long (out-of-line) EXPECT_EQ( ValidateStringValues(type, {"", "abc", "\xc3\xa9t\xc3\xa9", "\xe2\x82\xac", @@ -3109,7 +3111,7 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { {"\xff", 0}, // Never valid {"abcdefg\xffhijklmno", 7}, // Within an eight-byte word {long_ascii + "\xff", 40}, // After the ASCII fast path - {long_ascii + "\xc3\xa9\xa9", 42}}; // After a valid multi-byte sequence + {long_ascii + "\xc3\xa9\xa9", 42}}; // Out-of-line in a view for (const auto& item : invalid) { SCOPED_TRACE(item.second); EXPECT_EQ(ValidateStringValues(type, {"abc", item.first}, @@ -3155,7 +3157,13 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(with_nulls.get()), 1, 1); ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(with_nulls.get()), 0, 2); with_nulls->null_count = 2; - ArrowArrayBuffer(with_nulls.get(), 2)->data[4] = 0xff; + if (type == NANOARROW_TYPE_STRING_VIEW) { + uint8_t* out_of_line = ArrowArrayBuffer(with_nulls.get(), 2)->data; + out_of_line[0] = 0xff; + } else { + uint8_t* data = ArrowArrayBuffer(with_nulls.get(), 2)->data; + data[4] = 0xff; + } struct ArrowError error; EXPECT_EQ(ArrowArrayFinishBuilding(with_nulls.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), @@ -3174,7 +3182,8 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { } // Binary values are not checked - for (const auto type : {NANOARROW_TYPE_BINARY, NANOARROW_TYPE_LARGE_BINARY}) { + for (const auto type : + {NANOARROW_TYPE_BINARY, NANOARROW_TYPE_LARGE_BINARY, NANOARROW_TYPE_BINARY_VIEW}) { EXPECT_EQ(ValidateStringValues(type, {"\xff", long_ascii + "\xff"}, NANOARROW_VALIDATION_LEVEL_FULL, &message), NANOARROW_OK) @@ -3182,6 +3191,106 @@ TEST(ArrayTest, ArrayViewTestValidateUtf8) { } } +TEST(ArrayTest, ArrayTestFinishBuildingFullViewArray) { + // ArrowArrayFinishBuilding() validates through an internal ArrowArrayView, which must + // see the variadic buffers to validate out-of-line values at + // NANOARROW_VALIDATION_LEVEL_FULL. Append enough out-of-line values to need more than + // one variadic buffer. + const std::string value = "longer than 12 bytes"; + for (const auto type : {NANOARROW_TYPE_BINARY_VIEW, NANOARROW_TYPE_STRING_VIEW}) { + SCOPED_TRACE(ArrowTypeString(type)); + nanoarrow::UniqueArray array; + struct ArrowError error; + ASSERT_EQ(ArrowArrayInitFromType(array.get(), type), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(array.get(), "short"_asv), NANOARROW_OK); + for (int i = 0; i < 4000; i++) { + ASSERT_EQ( + ArrowArrayAppendString(array.get(), {value.data(), (int64_t)value.size()}), + NANOARROW_OK); + } + ASSERT_GT(ArrowArrayVariadicBufferCount(array.get()), 1); + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + NANOARROW_OK) + << error.message; + } +} + +TEST(ArrayTest, ArrayViewTestValidateBinaryViewBounds) { + for (const auto type : {NANOARROW_TYPE_BINARY_VIEW, NANOARROW_TYPE_STRING_VIEW}) { + SCOPED_TRACE(ArrowTypeString(type)); + nanoarrow::UniqueArray array; + struct ArrowError error; + ASSERT_EQ(ArrowArrayInitFromType(array.get(), type), NANOARROW_OK); + ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(array.get(), "short"_asv), NANOARROW_OK); + ASSERT_EQ(ArrowArrayAppendString(array.get(), "longer than 12 bytes"_asv), + NANOARROW_OK); + ASSERT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + NANOARROW_OK) + << error.message; + + auto* views = + reinterpret_cast(ArrowArrayBuffer(array.get(), 1)->data); + const union ArrowBinaryView good = views[1]; + + views[1].ref.buffer_index = 1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_DEFAULT, &error), + NANOARROW_OK); + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + EINVAL); + EXPECT_STREQ(error.message, + "[1] Expected view buffer index >= 0 and < 1 but found 1"); + + views[1].ref.buffer_index = -1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + EINVAL); + EXPECT_STREQ(error.message, + "[1] Expected view buffer index >= 0 and < 1 but found -1"); + views[1] = good; + + views[1].ref.offset = 1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + EINVAL); + EXPECT_STREQ( + error.message, + "[1] Expected view range 1-21 to be within variadic buffer 0 of size 20"); + + views[1].ref.offset = -1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + EINVAL); + EXPECT_STREQ( + error.message, + "[1] Expected view range -1-19 to be within variadic buffer 0 of size 20"); + views[1] = good; + + views[0].inlined.size = -1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + EINVAL); + EXPECT_STREQ(error.message, "[0] Expected view size >= 0 but found -1"); + views[0].inlined.size = 5; + + // A null view is not checked + views[1].ref.buffer_index = 7; + ASSERT_EQ(ArrowBitmapReserve(ArrowArrayValidityBitmap(array.get()), 2), NANOARROW_OK); + ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(array.get()), 1, 1); + ArrowBitmapAppendUnsafe(ArrowArrayValidityBitmap(array.get()), 0, 1); + array->null_count = 1; + EXPECT_EQ( + ArrowArrayFinishBuilding(array.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), + NANOARROW_OK) + << error.message; + } +} + TEST(ArrayTest, ArrayViewTestLargeString) { struct ArrowArrayView array_view; struct ArrowError error; diff --git a/src/nanoarrow/common/inline_types.h b/src/nanoarrow/common/inline_types.h index 5b1eb1e55..ab4a846dd 100644 --- a/src/nanoarrow/common/inline_types.h +++ b/src/nanoarrow/common/inline_types.h @@ -617,8 +617,9 @@ enum ArrowValidationLevel { /// \brief Validate all buffer sizes and all buffer content. This is useful in the /// context of untrusted input or input that may have been corrupted in transit. - /// This includes checking that every non-null string and large string value is - /// valid UTF-8. + /// This includes checking that every non-null string, large string and string view + /// value is valid UTF-8 and that every non-null binary view or string view refers + /// to bytes within its variadic buffer. NANOARROW_VALIDATION_LEVEL_FULL = 3 }; From 0fd9951ff214b5b04c1cbaa06927a669312bb86b Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 26 Oct 2026 12:20:55 -0400 Subject: [PATCH 5/5] test: Avoid GCC bounds false positives in overflow tests --- src/nanoarrow/common/array_test.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc index 3331e6724..1ae9e6fb1 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -1321,7 +1321,10 @@ TEST(ArrayTest, ArrayTestAppendToBinaryArrayErrors) { struct ArrowBufferView item; item.data.as_char = ""; item.size_bytes = static_cast(INT_MAX) + 1; - EXPECT_EQ(ArrowArrayAppendBytes(&array, item), EOVERFLOW); + // Overflow must be rejected before accessing item.data. Prevent inlining so + // GCC does not diagnose an oversized memcpy in the other storage-type paths. + auto* volatile append_bytes = &ArrowArrayAppendBytes; + EXPECT_EQ(append_bytes(&array, item), EOVERFLOW); ArrowArrayRelease(&array); } @@ -1334,7 +1337,10 @@ TEST(ArrayTest, ArrayTestAppendToBinaryViewArrayErrors) { struct ArrowBufferView item; item.data.as_char = ""; item.size_bytes = static_cast(INT32_MAX) + 1; - EXPECT_EQ(ArrowArrayAppendBytes(&array, item), EOVERFLOW); + // Overflow must be rejected before accessing item.data. Prevent inlining so + // GCC does not diagnose an oversized memcpy in the other storage-type paths. + auto* volatile append_bytes = &ArrowArrayAppendBytes; + EXPECT_EQ(append_bytes(&array, item), EOVERFLOW); ArrowArrayRelease(&array); }