Skip to content

fix(sigv4): preserve equals in canonical uri paths#10

Closed
jlon wants to merge 1 commit into
rustfs:mainfrom
jlon:fix-sigv4-equals-path-2477
Closed

fix(sigv4): preserve equals in canonical uri paths#10
jlon wants to merge 1 commit into
rustfs:mainfrom
jlon:fix-sigv4-equals-path-2477

Conversation

@jlon

@jlon jlon commented Apr 12, 2026

Copy link
Copy Markdown

Summary

  • preserve = in SigV4 canonical URI paths instead of percent-encoding it during canonical request construction
  • keep canonical URI encoding for other reserved path characters such as $, space, and %
  • add regression tests for both header-auth and presigned canonical requests whose object key ends with =

Related Context

  • fixes the root cause behind rustfs/rustfs#2477
  • this change is intended to unblock the follow-up RustFS PR in https://github.com/rustfs/rustfs/pulls

Verification

  • cargo test -p s3s canonical_request_preserves_trailing_equals_in_uri_path -- --nocapture
  • cargo test -p s3s presigned_canonical_request_preserves_trailing_equals_in_uri_path -- --nocapture
  • cargo test -p s3s canonical_request_still_encodes_other_reserved_uri_path_characters -- --nocapture

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bc4c45a6e6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/s3s/src/sig_v4/methods.rs Outdated
Comment thread crates/s3s/src/sig_v4/methods.rs Outdated
@jlon jlon force-pushed the fix-sigv4-equals-path-2477 branch 3 times, most recently from 04d3b42 to 478ddd4 Compare April 12, 2026 00:53
@codecov

codecov Bot commented Apr 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
crates/s3s/src/sig_v4/methods.rs 98.57% <100.00%> (+0.24%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@houseme

houseme commented Apr 12, 2026

Copy link
Copy Markdown
Collaborator

Thank you for your contribution, we suggest you also submit a PR to https://github.com/s3s-project/s3s, thank you! @jlon

Signed-off-by: jianglong <jianglong@oppo.com>
@jlon jlon force-pushed the fix-sigv4-equals-path-2477 branch from 478ddd4 to a060344 Compare April 12, 2026 07:23
@houseme houseme requested a review from overtrue June 11, 2026 02:15
@houseme

houseme commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

s3s-project#589

@houseme

houseme commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Closing this because main already contains the broader upstream fix from s3s-project/s3s#589 via commit 928550a (fix(s3s): allow raw URI path SigV4 fallback (#589)). Compared with this PR, #589 keeps the standard canonical URI behavior and adds a narrow raw-path fallback for header auth, presigned URLs, HTTP/2 authority handling, and streaming-body seed signatures. Cherry-picking #10 now would overlap with the same files and move us to a narrower preserve = globally approach, so the current mainline solution is the one to keep. Thanks again for the report and patch.

@houseme houseme closed this Jun 11, 2026
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