Repository navigation
feat(core): add safe decoding helpers for Parquet footers - #423
Open
prudhvimaharishi wants to merge 2 commits into
Open
prudhvimaharishi wants to merge 2 commits into
prudhvimaharishi wants to merge 2 commits into
Conversation
This was referenced Oct 27, 2026
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces robust, safe decoding helpers for Parquet footers to improve security and reliability when processing untrusted data. By implementing strict bounds on Thrift message structures and providing utilities for dictionary encoding validation and schema fingerprinting, the changes ensure that the system can safely handle potentially malicious or malformed Parquet files without risking memory or stack overflows. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #423 +/- ##
==========================================
- Coverage 98.47% 98.44% -0.03%
==========================================
Files 79 82 +3
Lines 3922 3977 +55
Branches 457 472 +15
==========================================
+ Hits 3862 3915 +53
Misses 32 32
- Partials 28 30 +2
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces several new utility classes to safely handle Parquet metadata and protocols, including BoundedCompactProtocol to prevent stack/heap exhaustion, ParquetDictionaryEncoding to check dictionary encoding, and ParquetSchemaFingerprint to compute stable schema fingerprints, along with their respective unit tests. The review feedback is highly constructive, focusing on aligning the new code with the repository's style guide by strictly applying @nonnull annotations, using Guava's Preconditions.checkNotNull for boundary input validation, and adding defensive checks to prevent nesting depth underflow or null pointer exceptions.
Collaborator
📊 Hermetic JMH Benchmark Summary (PR vs Main)PR: Published Assets: 🌐 Interactive HTML Report | 📄 Markdown Report | 📦 Raw JMH JSON | 🔬 Kokoro / Sponge Artifacts Suite-Level Executive Summary
📋 Full Hermetic Benchmark Breakdown (45 cases)
|
prudhvimaharishi
force-pushed
the
parquet_footer_decoding
branch
2 times, most recently
from
November 2, 2026 12:13
258b9c3 to
4544d0d
Compare
Footers come from untrusted objects, so decoding them must never exhaust the heap or the stack, and decisions taken from them must be conservative. - BoundedCompactProtocol caps strings, binary values, and containers at the message length and limits how deep an unknown value can nest when it is skipped, so crafted footers fail with a TException instead of an OutOfMemoryError or StackOverflowError. It relies on Thrift's own limits and only adds the binary length check that Thrift lacks. - ParquetDictionaryEncoding decides whether a chunk's dictionary page can filter the chunk on its own, from page encoding stats or, like parquet-mr's DictionaryFilter, from the encodings when stats are missing. - ParquetSchemaFingerprint fingerprints the column tree (names, child counts, and physical types) with a 64-bit FarmHash and ignores the writer-specific root name. Adds parquet-format-structures, shaded and relocated into this jar so its Thrift runtime never clashes with the one on a consumer's classpath.
prudhvimaharishi
force-pushed
the
parquet_footer_decoding
branch
from
November 2, 2026 16:51
642819d to
6b17783
Compare
Leave the dependency's manifest, Maven metadata, Util, and event package out of the shaded jar. Util decodes footers without per-value limits, so keeping it out leaves BoundedCompactProtocol as the only decoder. Correct the skip depth Javadoc, which held only for the packaged jar, note where ParquetDictionaryEncoding is stricter than parquet-mr, explain the length prefix and the optional dependency accurately, and warn that integration tests run against the shaded jar. Add tests for a binary value that runs past the remaining bytes and for a leaf and an empty group with the same name.
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
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.
Type of Change
feat: A new featurefix: A bug fixdocs: Documentation only changesstyle: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)refactor: A code change that neither fixes a bug nor adds a featureperf: A code change that improves performancetest: Adding missing tests or correcting existing testschore: Changes to the build process or auxiliary tools and libraries such as documentation generationDescription
What?
Adds three package-private helpers for decoding Parquet footers safely:
BoundedCompactProtocolcaps every Thrift string, binary value, and container at the message length, and caps how deep an unknown field can nest while it is skipped at 64 levels. Without it, a 7-byte list header makes the generated code allocate a two-billion-element list (OutOfMemoryError), a binary value can declare up to 100 MB or a negative length, and about 100 KB of nested unknown fields overflows the stack (StackOverflowError); the errors escape ordinary exception handling.ParquetDictionaryEncodingdecides whether a chunk's dictionary page can filter the chunk on its own, from page encoding stats or, like parquet-mr'sDictionaryFilter, from the encodings when stats are missing. Where it differs from parquet-mr, it is stricter.ParquetSchemaFingerprintfingerprints the column tree (names, child counts, and physical types) with a 64-bit FarmHash and ignores the writer-specific root name, soa{b}, canda{b, c}get different fingerprints.Packaging
parquet-format-structures, which bundles a minimal Thrift runtime and does not depend on Hadoop, withoutjavax.annotation-api, which none of its classes reference.org.apache.parquet.formatand its Thrift runtime undercom.google.cloud.gcs.analyticscore.internal.shaded, so consumers never see these classes. The dependency isoptional, so it is not passed on transitively. The core jar grows from about 48 KB to about 890 KB.Utiland theeventpackage, which decode footers without per-value limits, plus the dependency's manifest and Maven metadata.slf4j-apidirectly, because the bundled Thrift transport logs through it. Core already got it throughclient.Why?
Footers come from untrusted objects, so decoding them must never exhaust the heap or the stack, and decisions taken from them must be conservative.
This PR is the second of three that replace the original, larger version of #420:
Checklist
feat(core): ...)Generated/Assisted by Agent? Yes