paleolimbot commented on code in PR #964:
URL: https://github.com/apache/arrow-nanoarrow/pull/964#discussion_r4295794239
##########
src/nanoarrow/ipc/reader_test.cc:
##########
@@ -586,6 +590,70 @@ TEST(NanoarrowIpcReader, StreamReaderDictionary) {
ArrowArrayStreamRelease(&stream);
}
+TEST(NanoarrowIpcReader, StreamReaderValidationLevel) {
+ // Point the last dictionary index past the end of the three-value
dictionary.
+ // Only full validation inspects index values, so a lower level accepts the
batch.
+ uint8_t record_batch[sizeof(kDictionaryRecordBatch)];
+ memcpy(record_batch, kDictionaryRecordBatch, sizeof(kDictionaryRecordBatch));
+ ASSERT_EQ(record_batch[sizeof(record_batch) - 6], 0x02);
+ record_batch[sizeof(record_batch) - 6] = 0x07;
+
+ for (auto validation_level :
+ {NANOARROW_VALIDATION_LEVEL_FULL, NANOARROW_VALIDATION_LEVEL_DEFAULT}) {
+ SCOPED_TRACE(validation_level);
+ struct ArrowBuffer input_buffer;
+ ArrowBufferInit(&input_buffer);
+ ASSERT_EQ(
+ ArrowBufferAppend(&input_buffer, kDictionarySchema,
sizeof(kDictionarySchema)),
+ NANOARROW_OK);
+ ASSERT_EQ(
+ ArrowBufferAppend(&input_buffer, kDictionaryBatch,
sizeof(kDictionaryBatch)),
+ NANOARROW_OK);
+ ASSERT_EQ(ArrowBufferAppend(&input_buffer, record_batch,
sizeof(record_batch)),
+ NANOARROW_OK);
+
+ struct ArrowIpcInputStream input;
+ ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, &input_buffer),
NANOARROW_OK);
+
+ struct ArrowArrayStream stream;
+ struct ArrowIpcArrayStreamReaderOptions options;
+ options.field_index = -1;
+ options.use_shared_buffers = ArrowSharedBufferIsThreadSafe();
+ options.validation_level = validation_level;
+ ASSERT_EQ(ArrowIpcArrayStreamReaderInit(&stream, &input, &options),
NANOARROW_OK);
+
+ struct ArrowArray array;
+ if (validation_level == NANOARROW_VALIDATION_LEVEL_FULL) {
+ ASSERT_EQ(ArrowArrayStreamGetNext(&stream, &array, nullptr), EINVAL);
+ EXPECT_NE(std::string(ArrowArrayStreamGetLastError(&stream))
+ .find("Expected dictionary index >= 0 and < 3 but found
value 7"),
+ std::string::npos);
+ } else {
+ ASSERT_EQ(ArrowArrayStreamGetNext(&stream, &array, nullptr),
NANOARROW_OK);
+ EXPECT_EQ(array.length, 3);
+ ArrowArrayRelease(&array);
+ }
+
+ ArrowArrayStreamRelease(&stream);
+ }
+}
+
+TEST(NanoarrowIpcReader, StreamReaderInvalidValidationLevel) {
+ struct ArrowBuffer input_buffer;
+ ArrowBufferInit(&input_buffer);
+ struct ArrowIpcInputStream input;
+ ASSERT_EQ(ArrowIpcInputStreamInitBuffer(&input, &input_buffer),
NANOARROW_OK);
+
+ struct ArrowArrayStream stream;
+ struct ArrowIpcArrayStreamReaderOptions options;
+ options.field_index = -1;
+ options.use_shared_buffers = 0;
+ options.validation_level = static_cast<ArrowValidationLevel>(4);
+ ASSERT_EQ(ArrowIpcArrayStreamReaderInit(&stream, &input, &options), EINVAL);
Review Comment:
clang-tidy objects to `4` here since it's an invalid value. I think you can
add a clang-tidy ignore line, but if not, you can also remove this test.
##########
src/nanoarrow/ipc/reader.c:
##########
@@ -573,6 +574,12 @@ static const char* ArrowIpcArrayStreamReaderGetLastError(
ArrowErrorCode ArrowIpcArrayStreamReaderInit(
struct ArrowArrayStream* out, struct ArrowIpcInputStream* input_stream,
struct ArrowIpcArrayStreamReaderOptions* options) {
+ if (options != NULL &&
+ ((int)options->validation_level < (int)NANOARROW_VALIDATION_LEVEL_NONE ||
+ (int)options->validation_level > (int)NANOARROW_VALIDATION_LEVEL_FULL))
{
+ return EINVAL;
+ }
Review Comment:
a `switch()` is what we do in other places for enum validation and I think
is just as compact here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
