Skip to content

Validate Iceberg metadata file path for null bytes#100283

Merged
alexey-milovidov merged 2 commits intomasterfrom
fix-iceberg-null-byte-in-path
Mar 21, 2026
Merged

Validate Iceberg metadata file path for null bytes#100283
alexey-milovidov merged 2 commits intomasterfrom
fix-iceberg-null-byte-in-path

Conversation

@alexey-milovidov
Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov commented Mar 21, 2026

A null byte in the iceberg_metadata_file_path setting causes std::length_error in filesystem path operations. Since std::length_error is a std::logic_error, the exception handler in executeQuery.cpp treats it as a logical error and aborts the server in debug/sanitizer builds.

Reject paths containing null bytes with a proper BAD_ARGUMENTS exception instead.

Closes #96811
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100176&sha=32a8de2243259ca5c69f22e0599aa41189972a69&name_0=PR&name_1=AST%20fuzzer%20%28amd_tsan%29
#100176

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix exception when Iceberg metadata file path setting contains a null byte.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

A null byte in the `iceberg_metadata_file_path` setting causes
`std::length_error` in filesystem path operations. Since
`std::length_error` is a `std::logic_error`, this triggers an abort
in debug/sanitizer builds.

Reject paths containing null bytes with a proper `BAD_ARGUMENTS`
exception instead.

Closes #96811
https://s3.amazonaws.com/clickhouse-test-reports/json.html?PR=100176&sha=32a8de2243259ca5c69f22e0599aa41189972a69&name_0=PR&name_1=AST%20fuzzer%20%28amd_tsan%29

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 21, 2026

Workflow [PR], commit [44401dd]

Summary:


AI Review

Summary

This PR validates iceberg_metadata_file_path against embedded null bytes before constructing std::filesystem::path, converting a debug-build terminating std::length_error path into a proper user-facing BAD_ARGUMENTS exception. The change is small, targeted, and matches the linked issue scope. I did not find correctness, safety, or rollout problems in the diff.

ClickHouse Rules

Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time

Final Verdict

  • Status: ✅ Approve

@clickhouse-gh clickhouse-gh Bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 21, 2026
The null byte check throws `BAD_ARGUMENTS`, not `ICEBERG_SPECIFICATION_VIOLATION`.
For a path with null byte in the middle (Test 3), the null byte check also fires
first before any path resolution, so it's also `BAD_ARGUMENTS`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Mar 21, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 83.80% 83.80% +0.00%
Functions 24.60% 24.60% +0.00%
Branches 76.50% 76.40% -0.10%

PR changed lines: PR changed-lines coverage: 87.50% (7/8, 0 noise lines excluded)
Diff coverage report
Uncovered code

@alexey-milovidov alexey-milovidov self-assigned this Mar 21, 2026
Copy link
Copy Markdown
Member Author

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Ok.

@alexey-milovidov alexey-milovidov merged commit 80e6f91 into master Mar 21, 2026
151 of 152 checks passed
@alexey-milovidov alexey-milovidov deleted the fix-iceberg-null-byte-in-path branch March 21, 2026 21:04
@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Another std::exception. Code: 1001, type: std::length_error with Iceberg

2 participants