Repository navigation
feat: Validate binary view and string view values at NANOARROW_VALIDATION_LEVEL_FULL - #982
Draft
rustyconover wants to merge 3 commits into
Draft
rustyconover wants to merge 3 commits into
rustyconover wants to merge 3 commits into
Conversation
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) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #982 +/- ##
==========================================
- Coverage 78.30% 78.22% -0.08%
==========================================
Files 106 106
Lines 16919 17168 +249
Branches 2016 2018 +2
==========================================
+ Hits 13248 13430 +182
- Misses 2429 2495 +66
- Partials 1242 1243 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…TION_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) <noreply@anthropic.com>
rustyconover
force-pushed
the
feat/validate-views-full
branch
from
October 20, 2026 03:48
9982ef6 to
d9dce1d
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #981 (
feat/validate-utf8-full), which adds UTF-8 validation for string and large_string at FULL. This PR is stacked on it; only the last commit is new.NANOARROW_VALIDATION_LEVEL_FULLnever checked that a binary view or string view refers to bytes that exist. An out-of-line view from untrusted input could name a variadic buffer that doesn't exist, or a range outside its buffer, and FULL accepted it. Arrow C++'sValidateFull()rejects both.For every non-null view of a
binary_vieworstring_viewarray, FULL validation now checks:string_view, that the value is valid UTF-8, as the UTF-8 PR does for string and large_string.As in Arrow C++ (checked against pyarrow 25.0.1), null views are not checked. Arrow C++ also checks that an out-of-line view's 4-byte prefix matches its data and that an inline view's padding is zero. Neither affects memory safety, so they are left out of this PR. Errors name the view, e.g.
[1] Expected view buffer index >= 0 and < 1 but found 1or[1] Expected view range 1-21 to be within variadic buffer 0 of size 20.ArrowArrayFinishBuilding()validates through an internalArrowArrayView(ArrowArrayViewInitFromArray()) that did not point at the variadic buffers or their sizes. Nothing read them at any level before, so this was invisible. With these checks, though, every built view array with an out-of-line value would fail FULL withExpected view buffer index >= 0 and < 0 but found 0. It now points at them, asArrowArrayViewSetArray()does.ArrayTestFinishBuildingFullViewArraycovers this: it builds binary_view and string_view arrays that span several variadic buffers and finishes them at FULL. That test fails without the fix.Like the UTF-8 PR, FULL now rejects data it used to accept: views that point outside their buffers, and string views that aren't valid UTF-8.
Cost of FULL validation over 10 MB, compared with
mainand with Arrow C++ValidateFull()(pyarrow 25.0.1) on the same machine (Linux aarch64, Neoverse-N1, Release build).mainchecks nothing per view.mainTests:
ArrayViewTestValidateUtf8now also covers string_view, with values both inlined and out-of-line.ArrayViewTestValidateBinaryViewBoundscovers an out-of-range buffer index (too large and negative), a negative offset, a range past the end of the buffer, a negative size, and a null view with a bad index, which is not checked.ArrayTestFinishBuildingFullViewArrayis described above.ctestpasses (Linux aarch64, Debug, 872 tests), including under ASan/UBSan with clang (397 tests). The changed files are formatted with clang-format 16.0.6, the pre-commit version.This PR's code and description were written by AI (Claude Code) and reviewed by a human maintainer.
🤖 Generated with Claude Code