From f91d0eb553259c244ea3a555f7f86ce2b3df8ce0 Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 15:28:53 -0400 Subject: [PATCH 1/3] 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 e4538955c..887ef720b 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 9324cdde6..fb3282f82 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 @@ -3015,6 +3018,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 1c1cd0bbc..f0b4ae826 100644 --- a/src/nanoarrow/common/inline_types.h +++ b/src/nanoarrow/common/inline_types.h @@ -601,6 +601,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 718d6a6609bc8a1fb7a6a52474a25655025a6a8d Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 23:42:29 -0400 Subject: [PATCH 2/3] 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 fb3282f82..37b9deaa3 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -3081,7 +3081,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 d9dce1d860370bd652ec6011a2adfb0dc12798cd Mon Sep 17 00:00:00 2001 From: Rusty Conover Date: Mon, 19 Oct 2026 15:32:00 -0400 Subject: [PATCH 3/3] 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 887ef720b..fc843b86f 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); @@ -2272,6 +2281,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++) { @@ -2306,6 +2388,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 37b9deaa3..8ced51149 100644 --- a/src/nanoarrow/common/array_test.cc +++ b/src/nanoarrow/common/array_test.cc @@ -3070,7 +3070,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) { @@ -3078,7 +3079,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", @@ -3106,7 +3108,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}, @@ -3152,7 +3154,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), @@ -3171,7 +3179,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) @@ -3179,6 +3188,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 f0b4ae826..1e570d980 100644 --- a/src/nanoarrow/common/inline_types.h +++ b/src/nanoarrow/common/inline_types.h @@ -601,8 +601,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 };