You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by bk...@apache.org on 2024/09/20 13:03:04 UTC
(arrow-nanoarrow) branch main updated: fix: Improve validation of offset buffers for sliced arrays (#626)
This is an automated email from the ASF dual-hosted git repository.
bkietz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new 5d537386 fix: Improve validation of offset buffers for sliced arrays (#626)
5d537386 is described below
commit 5d53738636cac9d6bf14aebf791a123814a00697
Author: Dewey Dunnington <de...@dunnington.ca>
AuthorDate: Fri Sep 20 08:03:00 2024 -0500
fix: Improve validation of offset buffers for sliced arrays (#626)
This PR updates validation of offset buffers in string/binary, large
string/binary, list, and large list types such that portions of the
buffers not strictly required by the slice referenced by `offset` and
`length` are not accessed/validated. This came up in the IPC integration
tests because C# exports full buffers rather than the portions of
buffers strictly required.
This highlights how our test suite would benefit from helpers to reduce
repetition (although I'd prefer to handle that at a later time if
possible).
---------
Co-authored-by: Benjamin Kietzman <be...@gmail.com>
---
.github/workflows/integration.yaml | 1 +
docker-compose.yml | 2 +-
src/nanoarrow/common/array.c | 49 ++++++++--
src/nanoarrow/common/array_test.cc | 182 ++++++++++++++++++++++++++++++++++---
4 files changed, 211 insertions(+), 23 deletions(-)
diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml
index 8a2e9d63..0d7e3e87 100644
--- a/.github/workflows/integration.yaml
+++ b/.github/workflows/integration.yaml
@@ -25,6 +25,7 @@ on:
pull_request:
paths:
- .github/workflows/integration.yaml
+ - ci/docker/integration.dockerfile
- docker-compose.yml
schedule:
- cron: '5 0 * * 0'
diff --git a/docker-compose.yml b/docker-compose.yml
index 2fb86c93..bd651cb7 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -54,7 +54,7 @@ services:
volumes:
- ${NANOARROW_DOCKER_SOURCE_DIR}:/arrow-integration/nanoarrow
environment:
- ARCHERY_INTEGRATION_TARGET_LANGUAGES: "nanoarrow"
+ ARCHERY_INTEGRATION_TARGET_IMPLEMENTATIONS: "nanoarrow"
command:
["echo '::group::Build nanoarrow' &&
conda run --no-capture-output /arrow-integration/ci/scripts/nanoarrow_build.sh /arrow-integration /build &&
diff --git a/src/nanoarrow/common/array.c b/src/nanoarrow/common/array.c
index 6446d4b0..72cb5203 100644
--- a/src/nanoarrow/common/array.c
+++ b/src/nanoarrow/common/array.c
@@ -990,7 +990,7 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
case NANOARROW_TYPE_STRING:
case NANOARROW_TYPE_BINARY:
if (array_view->buffer_views[1].size_bytes != 0) {
- first_offset = array_view->buffer_views[1].data.as_int32[0];
+ first_offset = array_view->buffer_views[1].data.as_int32[array_view->offset];
if (first_offset < 0) {
ArrowErrorSet(error, "Expected first offset >= 0 but found %" PRId64,
first_offset);
@@ -998,6 +998,11 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
}
last_offset = array_view->buffer_views[1].data.as_int32[offset_plus_length];
+ if (last_offset < 0) {
+ ArrowErrorSet(error, "Expected last offset >= 0 but found %" PRId64,
+ last_offset);
+ return EINVAL;
+ }
// If the data buffer size is unknown, assign it; otherwise, check it
if (array_view->buffer_views[2].size_bytes == -1) {
@@ -1021,7 +1026,7 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
case NANOARROW_TYPE_LARGE_STRING:
case NANOARROW_TYPE_LARGE_BINARY:
if (array_view->buffer_views[1].size_bytes != 0) {
- first_offset = array_view->buffer_views[1].data.as_int64[0];
+ first_offset = array_view->buffer_views[1].data.as_int64[array_view->offset];
if (first_offset < 0) {
ArrowErrorSet(error, "Expected first offset >= 0 but found %" PRId64,
first_offset);
@@ -1029,6 +1034,11 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
}
last_offset = array_view->buffer_views[1].data.as_int64[offset_plus_length];
+ if (last_offset < 0) {
+ ArrowErrorSet(error, "Expected last offset >= 0 but found %" PRId64,
+ last_offset);
+ return EINVAL;
+ }
// If the data buffer size is unknown, assign it; otherwise, check it
if (array_view->buffer_views[2].size_bytes == -1) {
@@ -1065,7 +1075,7 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
case NANOARROW_TYPE_LIST:
case NANOARROW_TYPE_MAP:
if (array_view->buffer_views[1].size_bytes != 0) {
- first_offset = array_view->buffer_views[1].data.as_int32[0];
+ first_offset = array_view->buffer_views[1].data.as_int32[array_view->offset];
if (first_offset < 0) {
ArrowErrorSet(error, "Expected first offset >= 0 but found %" PRId64,
first_offset);
@@ -1073,6 +1083,12 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
}
last_offset = array_view->buffer_views[1].data.as_int32[offset_plus_length];
+ if (last_offset < 0) {
+ ArrowErrorSet(error, "Expected last offset >= 0 but found %" PRId64,
+ last_offset);
+ return EINVAL;
+ }
+
if (array_view->children[0]->length < last_offset) {
ArrowErrorSet(error,
"Expected child of %s array to have length >= %" PRId64
@@ -1087,7 +1103,7 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
case NANOARROW_TYPE_LARGE_LIST:
if (array_view->buffer_views[1].size_bytes != 0) {
- first_offset = array_view->buffer_views[1].data.as_int64[0];
+ first_offset = array_view->buffer_views[1].data.as_int64[array_view->offset];
if (first_offset < 0) {
ArrowErrorSet(error, "Expected first offset >= 0 but found %" PRId64,
first_offset);
@@ -1095,6 +1111,12 @@ static int ArrowArrayViewValidateDefault(struct ArrowArrayView* array_view,
}
last_offset = array_view->buffer_views[1].data.as_int64[offset_plus_length];
+ if (last_offset < 0) {
+ ArrowErrorSet(error, "Expected last offset >= 0 but found %" PRId64,
+ last_offset);
+ return EINVAL;
+ }
+
if (array_view->children[0]->length < last_offset) {
ArrowErrorSet(error,
"Expected child of large list array to have length >= %" PRId64
@@ -1249,13 +1271,24 @@ static int ArrowArrayViewValidateFull(struct ArrowArrayView* array_view,
struct ArrowError* error) {
for (int i = 0; i < NANOARROW_MAX_FIXED_BUFFERS; i++) {
switch (array_view->layout.buffer_type[i]) {
+ // Only validate the portion of the buffer that is strictly required,
+ // which includes not validating the offset buffer of a zero-length array.
case NANOARROW_BUFFER_TYPE_DATA_OFFSET:
+ if (array_view->length == 0) {
+ continue;
+ }
if (array_view->layout.element_size_bits[i] == 32) {
- NANOARROW_RETURN_NOT_OK(
- ArrowAssertIncreasingInt32(array_view->buffer_views[i], error));
+ struct ArrowBufferView offset_minimal;
+ offset_minimal.data.as_int32 =
+ array_view->buffer_views[i].data.as_int32 + array_view->offset;
+ offset_minimal.size_bytes = (array_view->length + 1) * sizeof(int32_t);
+ NANOARROW_RETURN_NOT_OK(ArrowAssertIncreasingInt32(offset_minimal, error));
} else {
- NANOARROW_RETURN_NOT_OK(
- ArrowAssertIncreasingInt64(array_view->buffer_views[i], error));
+ struct ArrowBufferView offset_minimal;
+ offset_minimal.data.as_int64 =
+ array_view->buffer_views[i].data.as_int64 + array_view->offset;
+ offset_minimal.size_bytes = (array_view->length + 1) * sizeof(int64_t);
+ NANOARROW_RETURN_NOT_OK(ArrowAssertIncreasingInt64(offset_minimal, error));
}
break;
default:
diff --git a/src/nanoarrow/common/array_test.cc b/src/nanoarrow/common/array_test.cc
index dec7366f..eb770d4f 100644
--- a/src/nanoarrow/common/array_test.cc
+++ b/src/nanoarrow/common/array_test.cc
@@ -2138,6 +2138,10 @@ TEST(ArrayTest, ArrayViewTestString) {
EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
EXPECT_EQ(array_view.buffer_views[2].size_bytes, 0);
+ // This should pass validation even if all buffers are empty
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayViewSetLength(&array_view, 5);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 1);
EXPECT_EQ(array_view.buffer_views[1].size_bytes, (5 + 1) * sizeof(int32_t));
@@ -2192,21 +2196,54 @@ TEST(ArrayTest, ArrayViewTestString) {
offsets[0] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
+ // For a sliced array, this can still pass validation
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
+ // Check for last element >= 0
+ array.offset = 0;
+ array.length = array.length + 1;
offsets[0] = 0;
+ offsets[1] = 1;
+ offsets[2] = -1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected last offset >= 0 but found -1");
+
+ // ...but the array should be valid if we make it one element shorter
+ array.length = array.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+ // Check for negative element sizes
+ array.length = array.length + 1;
+ offsets[0] = 0;
offsets[1] = -1;
+ offsets[2] = 2;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
+ // Sliced array should also fail validation because the first element is negative
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
// Check sequential offsets whose diff causes overflow
- offsets[1] = 2080374784;
- offsets[2] = INT_MIN;
+ array.offset = 0;
+ array.length = array.length + 1;
+ offsets[0] = 2080374784;
+ offsets[1] = INT_MIN;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
- EXPECT_STREQ(error.message, "[2] Expected element size >= 0");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
ArrowArrayRelease(&array);
ArrowArrayViewReset(&array_view);
@@ -2232,6 +2269,10 @@ TEST(ArrayTest, ArrayViewTestLargeString) {
EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
EXPECT_EQ(array_view.buffer_views[2].size_bytes, 0);
+ // This should pass validation even if all buffers are empty
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayViewSetLength(&array_view, 5);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 1);
EXPECT_EQ(array_view.buffer_views[1].size_bytes, (5 + 1) * sizeof(int64_t));
@@ -2261,20 +2302,22 @@ TEST(ArrayTest, ArrayViewTestLargeString) {
EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
EXPECT_EQ(array_view.buffer_views[2].size_bytes, 0);
- // Build non-zero length (the array ["abcd"])
+ // Build non-zero length (the array ["abcd", "efg"])
ASSERT_EQ(ArrowBufferAppendInt64(ArrowArrayBuffer(&array, 1), 0), NANOARROW_OK);
ASSERT_EQ(ArrowBufferAppendInt64(ArrowArrayBuffer(&array, 1), 4), NANOARROW_OK);
- ASSERT_EQ(ArrowBufferReserve(ArrowArrayBuffer(&array, 2), 4), NANOARROW_OK);
+ ASSERT_EQ(ArrowBufferAppendInt64(ArrowArrayBuffer(&array, 1), 7), NANOARROW_OK);
+ ASSERT_EQ(ArrowBufferReserve(ArrowArrayBuffer(&array, 2), 7), NANOARROW_OK);
ArrowBufferAppendUnsafe(ArrowArrayBuffer(&array, 2), "abcd", 4);
- array.length = 1;
+ ArrowBufferAppendUnsafe(ArrowArrayBuffer(&array, 2), "efg", 3);
+ array.length = 2;
ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array, nullptr), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
NANOARROW_OK);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 0);
- EXPECT_EQ(array_view.buffer_views[1].size_bytes, (1 + 1) * sizeof(int64_t));
- EXPECT_EQ(array_view.buffer_views[2].size_bytes, 4);
+ EXPECT_EQ(array_view.buffer_views[1].size_bytes, (1 + 2) * sizeof(int64_t));
+ EXPECT_EQ(array_view.buffer_views[2].size_bytes, 7);
// Expect error for offsets that will cause bad access
int64_t* offsets =
@@ -2283,14 +2326,45 @@ TEST(ArrayTest, ArrayViewTestLargeString) {
offsets[0] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
+ // For a sliced array, this can still pass validation
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
+ // Check for last element >= 0
+ array.offset = 0;
+ array.length = array.length + 1;
offsets[0] = 0;
+ offsets[1] = 1;
+ offsets[2] = -1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected last offset >= 0 but found -1");
+ // ...but the array should be valid if we make it one element shorter
+ array.length = array.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
+ // Check for negative element sizes
+ array.length = array.length + 1;
+ offsets[0] = 0;
offsets[1] = -1;
+ offsets[2] = 2;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
+ // Sliced array should also fail validation because the first element is negative
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
ArrowArrayRelease(&array);
ArrowArrayViewReset(&array_view);
}
@@ -2330,6 +2404,7 @@ TEST(ArrayTest, ArrayViewTestStruct) {
TEST(ArrayTest, ArrayViewTestList) {
struct ArrowArrayView array_view;
+ struct ArrowError error;
ArrowArrayViewInitFromType(&array_view, NANOARROW_TYPE_LIST);
EXPECT_EQ(array_view.array, nullptr);
@@ -2344,11 +2419,20 @@ TEST(ArrayTest, ArrayViewTestList) {
ArrowArrayViewInitFromType(array_view.children[0], NANOARROW_TYPE_INT32);
EXPECT_EQ(array_view.children[0]->storage_type, NANOARROW_TYPE_INT32);
+ // Can't assume the offsets buffer exists for length == 0
+ ArrowArrayViewSetLength(&array_view, 0);
+ EXPECT_EQ(array_view.buffer_views[0].size_bytes, 0);
+ EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
+
+ // This should pass validation even if all buffers are empty
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayViewSetLength(&array_view, 5);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 1);
EXPECT_EQ(array_view.buffer_views[1].size_bytes, (5 + 1) * sizeof(int32_t));
- // Build a valid array
+ // Build a valid array ([[1234], []])
struct ArrowArray array;
ASSERT_EQ(ArrowArrayInitFromType(&array, NANOARROW_TYPE_LIST), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAllocateChildren(&array, 1), NANOARROW_OK);
@@ -2357,6 +2441,7 @@ TEST(ArrayTest, ArrayViewTestList) {
ASSERT_EQ(ArrowArrayStartAppending(&array), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 1234), NANOARROW_OK);
ASSERT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);
ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array, nullptr), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, nullptr), NANOARROW_OK);
@@ -2364,21 +2449,50 @@ TEST(ArrayTest, ArrayViewTestList) {
NANOARROW_OK);
// Expect error for offsets that will cause bad access
- struct ArrowError error;
int32_t* offsets =
const_cast<int32_t*>(reinterpret_cast<const int32_t*>(array.buffers[1]));
offsets[0] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
- offsets[0] = 0;
+ // For a sliced array, this can still pass validation
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
+ // Check for negative element sizes
+ array.offset = 0;
+ array.length = array.length + 1;
+ offsets[0] = 0;
offsets[1] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
+ // Sliced array should also fail validation because the first element is negative
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
+ // Check for last element >= 0
+ array.offset = 0;
+ array.length = array.length + 1;
+ offsets[1] = 1;
+ offsets[2] = -1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected last offset >= 0 but found -1");
+
+ // ...but the array should be valid if we make it one element shorter
+ array.length = 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayRelease(&array);
ArrowArrayViewReset(&array_view);
}
@@ -2451,6 +2565,7 @@ TEST(ArrayTest, ArrayViewTestLargeListGet) {
TEST(ArrayTest, ArrayViewTestLargeList) {
struct ArrowArrayView array_view;
+ struct ArrowError error;
ArrowArrayViewInitFromType(&array_view, NANOARROW_TYPE_LARGE_LIST);
EXPECT_EQ(array_view.array, nullptr);
@@ -2465,11 +2580,20 @@ TEST(ArrayTest, ArrayViewTestLargeList) {
ArrowArrayViewInitFromType(array_view.children[0], NANOARROW_TYPE_INT32);
EXPECT_EQ(array_view.children[0]->storage_type, NANOARROW_TYPE_INT32);
+ // Can't assume the offsets buffer exists for length == 0
+ ArrowArrayViewSetLength(&array_view, 0);
+ EXPECT_EQ(array_view.buffer_views[0].size_bytes, 0);
+ EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
+
+ // This should pass validation even if all buffers are empty
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayViewSetLength(&array_view, 5);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 1);
EXPECT_EQ(array_view.buffer_views[1].size_bytes, (5 + 1) * sizeof(int64_t));
- // Build a valid array
+ // Build a valid array ([[1234], []])
struct ArrowArray array;
ASSERT_EQ(ArrowArrayInitFromType(&array, NANOARROW_TYPE_LARGE_LIST), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAllocateChildren(&array, 1), NANOARROW_OK);
@@ -2478,6 +2602,7 @@ TEST(ArrayTest, ArrayViewTestLargeList) {
ASSERT_EQ(ArrowArrayStartAppending(&array), NANOARROW_OK);
ASSERT_EQ(ArrowArrayAppendInt(array.children[0], 1234), NANOARROW_OK);
ASSERT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);
+ ASSERT_EQ(ArrowArrayFinishElement(&array), NANOARROW_OK);
ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array, nullptr), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, nullptr), NANOARROW_OK);
@@ -2485,21 +2610,50 @@ TEST(ArrayTest, ArrayViewTestLargeList) {
NANOARROW_OK);
// Expect error for offsets that will cause bad access
- struct ArrowError error;
int64_t* offsets =
const_cast<int64_t*>(reinterpret_cast<const int64_t*>(array.buffers[1]));
offsets[0] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
- offsets[0] = 0;
+ // For a sliced array, this can still pass validation
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
+ // Check for negative element sizes
+ array.offset = 0;
+ array.length = array.length + 1;
+ offsets[0] = 0;
offsets[1] = -1;
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
+ // Sliced array should also fail validation because the first element is negative
+ array.offset = 1;
+ array.length = array_view.length - 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected first offset >= 0 but found -1");
+
+ // Check for last element >= 0
+ array.offset = 0;
+ array.length = array.length + 1;
+ offsets[1] = 1;
+ offsets[2] = -1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), EINVAL);
+ EXPECT_STREQ(error.message, "Expected last offset >= 0 but found -1");
+
+ // ...but the array should be valid if we make it one element shorter
+ array.length = 1;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view, NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ NANOARROW_OK);
+
ArrowArrayRelease(&array);
ArrowArrayViewReset(&array_view);
}