Skip to content

fix(services/b2): percent-encode the object path in presigned requests - #8076

Merged
Xuanwo merged 1 commit into
apache:mainfrom
PDGGK:fix-b2-presign-encoding
Aug 14, 2026
Merged

fix(services/b2): percent-encode the object path in presigned requests#8076
Xuanwo merged 1 commit into
apache:mainfrom
PDGGK:fix-b2-presign-encoding

Conversation

@PDGGK

@PDGGK PDGGK commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

None — found while reading the service.

Rationale for this change

The three presign arms interpolate the object path raw, while the ordinary read and write paths in the same crate encode it.

Presign (backend.rs), Stat and Read:

let url = format!(
    "{}/file/{}/{}?Authorization={}",
    auth_info.download_url, self.core.bucket, path, resp.authorization_token
);

Presign Write:

req = req.header("X-Bz-File-Name", build_abs_path(&self.core.root, path));

The reference, core.rs:

// :144-149  download_file_by_name
let url = format!("{}/file/{}/{}",
    auth_info.download_url, self.bucket, percent_encode_path(&path));

// :262  upload_file
req = req.header(X_BZ_FILE_NAME, percent_encode_path(&p));

Handing the formatted strings to http::Request::get shows what a presigned URL actually becomes:

path resulting URI
dir/file.txt path=/file/bkt/dir/file.txt, query=Authorization=TOK
a#b.txt path=/file/bkt/a, query=None
a?b.txt path=/file/bkt/a, query=b.txt?Authorization=TOK
a b.txt BUILD ERROR: invalid uri character
a%20b.txt sends a%20b.txt, which B2 decodes to a b.txt

The # row is the worst: the URL is truncated to a different object and the entire ?Authorization= is swallowed as a fragment, so the presigned URL carries no token at all. a b.txt cannot be presigned even though op.read("a b.txt") on the same object works — that call goes through the encoding path. And the write header sends a%20b.txt where op.write("a%20b.txt") sends a%2520b.txt, so a presigned upload lands on a different key than an ordinary write for the same OpenDAL path.

Blast radius — the first row is the one that matters: a path with no reserved characters encodes to itself, because percent_encode_path leaves / alone. Presigned URLs for ordinary object names are byte-identical to what they are today; only names that are currently broken change.

What changes are included in this PR?

One call to percent_encode_path at each of the three sites, making presign agree with download_file_by_name and upload_file.

Tests

No new tests, deliberately: these are format! arguments and a header value inside an async method whose only seam is a live B2 authorization call, and asserting on them would mean restructuring the presign arms — which I would rather not fold into a fix. The 3 existing unit tests pass; cargo fmt --all -- --check and cargo clippy -p opendal-service-b2 --all-targets are clean with zero warnings.

Note for rebasing

#7801 renames build_abs_path at these same three sites. If it lands first, the X-Bz-File-Name line needs a one-word rebase.

Are there any user-facing changes?

Yes, for services-b2: presign_read / presign_stat / presign_write on an object whose name contains a character that is reserved in a URL now address that object, keep their authorization token, and agree with what a non-presigned read or write would do. Presigned URLs for names without reserved characters are unchanged.

The three presign arms interpolate the path raw, while the ordinary
read and write paths in the same crate encode it:

    // backend.rs, presign Stat and Read
    let url = format!(
        "{}/file/{}/{}?Authorization={}",
        auth_info.download_url, self.core.bucket, path, resp.authorization_token
    );

    // backend.rs, presign Write
    req = req.header("X-Bz-File-Name", build_abs_path(&self.core.root, path));

    // core.rs:144-149, download_file_by_name -- the reference
    let url = format!("{}/file/{}/{}",
        auth_info.download_url, self.bucket, percent_encode_path(&path));

    // core.rs:262, upload_file -- the reference
    req = req.header(X_BZ_FILE_NAME, percent_encode_path(&p));

Measured by handing the formatted strings to http::Request::get:

    dir/file.txt   path=/file/bkt/dir/file.txt  query=Authorization=TOK
    a#b.txt        path=/file/bkt/a             query=None
    a?b.txt        path=/file/bkt/a             query=b.txt?Authorization=TOK
    a b.txt        BUILD ERROR: invalid uri character
    a%20b.txt      sends a%20b.txt, which B2 decodes to "a b.txt"

The `#` case is the worst: the URL is truncated to a different object
*and* the whole ?Authorization= is swallowed as a fragment, so the
presigned URL carries no token at all. `a b.txt` cannot be presigned even
though op.read("a b.txt") on the same object works, because that goes
through the encoding path. And the write header sends a%20b.txt where
op.write("a%20b.txt") sends a%2520b.txt, so a presigned upload lands on a
different key than an ordinary write for the same OpenDAL path.

The first line above is the one that matters for blast radius: a path
with no reserved characters encodes to itself, because
percent_encode_path leaves `/` alone. Presigned URLs for ordinary object
names are byte-identical to what they are today.

No new tests: these are format! arguments inside an async method whose
only seam is a live B2 authorization call, and asserting on them would
mean restructuring the presign arms. The three existing unit tests pass,
fmt and clippy are clean.

Note for rebasing: apache#7801 renames build_abs_path at these same three
sites. If it lands first the X-Bz-File-Name line needs a one-word
rebase.
@PDGGK
PDGGK requested a review from Xuanwo as a code owner August 14, 2026 17:33
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. releases-note/fix The PR fixes a bug or has a title that begins with "fix" labels Aug 14, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 14, 2026
@Xuanwo

Xuanwo commented Aug 14, 2026

Copy link
Copy Markdown
Member

Next time, please avoid submitting similar PRs.

@Xuanwo
Xuanwo merged commit 2988850 into apache:main Aug 14, 2026
105 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants