GH-51354: [C++][Parquet] Update bundled Apache Thrift to 0.24.0 - #51559
Open
Hanayoshi-8744 wants to merge 1 commit into
Open
Hanayoshi-8744 wants to merge 1 commit into
Hanayoshi-8744 wants to merge 1 commit into
Conversation
Update the bundled Apache Thrift dependency from 0.22.0 to 0.24.0 to address CVE-2026-55969, an integer overflow vulnerability in TTransport::checkReadBytesAvailable() that could bypass message size checks when reading maliciously crafted Thrift-encoded data (e.g. via Parquet file metadata parsed with the compact protocol). The Clang-only patch (thrift-3187.patch) that suppressed a gnu-zero-variadic-macro-arguments warning is removed, since the underlying fix (THRIFT-3268) has been merged upstream and is already included in Thrift 0.24.0; keeping the patch would cause `git apply`/ `patch` to fail and break the build. Verified: - Downloaded thrift-0.24.0.tar.gz and confirmed its SHA256 matches the official checksum published at downloads.apache.org. - Confirmed the CVE fix is present in Thrift 0.24.0 by diffing TProtocol.h/TCompactProtocol.h/TBinaryProtocol.h/TTransport.h against 0.22.0. - Confirmed thrift-3187.patch no longer applies cleanly against 0.24.0 (already fixed upstream). - Built Arrow C++ with -DARROW_PARQUET=ON -DARROW_BUILD_TESTS=ON -DThrift_SOURCE=BUNDLED and ran parquet-internals-test, parquet-file-deserialize-test, parquet-schema-test, parquet-reader-test, and parquet-writer-test; all passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
Author
|
Hi @wgtmac, this is my first contribution to Arrow. No rush at all, but I noticed the CI workflows for this PR seem to be stuck in "action_required" (likely pending approval for a first-time contributor). Could someone take a look when you have a chance? Thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The bundled Apache Thrift dependency (0.22.0) is affected by
CVE-2026-55969, an integer overflow vulnerability in
TTransport::checkReadBytesAvailable()that could bypass messagesize checks when reading maliciously crafted Thrift-encoded data.
Arrow's Parquet module relies on Thrift's compact protocol to
deserialize Parquet file metadata, so this is relevant to Arrow.
What changes are included in this PR?
cpp/thirdparty/versions.txt(including the SHA256 checksum).thrift-3187.patchand its application logicin
ThirdpartyToolchain.cmake. This patch pre-applied Thrift'supstream fix for THRIFT-3268 (a compiler warning), which has since
been merged into Thrift itself and is already included in 0.24.0.
Keeping the patch would make
git apply/patchfail during thebundled build with Clang.
Are these changes tested?
thrift-0.24.0.tar.gzdownloads correctly and itsSHA256 checksum matches the value published at
downloads.apache.org.
TProtocol.h,TCompactProtocol.h,TBinaryProtocol.h, andTTransport.hagainst 0.22.0.thrift-3187.patchno longer applies cleanly against0.24.0 sources (already fixed upstream), which is why it was
removed rather than kept as a no-op.
-DARROW_PARQUET=ON -DARROW_BUILD_TESTS=ON -DThrift_SOURCE=BUNDLEDand ranparquet-internals-test,parquet-file-deserialize-test,parquet-schema-test,parquet-reader-test, andparquet-writer-test. All tests passed.Are there any user-facing changes?
No.
Disclosure: this change was prepared with the assistance of an AI
coding tool (Claude Code). I reviewed the diff, verified the CVE fix
and checksum myself, and ran the test suite locally before opening
this PR.
🤖 Generated with Claude Code