From 3d4fea776cac3becadad5c1475f8d20614f539ad Mon Sep 17 00:00:00 2001 From: hanna Date: Sun, 12 Jul 2026 17:25:43 +0300 Subject: [PATCH 1/4] GH-50481: [C++] Fix CSV reader mis-parsing rows with an embedded NUL byte SSE42Filter::Matches used _mm_cmpistrc, an implicit-length SSE4.2 string-compare intrinsic that treats 0x00 as a terminator. Since it scans 8 raw CSV bytes at a time, a NUL byte embedded in a field could hide a real delimiter/quote/newline sharing the same 8-byte word, causing RunBulkFilter to blindly bulk-copy the word and silently mis-split the row. Switch to the explicit-length _mm_cmpestrc, passing the true lengths of both operands (sizeof(WordType) for the data word, sizeof(BulkFilterType) for the filter) instead of relying on NUL-termination. Verified locally: reverting just this fix reproduces the exact predicted failure ("Expected 64 columns, got 1") in the added regression test; with the fix, that test and the rest of the CSV test suite (272 tests) pass. This fix and its regression test were AI-generated (Claude), under human review and local verification. Co-Authored-By: Claude --- cpp/src/arrow/csv/lexing_internal.h | 7 ++++-- cpp/src/arrow/csv/parser_test.cc | 39 +++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/csv/lexing_internal.h b/cpp/src/arrow/csv/lexing_internal.h index b1da12750ac5..b919c3833393 100644 --- a/cpp/src/arrow/csv/lexing_internal.h +++ b/cpp/src/arrow/csv/lexing_internal.h @@ -133,8 +133,11 @@ class SSE42Filter { explicit SSE42Filter(const ParseOptions& options) : filter_(MakeFilter(options)) {} bool Matches(WordType w) const { - // Look up every byte in `w` in the SIMD filter. - return _mm_cmpistrc(_mm_set1_epi64x(w), filter_, + // Look up every byte in `w` in the SIMD filter. Use the explicit-length + // comparison since `w` may contain an embedded NUL byte, which the + // implicit-length _mm_cmpistrc would otherwise treat as a terminator. + return _mm_cmpestrc(_mm_set1_epi64x(w), sizeof(WordType), filter_, + sizeof(BulkFilterType), _SIDD_UBYTE_OPS | _SIDD_CMP_EQUAL_ANY); } diff --git a/cpp/src/arrow/csv/parser_test.cc b/cpp/src/arrow/csv/parser_test.cc index abe57dd2e89c..e809b8374cd0 100644 --- a/cpp/src/arrow/csv/parser_test.cc +++ b/cpp/src/arrow/csv/parser_test.cc @@ -936,5 +936,44 @@ TEST(BlockParser, RowNumberAppendedToError) { } } +TEST(BlockParser, EmbeddedNulInQuotedFieldAfterBulkFilterActivates) { + // Regression test for GH-50481. Once the running average value length + // crosses the bulk filter's activation threshold, a NUL byte embedded in a + // quoted field could hide the real closing quote if both landed in the + // same 8-byte SIMD word, causing the parser to keep consuming subsequent + // bytes as if still inside the quoted field. + constexpr int32_t num_cols = 64; + constexpr int32_t num_filler_rows = 512; // = kTargetChunkSize / num_cols + + std::string csv; + for (int32_t r = 0; r < num_filler_rows; ++r) { + for (int32_t c = 0; c < num_cols; ++c) { + if (c) csv += ','; + // 12 bytes/value, well above the bulk filter's 10-bytes/value + // activation threshold. + csv += "xxxxxxxxxxxx"; + } + csv += '\n'; + } + // This row's first field carries a real embedded NUL byte immediately + // before its closing quote - the byte pattern a misaligned + // implicit-length SIMD scan can hide. + csv += "\"abc"; + csv += '\0'; + csv += "def\""; + for (int32_t c = 1; c < num_cols; ++c) { + csv += ",xxxxxxxxxxxx"; + } + csv += '\n'; + + BlockParser parser(ParseOptions::Defaults()); + AssertParseFinal(parser, csv); + ASSERT_EQ(parser.num_rows(), num_filler_rows + 1); + + std::vector last_row; + GetLastRow(parser, &last_row); + ASSERT_EQ(last_row[0], std::string("abc\0def", 7)); +} + } // namespace csv } // namespace arrow From 576461bcfd8b78c7c091350fd8a707844ea2c150 Mon Sep 17 00:00:00 2001 From: Hanna Weissberg <93118219+HannaWeissberg@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:34:05 +0300 Subject: [PATCH 2/4] Adding int casting to sizeof() Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cpp/src/arrow/csv/lexing_internal.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/csv/lexing_internal.h b/cpp/src/arrow/csv/lexing_internal.h index b919c3833393..f23472fe629b 100644 --- a/cpp/src/arrow/csv/lexing_internal.h +++ b/cpp/src/arrow/csv/lexing_internal.h @@ -136,8 +136,8 @@ class SSE42Filter { // Look up every byte in `w` in the SIMD filter. Use the explicit-length // comparison since `w` may contain an embedded NUL byte, which the // implicit-length _mm_cmpistrc would otherwise treat as a terminator. - return _mm_cmpestrc(_mm_set1_epi64x(w), sizeof(WordType), filter_, - sizeof(BulkFilterType), + return _mm_cmpestrc(_mm_set1_epi64x(w), static_cast(sizeof(WordType)), filter_, + static_cast(sizeof(BulkFilterType)), _SIDD_UBYTE_OPS | _SIDD_CMP_EQUAL_ANY); } From 4b2dca0e166f1b9491d6e18c03b166837da2292e Mon Sep 17 00:00:00 2001 From: Hanna Weissberg <93118219+HannaWeissberg@users.noreply.github.com> Date: Sun, 12 Jul 2026 17:41:32 +0300 Subject: [PATCH 3/4] Adding full size assert check --- cpp/src/arrow/csv/parser_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/arrow/csv/parser_test.cc b/cpp/src/arrow/csv/parser_test.cc index e809b8374cd0..129498572e62 100644 --- a/cpp/src/arrow/csv/parser_test.cc +++ b/cpp/src/arrow/csv/parser_test.cc @@ -972,6 +972,7 @@ TEST(BlockParser, EmbeddedNulInQuotedFieldAfterBulkFilterActivates) { std::vector last_row; GetLastRow(parser, &last_row); + ASSERT_EQ(last_row.size(), static_cast(num_cols)); ASSERT_EQ(last_row[0], std::string("abc\0def", 7)); } From 92cb521393659dbde7579d744c7b1fea0f33a938 Mon Sep 17 00:00:00 2001 From: Hanna Weissberg <93118219+HannaWeissberg@users.noreply.github.com> Date: Sun, 12 Jul 2026 18:10:46 +0300 Subject: [PATCH 4/4] Fix test Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cpp/src/arrow/csv/parser_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/csv/parser_test.cc b/cpp/src/arrow/csv/parser_test.cc index 129498572e62..0abe50f484c0 100644 --- a/cpp/src/arrow/csv/parser_test.cc +++ b/cpp/src/arrow/csv/parser_test.cc @@ -966,7 +966,7 @@ TEST(BlockParser, EmbeddedNulInQuotedFieldAfterBulkFilterActivates) { } csv += '\n'; - BlockParser parser(ParseOptions::Defaults()); + BlockParser parser(ParseOptions::Defaults(), /*num_cols=*/num_cols); AssertParseFinal(parser, csv); ASSERT_EQ(parser.num_rows(), num_filler_rows + 1);