Skip to content

fix(storage-azdls): relative path is wrong for non-ASCII and spaces - #3163

Open
jackylee-ch wants to merge 1 commit into
apache:mainfrom
jackylee-ch:fix-azdls-relative-path-encoding
Open

fix(storage-azdls): relative path is wrong for non-ASCII and spaces#3163
jackylee-ch wants to merge 1 commit into
apache:mainfrom
jackylee-ch:fix-azdls-relative-path-encoding

Conversation

@jackylee-ch

Copy link
Copy Markdown

Which issue does this PR close?

None — filed directly.

What changes are included in this PR?

AzureStoragePath::path stored Url::path(), which is percent-encoded, while both consumers
recover the relative path by slicing that many bytes off the raw input string
(azdls.rs azdls_create_operator, and lib.rs relativize_path, used by delete_stream). Any
space or non-ASCII byte makes the encoded length longer than the input, so the offset lands inside
the host:

abfss://myfs@myaccount.dfs.core.windows.net/仓库/db/t/v1.json
  was:  .windows.net/仓库/db/t/v1.json
  now:  /仓库/db/t/v1.json

Reads then miss a blob that exists; writes create it under the mangled key while the manifest
records the correct abfss:// URI. Keeping the raw substring fixes both consumers, so lib.rs
needs no change. HfUri::parse already derives its path from the raw input this way.

Are these changes tested?

Yes — two cases added to test_azdls_create_operator (a non-ASCII segment and a space); both fail
on the parent commit. cargo test --release -p iceberg-storage-opendal --lib --features opendal-azdls → 14 passed (13 before). With --all-features, file_io_gcs_test fails 4 tests with
Connection refused here and identically on unmodified main — it needs the docker fixture.

AI Disclosure

Written with AI assistance (Claude Code); I reviewed the change and ran the tests above.
Note for reviewers: this backend still has no integration test, so the fix is covered by unit tests
only. I did not verify against a live ADLS account.

AzureStoragePath::path held Url::path(), which is percent-encoded, while both
consumers recover the relative path by slicing that many bytes off the raw
input. Keep the raw substring instead, as HfUri::parse already does.
Copilot AI lite review requested due to automatic review settings September 7, 2026 08:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The change is narrowly scoped to ADLS path parsing, matches the stated root cause, and is covered by targeted unit tests for the reported failure cases.

Pull request overview

Fixes OpenDAL ADLS (azdls) path handling so relative-path extraction remains correct when absolute URIs contain spaces or non-ASCII characters, avoiding percent-encoding length mismatches that can corrupt the sliced relative path used by ADLS operations.

Changes:

  • Store AzureStoragePath::path as a raw substring of the input URI (instead of Url::path()), preserving byte-for-byte identity for consumers that slice by length.
  • Add unit test coverage for ADLS URIs containing a non-ASCII segment and a space in the path.
File summaries
File Description
crates/storage/opendal/src/azdls.rs Preserve raw (non-percent-encoded) path for ADLS URI parsing and extend unit tests for non-ASCII/space path cases.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants