Skip to content

GH-49559: [C++][Parquet] Fix uncontrolled recursion in WKBGeometryBounder::MergeGeometryInternal#49558

Merged
wgtmac merged 2 commits into
apache:mainfrom
wgtmac:fix-wkb-recursion-dos-6705798834579741264
Mar 23, 2026
Merged

GH-49559: [C++][Parquet] Fix uncontrolled recursion in WKBGeometryBounder::MergeGeometryInternal#49558
wgtmac merged 2 commits into
apache:mainfrom
wgtmac:fix-wkb-recursion-dos-6705798834579741264

Conversation

@wgtmac

@wgtmac wgtmac commented Mar 19, 2026

Copy link
Copy Markdown
Member

Rationale for this change

Fix MergeGeometryInternal stack overflow from deeply nested WKB GeometryCollection inputs.

What changes are included in this PR?

Added a depth limit (128) to MergeGeometryInternal to prevent stack overflow when parsing deeply nested WKB GeometryCollection inputs.

Are these changes tested?

A unit test TestWKBBounderErrorForDeepNesting has been included to ensure proper exception throwing upon exceeding the limit.

Are there any user-facing changes?

No.

Co-authored-by: wgtmac <4684607+wgtmac@users.noreply.github.com>
@github-actions

This comment was marked as resolved.

@wgtmac wgtmac changed the title Fix uncontrolled recursion in WKBGeometryBounder::MergeGeometryInternal GH-49559: [C++][Parquet] Fix uncontrolled recursion in WKBGeometryBounder::MergeGeometryInternal Mar 19, 2026
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #49559 has been automatically assigned in GitHub to PR creator.

@wgtmac

wgtmac commented Mar 19, 2026

Copy link
Copy Markdown
Member Author

What do you think of this fix? @paleolimbot @pitrou

Disclaimer: this PR is created automatically by Jules from wgtmac#3

@pitrou

pitrou commented Mar 19, 2026

Copy link
Copy Markdown
Member

Jules ? 🤨

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an oversight on my part 😬 ...thank you!

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Mar 19, 2026
@wgtmac

wgtmac commented Mar 20, 2026

Copy link
Copy Markdown
Member Author

Jules ? 🤨

@pitrou Jules is Google's AI-powered asynchronous coding agent: https://jules.google.com :)

Co-authored-by: wgtmac <4684607+wgtmac@users.noreply.github.com>
@wgtmac
wgtmac merged commit 1ffcb4e into apache:main Mar 23, 2026
63 of 66 checks passed
@wgtmac wgtmac removed the awaiting merge Awaiting merge label Mar 23, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit 1ffcb4e.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.

thisisnic pushed a commit to thisisnic/arrow that referenced this pull request Apr 6, 2026
…tryBounder::MergeGeometryInternal (apache#49558)

### Rationale for this change

Fix `MergeGeometryInternal` stack overflow from deeply nested WKB GeometryCollection inputs.

### What changes are included in this PR?

Added a depth limit (128) to `MergeGeometryInternal` to prevent stack overflow when parsing deeply nested WKB GeometryCollection inputs.

### Are these changes tested?

A unit test `TestWKBBounderErrorForDeepNesting` has been included to ensure proper exception throwing upon exceeding the limit.

### Are there any user-facing changes?

No.

* GitHub Issue: apache#49559

Lead-authored-by: Gang Wu <ustcwg@gmail.com>
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: wgtmac <4684607+wgtmac@users.noreply.github.com>
Signed-off-by: Gang Wu <ustcwg@gmail.com>
Mottl pushed a commit to Mottl/arrow that referenced this pull request May 26, 2026
…tryBounder::MergeGeometryInternal (apache#49558)

### Rationale for this change

Fix `MergeGeometryInternal` stack overflow from deeply nested WKB GeometryCollection inputs.

### What changes are included in this PR?

Added a depth limit (128) to `MergeGeometryInternal` to prevent stack overflow when parsing deeply nested WKB GeometryCollection inputs.

### Are these changes tested?

A unit test `TestWKBBounderErrorForDeepNesting` has been included to ensure proper exception throwing upon exceeding the limit.

### Are there any user-facing changes?

No.

* GitHub Issue: apache#49559

Lead-authored-by: Gang Wu <ustcwg@gmail.com>
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Co-authored-by: wgtmac <4684607+wgtmac@users.noreply.github.com>
Signed-off-by: Gang Wu <ustcwg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants