Fix HeadObject body headers

[durch]

Linked issue

Fixes #450.

Problem

Bucket::head_object builds its request through the common request header path. The generic fallback branch added Content-Length: 0 and Content-Type: text/plain for commands that are not explicitly exempted. GetObject was exempted, but HeadObject was not, so HEAD requests emitted and signed unnecessary body headers.

Solution

Add Command::HeadObject to the same narrow no-body-header branch used by GetObject/listing-style commands. This avoids PR #452's broader has_body() style change and leaves upload, multipart initiation/completion, GCS-related multipart paths, and other body-bearing commands untouched.

Focused verification

• cargo test -p rust-s3 --no-default-features --features sync head_object_omits_body_headers_from_request_and_signature -- --nocapture
  • Result: passed; 1 passed, 0 failed.
• cargo test -p rust-s3 --no-default-features --features sync request::blocking::tests -- --nocapture
  • Result: failed in two pre-existing path-style host assertions; the new head_object_omits_body_headers_from_request_and_signature test passed. The failures assert path-style host behavior and are unrelated to HeadObject/body headers.

Risk notes

• Signing risk: intentionally changes the HeadObject canonical signed header set by removing content-length and content-type; regression coverage asserts those headers are absent both from the request and from SignedHeaders while preserving x-amz-content-sha256.
• Provider/runtime cordons: change is in the shared request trait header construction, so it applies across sync/tokio/async-std backends. No URL construction, credential resolution, security token handling, request payload SHA256, multipart upload, or GCS-specific multipart behavior was changed.

---

This change is 

Summary by CodeRabbit

• Bug Fixes

  • HEAD Object requests now correctly exclude Content-Length and Content-Type from both the sent request and the signature, improving API compliance.

• Tests

  • Added a unit test that verifies HEAD Object requests omit body-related headers from request headers and the signed headers list.

[brownian-motion-v0]

Brownian Motion (Brass)

Recommendation: Proceed

Summary: PR effectively addresses unnecessary headers in HeadObject requests.
Risk: Medium · Confidence: 80%

Highlights

• Good test coverage
• Addresses a clear bug
• Maintains existing functionality

Next actions

• Keep: Tests for header omission
• Drop: Unused headers in other commands
• Add: Documentation on header behavior for HeadObject

Reflection questions

• What core assumption underpins this PR's approach?
• How does this change align with the project's longer-term goals?
• Could there be a simpler way to achieve the primary objective here?

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 43aeef36-bae9-46a4-953b-fecacd7d38c6

📥 Commits

Reviewing files that changed from the base of the PR and between 49b7980 and 8c08a2c.

📒 Files selected for processing (2)

• s3/src/request/blocking.rs
• s3/src/request/request_trait.rs

✅ Files skipped from review due to trivial changes (1)

• s3/src/request/blocking.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• s3/src/request/request_trait.rs

---

📝 Walkthrough

Walkthrough

The PR prevents HeadObject requests from receiving body-related headers by adding a dedicated Command::HeadObject match arm in Request::headers() and adds a unit test asserting Content-Length and Content-Type are omitted from both request headers and the SignedHeaders in the Authorization header.

Changes

HeadObject Header Omission

Layer / File(s)	Summary
Core Implementation s3/src/request/request_trait.rs	Adds an explicit Command::HeadObject => {} arm in Request::headers() so CONTENT_LENGTH and CONTENT_TYPE are not inserted via the _ fallback.
Tests s3/src/request/blocking.rs	New test head_object_omits_body_headers_from_request_and_signature verifies Content-Length/Content-Type are absent from the generated request headers and from the SignedHeaders in the Authorization header.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I checked the HEAD, left no weight to bear,

No Content-Type, no Length floating in air;
Signatures tidy, headers quiet and neat,
Caddy no longer trips on that tricksy repeat. 🥕

🚥 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 'Fix HeadObject body headers' directly and accurately summarizes the main change: preventing HeadObject requests from including unnecessary Content-Type and Content-Length headers.
Linked Issues check	✅ Passed	The pull request fully addresses issue #450 by exempting HeadObject from the generic body-header fallback, ensuring HEAD requests omit Content-Type and Content-Length headers from both the request and SignedHeaders.
Out of Scope Changes check	✅ Passed	All changes are strictly scoped to fixing HeadObject body headers: one code change to exempt HeadObject and one test to verify the fix. No unrelated modifications are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-450-head-object-headers

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 6/8 reviews remaining, refill in 14 minutes and 1 second.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

s3/src/request/blocking.rs (1)

244-267: 💤 Low value

LGTM — test covers all three specified objectives cleanly.

The three assertions correctly validate: (1) Content-Length and Content-Type absent from the request HeaderMap, (2) x-amz-content-sha256 still present, and (3) both fields excluded from the SignedHeaders component of the Authorization header.

One optional hardening: the signed_headers.contains("content-type") substring check would produce a false negative if a future signed header happened to embed "content-type" in its name. Splitting on ';' and doing exact-field comparison is marginally more precise:

🔍 Optional: exact-field SignedHeaders assertion

-assert!(!signed_headers.contains("content-length"));
-assert!(!signed_headers.contains("content-type"));
+let signed_header_list: Vec<&str> = signed_headers.split(';').collect();
+assert!(!signed_header_list.contains(&"content-length"));
+assert!(!signed_header_list.contains(&"content-type"));

No action needed — the current form is safe given the actual header names involved.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@s3/src/request/blocking.rs` around lines 244 - 267, The test
head_object_omits_body_headers_from_request_and_signature should verify signed
headers by exact field names instead of substring checks; update the
Authorization parsing in that test (authorization, signed_headers) so after
extracting the SignedHeaders value you split it on ';' (SignedHeaders are
semicolon-separated) into discrete header tokens and assert that none of the
tokens equals "content-length" or "content-type" (rather than using
signed_headers.contains(...)), keeping the rest of the test (headers checks and
x-amz-content-sha256) the same.

s3/src/request/request_trait.rs (1)

708-726: ⚖️ Poor tradeoff

LGTM — correct minimal fix for the no-body-headers oversight.

The Command::HeadObject => {} arm cleanly prevents CONTENT_LENGTH/CONTENT_TYPE from being inserted or signed, matching the same pattern already used for GetObject.

Worth noting as a follow-up: several other bodyless commands (DeleteObject, AbortMultipartUpload, GetObjectRange, GetObjectTorrent, PresignGet, PresignDelete) still fall through to the _ arm and silently acquire Content-Length: 0 / Content-Type: text/plain. These haven't triggered proxy failures yet, but the underlying inconsistency remains. A future Command::has_body() predicate (as noted in the PR description regarding PR #452) would resolve all of them uniformly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@s3/src/request/request_trait.rs` around lines 708 - 726, The match in
request_trait.rs currently special-cases HeadObject to avoid inserting
CONTENT_LENGTH/CONTENT_TYPE but several other bodyless commands (e.g.,
DeleteObject, AbortMultipartUpload, GetObjectRange, GetObjectTorrent,
PresignGet, PresignDelete) still fall through and get Content-Length: 0 /
Content-Type: text/plain; update the match on self.command() (or implement and
call a Command::has_body() predicate) so that all bodyless variants are handled
like HeadObject (add explicit arms for DeleteObject, AbortMultipartUpload,
GetObjectRange, GetObjectTorrent, PresignGet, PresignDelete to skip inserting
CONTENT_LENGTH and CONTENT_TYPE, or replace the current logic with if
self.command().has_body() { insert CONTENT_LENGTH/CONTENT_TYPE using
self.command().content_length() / content_type() }).

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@s3/src/request/blocking.rs`:
- Around line 244-267: The test
head_object_omits_body_headers_from_request_and_signature should verify signed
headers by exact field names instead of substring checks; update the
Authorization parsing in that test (authorization, signed_headers) so after
extracting the SignedHeaders value you split it on ';' (SignedHeaders are
semicolon-separated) into discrete header tokens and assert that none of the
tokens equals "content-length" or "content-type" (rather than using
signed_headers.contains(...)), keeping the rest of the test (headers checks and
x-amz-content-sha256) the same.

In `@s3/src/request/request_trait.rs`:
- Around line 708-726: The match in request_trait.rs currently special-cases
HeadObject to avoid inserting CONTENT_LENGTH/CONTENT_TYPE but several other
bodyless commands (e.g., DeleteObject, AbortMultipartUpload, GetObjectRange,
GetObjectTorrent, PresignGet, PresignDelete) still fall through and get
Content-Length: 0 / Content-Type: text/plain; update the match on self.command()
(or implement and call a Command::has_body() predicate) so that all bodyless
variants are handled like HeadObject (add explicit arms for DeleteObject,
AbortMultipartUpload, GetObjectRange, GetObjectTorrent, PresignGet,
PresignDelete to skip inserting CONTENT_LENGTH and CONTENT_TYPE, or replace the
current logic with if self.command().has_body() { insert
CONTENT_LENGTH/CONTENT_TYPE using self.command().content_length() /
content_type() }).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 34932619-66ec-4855-9d78-b3e45972ff9c

📥 Commits

Reviewing files that changed from the base of the PR and between 57525a4 and 49b7980.

📒 Files selected for processing (2)

• s3/src/request/blocking.rs
• s3/src/request/request_trait.rs

[brownian-motion-v0]

Brownian Motion (Brass)

Recommendation: Proceed

Summary: PR effectively addresses unnecessary headers in HEAD requests.
Risk: Medium · Confidence: 85%

Highlights

• Good test coverage
• Addresses a clear bug
• Maintains existing functionality

Next actions

• Keep: The focused test for HEAD requests.
• Drop: Any unrelated changes or comments.
• Add: Documentation on the impact of this change.

Reflection questions

• What core assumption underpins this PR's approach?
• How does this change align with the project's longer-term goals?
• Could there be a simpler way to achieve the primary objective here?

[durch]

Thanks for the focused fix. This keeps the header/signing change narrow: HeadObject now follows the existing no-body request behavior without adopting the broader has_body() approach that caused provider compatibility problems elsewhere.

Verification before merge:

• local make ci passed in the issue worktree
• refreshed GitHub checks are green after Fix presign tests: use fake credentials instead of minio env vars #453 restored the presign-test baseline
• reviewed under triage rules: PASS, with the main risk being provider-sensitive signing/header behavior; the change is intentionally limited to HeadObject

Merging this as the fix for #450.