From af99348b644f9faae2ed33e2b7b56d270bdd2ede Mon Sep 17 00:00:00 2001 From: pedrobtz Date: Thu, 22 Oct 2026 00:00:25 +0200 Subject: [PATCH] fix(ipc): Reject fields that share a dictionary with different value types A dictionary batch is decoded with the value type of the first field that uses its id. A second field with the same id and another value type then viewed those buffers as its own type and read past them. Co-Authored-By: Claude Opus 5.5 --- src/nanoarrow/ipc/decoder.c | 40 +++++++++++++++++++++ src/nanoarrow/ipc/decoder_test.cc | 58 +++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/src/nanoarrow/ipc/decoder.c b/src/nanoarrow/ipc/decoder.c index 80e53ec8d..0b95e9eae 100644 --- a/src/nanoarrow/ipc/decoder.c +++ b/src/nanoarrow/ipc/decoder.c @@ -432,6 +432,27 @@ static void ArrowIpcDictionariesNoOpSchemaRelease(struct ArrowSchema* schema) { NANOARROW_UNUSED(schema); } +// Whether two value types are the same: their formats, children and dictionaries +// (names, flags and metadata are not part of the type) +static int ArrowIpcDictionaryValueTypesEqual(const struct ArrowSchema* lhs, + const struct ArrowSchema* rhs) { + if (lhs == NULL || rhs == NULL) { + return lhs == rhs; + } + + if (strcmp(lhs->format, rhs->format) != 0 || lhs->n_children != rhs->n_children) { + return 0; + } + + for (int64_t i = 0; i < lhs->n_children; i++) { + if (!ArrowIpcDictionaryValueTypesEqual(lhs->children[i], rhs->children[i])) { + return 0; + } + } + + return ArrowIpcDictionaryValueTypesEqual(lhs->dictionary, rhs->dictionary); +} + static ArrowErrorCode ArrowIpcDictionariesInitDictionaries( struct ArrowIpcDictionariesPrivate* private_data, const struct ArrowIpcDictionaryEncodings* dictionary_encodings, @@ -474,6 +495,25 @@ static ArrowErrorCode ArrowIpcDictionariesInitDictionaries( return EINVAL; } + // A dictionary batch is decoded with one value type, so every field that shares + // this dictionary must have that value type + const struct ArrowIpcDictionaryEncoding* encodings = + (const struct ArrowIpcDictionaryEncoding*)dictionary_encodings->encodings.data; + int64_t n_encodings = dictionary_encodings->encodings.size_bytes / + sizeof(struct ArrowIpcDictionaryEncoding); + for (int64_t j = 0; j < n_encodings; j++) { + if (encodings[j].id == unique_ids[i] && + !ArrowIpcDictionaryValueTypesEqual(encodings[j].schema->dictionary, + encoding->schema->dictionary)) { + ArrowErrorSet(error, + "Expected fields that share dictionary with id %" PRId64 + " to have the same value type", + unique_ids[i]); + *num_initialized_decoders_out = i + 1; + return EINVAL; + } + } + // The decoder's schema will be a struct with one column (the value type of // the dictionary-encoded field) decoder_schema.children = (struct ArrowSchema**)&encoding->schema->dictionary; diff --git a/src/nanoarrow/ipc/decoder_test.cc b/src/nanoarrow/ipc/decoder_test.cc index 8f89acc6f..efc347905 100644 --- a/src/nanoarrow/ipc/decoder_test.cc +++ b/src/nanoarrow/ipc/decoder_test.cc @@ -371,6 +371,64 @@ TEST(NanoarrowIpcTest, NanoarrowIpcCheckHeader) { ArrowIpcDecoderReset(&decoder); } +TEST(NanoarrowIpcTest, NanoarrowIpcSharedDictionaryValueTypes) { + struct ArrowError error; + + // Two dictionary-encoded fields that will share dictionary id 0 + nanoarrow::UniqueSchema schema; + ArrowSchemaInit(schema.get()); + ASSERT_EQ(ArrowSchemaSetTypeStruct(schema.get(), 2), NANOARROW_OK); + for (int64_t i = 0; i < 2; i++) { + ASSERT_EQ(ArrowSchemaSetType(schema->children[i], NANOARROW_TYPE_INT32), + NANOARROW_OK); + ASSERT_EQ(ArrowSchemaAllocateDictionary(schema->children[i]), NANOARROW_OK); + } + ASSERT_EQ( + ArrowSchemaInitFromType(schema->children[0]->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); + ASSERT_EQ( + ArrowSchemaInitFromType(schema->children[1]->dictionary, NANOARROW_TYPE_STRING), + NANOARROW_OK); + + auto encodings_with_shared_id = [&](nanoarrow::ipc::UniqueDictionaryEncodings* out) { + ArrowIpcDictionaryEncodingsInit(out->get()); + for (int64_t i = 0; i < 2; i++) { + struct ArrowIpcDictionaryEncoding encoding; + encoding.id = 0; + encoding.kind = NANOARROW_IPC_DICTIONARY_KIND_DENSE_ARRAY; + encoding.schema = schema->children[i]; + ASSERT_EQ(ArrowIpcDictionaryEncodingsAppend(out->get(), encoding), NANOARROW_OK); + } + }; + + // Sharing a dictionary is fine when the value types match + { + nanoarrow::ipc::UniqueDictionaryEncodings encodings; + encodings_with_shared_id(&encodings); + nanoarrow::ipc::UniqueDictionaries dictionaries; + EXPECT_EQ(ArrowIpcDictionariesInit(dictionaries.get(), encodings.get(), &error), + NANOARROW_OK) + << error.message; + } + + // ...but not when they differ: the dictionary would be decoded as one type and + // read as the other + ArrowSchemaRelease(schema->children[1]->dictionary); + ASSERT_EQ(ArrowSchemaInitFromType(schema->children[1]->dictionary, + NANOARROW_TYPE_LARGE_STRING), + NANOARROW_OK); + { + nanoarrow::ipc::UniqueDictionaryEncodings encodings; + encodings_with_shared_id(&encodings); + nanoarrow::ipc::UniqueDictionaries dictionaries; + EXPECT_EQ(ArrowIpcDictionariesInit(dictionaries.get(), encodings.get(), &error), + EINVAL); + EXPECT_STREQ(error.message, + "Expected fields that share dictionary with id 0 to have the same " + "value type"); + } +} + TEST(NanoarrowIpcTest, NanoarrowIpcPeekSimpleSchema) { struct ArrowIpcDecoder decoder; struct ArrowError error;