From e88b09c2d6a434d5fe348660f6c71a45498ba2a2 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 18 Oct 2026 11:14:26 -0400 Subject: [PATCH] Validate Array offsets when loading from the wire The ColumnArray constructor rejects decreasing offsets and a nested column whose size differs from the last offset, but ColumnArray::LoadBody checked neither. With decreasing offsets from the server, GetSize() and GetAsColumn() computed offsets[n] - offsets[n - 1] with unsigned wraparound; for Array(Nothing), whose Slice() keeps any length, a row then reported a length near 2^64. LoadBody now throws a ProtocolError on a decreasing offset and when the nested column does not hold exactly as many values as the last offset. When there are no rows or every array is empty it clears the stale nested values instead of returning early, so a reused column keeps that invariant too. --- clickhouse/columns/array.cpp | 16 +++++- ut/column_array_ut.cpp | 95 ++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/clickhouse/columns/array.cpp b/clickhouse/columns/array.cpp index fda893ef..cc204959 100644 --- a/clickhouse/columns/array.cpp +++ b/clickhouse/columns/array.cpp @@ -153,18 +153,32 @@ bool ColumnArray::LoadPrefix(InputStream* input, size_t rows) { bool ColumnArray::LoadBody(InputStream* input, size_t rows) { if (!rows) { + Clear(); return true; } if (!offsets_->LoadBody(input, rows)) { return false; } - const auto nested_rows = (*offsets_)[rows - 1]; + std::uint64_t nested_rows = 0; + for (size_t i = 0; i < rows; ++i) { + const auto offset = (*offsets_)[i]; + if (offset < nested_rows) { + throw ProtocolError("Array offsets must be non-decreasing: offset " + std::to_string(offset) + " at row " + std::to_string(i) + + " follows " + std::to_string(nested_rows)); + } + nested_rows = offset; + } if (nested_rows == 0) { + data_->Clear(); return true; } if (!data_->LoadBody(input, nested_rows)) { return false; } + if (data_->Size() != nested_rows) { + throw ProtocolError("Array nested column loaded " + std::to_string(data_->Size()) + " values, offsets expect " + + std::to_string(nested_rows)); + } return true; } diff --git a/ut/column_array_ut.cpp b/ut/column_array_ut.cpp index 21c5e4e0..49e4da07 100644 --- a/ut/column_array_ut.cpp +++ b/ut/column_array_ut.cpp @@ -281,6 +281,101 @@ TEST(ColumnArray, Slice) { EXPECT_ANY_THROW(untyped_array->Slice(0, values.size() + 1)); } +TEST(ColumnArray, Load) { + using namespace std::literals; + // Offsets 2, 2, 3 followed by three UInt8 values: [1, 2], [], [3]. + const auto data = + "\x02\x00\x00\x00\x00\x00\x00\x00" + "\x02\x00\x00\x00\x00\x00\x00\x00" + "\x03\x00\x00\x00\x00\x00\x00\x00" + "\x01\x02\x03"sv; + ArrayInput input(data.data(), data.size()); + auto array = CreateColumnByType("Array(UInt8)")->AsStrict(); + + ASSERT_TRUE(array->Load(&input, 3)); + ASSERT_EQ(3u, array->Size()); + EXPECT_TRUE(CompareRecursive(std::vector{1, 2}, *array->GetAsColumnTyped(0))); + EXPECT_EQ(0u, array->GetAsColumn(1)->Size()); + EXPECT_TRUE(CompareRecursive(std::vector{3}, *array->GetAsColumnTyped(2))); +} + +TEST(ColumnArray, Load_EmptyArrays) { + using namespace std::literals; + const auto data = + "\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00"sv; + ArrayInput input(data.data(), data.size()); + auto array = CreateColumnByType("Array(Nothing)")->AsStrict(); + + ASSERT_TRUE(array->Load(&input, 2)); + ASSERT_EQ(2u, array->Size()); + EXPECT_EQ(0u, array->GetAsColumn(0)->Size()); + EXPECT_EQ(0u, array->GetAsColumn(1)->Size()); +} + +TEST(ColumnArray, Load_ReplacesPreviousRows) { + using namespace std::literals; + // One row [1], then two empty rows, then no rows. + const auto data = + "\x01\x00\x00\x00\x00\x00\x00\x00" + "\x01" + "\x00\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00\x00\x00\x00\x00\x00\x00"sv; + ArrayInput input(data.data(), data.size()); + auto array = CreateColumnByType("Array(UInt8)")->AsStrict(); + + ASSERT_TRUE(array->Load(&input, 1)); + ASSERT_EQ(1u, array->GetData()->Size()); + + ASSERT_TRUE(array->Load(&input, 2)); + EXPECT_EQ(2u, array->Size()); + EXPECT_EQ(0u, array->GetData()->Size()); + + ASSERT_TRUE(array->Load(&input, 0)); + EXPECT_EQ(0u, array->Size()); + EXPECT_EQ(0u, array->GetData()->Size()); +} + +TEST(ColumnArray, Load_DecreasingOffsets) { + using namespace std::literals; + // Offsets 2, 1: the second row would span from offset 2 back to offset 1. + const auto data = + "\x02\x00\x00\x00\x00\x00\x00\x00" + "\x01\x00\x00\x00\x00\x00\x00\x00" + "\x00"sv; + ArrayInput input(data.data(), data.size()); + auto array = CreateColumnByType("Array(Nothing)"); + + EXPECT_THROW(array->Load(&input, 2), ProtocolError); +} + +TEST(ColumnArray, Load_FixedStringNestedSizeMismatch) { + using namespace std::literals; + // Offset 4 followed by 4 bytes; width * 4 wraps around size_t to 4, so the nested column holds 0 rows, not 4. + const auto data = + "\x04\x00\x00\x00\x00\x00\x00\x00" + "ABCD"sv; + ArrayInput input(data.data(), data.size()); + ColumnArray array(std::make_shared(static_cast(-1) / 4 + 2)); + + EXPECT_THROW(array.Load(&input, 1), ProtocolError); +} + +TEST(ColumnArray, Load_NestedSizeMismatch) { + using namespace std::literals; + // Nothing::LoadBody adds to the rows it already holds, so a second Load leaves more nested rows than the offsets cover. + const auto data = + "\x01\x00\x00\x00\x00\x00\x00\x00" + "\x00" + "\x02\x00\x00\x00\x00\x00\x00\x00" + "\x00\x00"sv; + ArrayInput input(data.data(), data.size()); + auto array = CreateColumnByType("Array(Nothing)"); + + ASSERT_TRUE(array->Load(&input, 1)); + EXPECT_THROW(array->Load(&input, 1), ProtocolError); +} + TEST(ColumnArray, Slice_2D) { // Verify that ColumnArray::Slice on 2D Array produces a 2D Array of proper type, size and contents. // Also check that slices can be of any size.