Skip to content

fix: do not sign Content-Length/Content-Type on bodyless DELETE requests#467

Open
eldon-databanx wants to merge 1 commit into
durch:masterfrom
Databanx:fix/delete-signed-headers
Open

fix: do not sign Content-Length/Content-Type on bodyless DELETE requests#467
eldon-databanx wants to merge 1 commit into
durch:masterfrom
Databanx:fix/delete-signed-headers

Conversation

@eldon-databanx

@eldon-databanx eldon-databanx commented Jul 2, 2026

Copy link
Copy Markdown

Fixes #466.

Problem

headers() in request_trait.rs falls through to the catch-all arm for DELETE commands and inserts Content-Length: 0 and Content-Type: text/plain into the signed header map. Since a DELETE has no body, the HTTP client never actually transmits those headers, so the canonical request signed by rust-s3 differs from the one the server reconstructs.

AWS S3 tolerates the discrepancy, which is why this goes unnoticed there. Cloudflare R2 enforces SigV4 strictly and rejects every DELETE with 403 SignatureDoesNotMatch — PUT/GET/HEAD work fine, but no object can ever be deleted on R2.

Fix

Skip inserting Content-Length/Content-Type for all bodyless DELETE commands: DeleteObject, DeleteObjectTagging, AbortMultipartUpload, DeleteBucket, DeleteBucketCors and DeleteBucketLifecycle. (DeleteObjects is a POST with an XML body and keeps its headers.)

Testing

Verified in production against Cloudflare R2 (Stalwart mail server, which had been unable to purge any blob on R2): with this patch, delete_object succeeds — a backlog of ~2.8k objects that had been failing with SignatureDoesNotMatch was deleted without a single error. Behaviour against AWS S3 is unchanged, since the headers were no-ops there.


This change is Reviewable

Summary by CodeRabbit

  • Bug Fixes
    • Improved request header handling for delete-style operations so they no longer include unnecessary Content-Length or Content-Type headers.
    • This makes bodyless requests more consistent and better aligned with expected service behavior.

A DELETE has no body, so rust-s3 signed content-length: 0 and
content-type: text/plain while the HTTP client never transmitted those
headers. AWS S3 tolerates the discrepancy; Cloudflare R2 enforces SigV4
strictly and rejects every DELETE with 403 SignatureDoesNotMatch,
making object deletion on R2 impossible.

Skip the two headers for all bodyless DELETE commands: DeleteObject,
DeleteObjectTagging, AbortMultipartUpload, DeleteBucket,
DeleteBucketCors and DeleteBucketLifecycle.

Fixes durch#466
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 644718f8-3af0-4405-8433-f0c6d3f25278

📥 Commits

Reviewing files that changed from the base of the PR and between b584ce7 and 505aded.

📒 Files selected for processing (1)
  • s3/src/request/request_trait.rs

📝 Walkthrough

Walkthrough

This PR modifies Request::headers() to add an explicit match branch for bodyless delete-style commands (DeleteObject, DeleteObjectTagging, AbortMultipartUpload, DeleteBucket, DeleteBucketCors, DeleteBucketLifecycle), preventing insertion of CONTENT_LENGTH and CONTENT_TYPE headers for these requests.

Changes

Header Construction Fix

Layer / File(s) Summary
Bodyless command header exclusion
s3/src/request/request_trait.rs
Adds an explicit match branch covering delete-like, bodyless commands so CONTENT_LENGTH and CONTENT_TYPE are no longer inserted, previously handled by the default branch, with added comments explaining SigV4 canonical request implications.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • durch/rust-s3#455: Both PRs update Request::headers() to omit Content-Length/Content-Type for bodyless commands at the same header-construction logic point.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: skipping Content-Length/Content-Type signing for bodyless DELETE requests.
Linked Issues check ✅ Passed The change addresses #466 by omitting body-related signed headers for DeleteObject and other bodyless DELETE operations.
Out of Scope Changes check ✅ Passed No out-of-scope changes are evident; the added command coverage stays within the stated fix for bodyless DELETE signing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@brownian-motion-v0

Copy link
Copy Markdown

Brownian Motion (Brass)

Recommendation: Proceed

Summary: Fixes R2 DELETE signatures by omitting headers clients never send
Risk: Medium · Confidence: 87%

Highlights

  • Targets a real interop bug with strict SigV4 providers
  • Minimal one-file change aligned with existing request signing logic
  • Author validated the fix against Cloudflare R2 in production

Unknowns

  • No automated test currently proves the canonical request mismatch is fixed
  • Unclear whether every backend/client combination already omits these headers for all listed commands
  • The current ci status is unstable, so broader regression risk is harder to rule out

Next actions

  • Keep: the narrow special-casing for bodyless DELETE commands
  • Keep: the explanatory comment tying the change to strict SigV4 validators
  • Add: a regression test that asserts DELETE canonical headers exclude content-length/content-type
  • Add: verify whether any other bodyless methods with explicit zero-length bodies need the same treatment, especially presign/delete variants
  • Drop: expanding scope to unrelated request builders unless they share the same signing path

Reflection questions

  • What exact headers does each backend actually transmit for these DELETE requests?
  • Are we fixing the signing model to match the wire, or just papering over one backend's behavior?
  • Can the request/header derivation be made source-of-truth instead of relying on command-specific exceptions?

@cvermilion

Copy link
Copy Markdown

I believe #452 also includes a (more general) fix.

1 similar comment
@cvermilion

Copy link
Copy Markdown

I believe #452 also includes a (more general) fix.

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.

DeleteObject signs content-length and content-type headers, causing 403 SignatureDoesNotMatch on Cloudflare R2

3 participants