Skip to content

Conversation

@loganintech
Copy link
Contributor

@loganintech loganintech commented Oct 9, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Prevents unintended caching of POST requests by automatically adding standard no-cache headers, improving data freshness and consistency across proxies and intermediaries.
  • Chores
    • Updated HTTP transport pipeline to apply no-cache behavior before other request interceptors.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Walkthrough

Adds a cacheControlTripper that injects no-cache headers on POST requests and delegates to the next RoundTripper. Updates the transport construction to include this tripper before userAgentTripper and existing interceptors.

Changes

Cohort / File(s) Change Summary
HTTP transport interceptors
pkg/uhttp/transport.go
Added cacheControlTripper to set Cache-Control, Pragma, and Expires headers for POST requests; integrated into transport chain ahead of userAgentTripper and other interceptors.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client
    participant CacheControlTripper as cacheControlTripper
    participant UserAgentTripper as userAgentTripper
    participant Next as next RoundTripper
    participant Server

    Client->>CacheControlTripper: RoundTrip(req)
    alt req.Method == POST
        Note right of CacheControlTripper: Inject headers<br/>Cache-Control: no-store, no-cache, must-revalidate<br/>Pragma: no-cache<br/>Expires: 0
    else Other methods
        Note right of CacheControlTripper: Pass-through
    end
    CacheControlTripper->>UserAgentTripper: RoundTrip(req)
    UserAgentTripper->>Next: RoundTrip(req)
    Next->>Server: HTTP request
    Server-->>Next: HTTP response
    Next-->>UserAgentTripper: response
    UserAgentTripper-->>CacheControlTripper: response
    CacheControlTripper-->>Client: response
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through headers, swift and sure,
On POSTs I whisper, “cache no more.”
A thump, a stamp, no stale reprise—
Pragma’s set, Expires dies.
Then onward down the trail I go,
User-Agent nods, the bytes all flow. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly captures the main change of the pull request by stating that POST requests will include no-cache headers, which directly reflects the introduction of the cacheControlTripper behavior described in the changeset. It is clear, specific, and focused on the primary modification without unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch logan/add-post-cache-control

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e2956f and 6bc63f0.

📒 Files selected for processing (1)
  • pkg/uhttp/transport.go (2 hunks)
🔇 Additional comments (3)
pkg/uhttp/transport.go (3)

178-180: LGTM!

The struct definition follows the established pattern of other trippers in this file.


231-231: LGTM!

The tripper is appropriately placed in the chain. Cache control headers will be set after debug logging but before other header modifications, which is a sensible ordering.


183-187: Verify POST-only scope and header overwriting are intentional.

  • Only POST requests get cache-control headers; consider if PUT/PATCH/DELETE should also be covered.
  • Set() unconditionally overwrites headers; ensure this won’t conflict with any upstream cache policies.

if req.Method == http.MethodPost {
req.Header.Set("Cache-Control", "no-store, no-cache, must-revalidate")
req.Header.Set("Pragma", "no-cache")
req.Header.Set("Expires", "0")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use RFC-compliant date format for Expires header.

The value "0" is not RFC 7234 compliant. While many caches may accept it, a proper HTTP-date format should be used.

Apply this diff to use a standards-compliant value:

-		req.Header.Set("Expires", "0")
+		req.Header.Set("Expires", "Thu, 01 Jan 1970 00:00:00 GMT")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
req.Header.Set("Expires", "0")
req.Header.Set("Expires", "Thu, 01 Jan 1970 00:00:00 GMT")
🤖 Prompt for AI Agents
In pkg/uhttp/transport.go around line 186, the Expires header is set to the
non‑RFC value "0"; replace that with an RFC‑compliant HTTP-date string (e.g.,
the Unix epoch or a past date) formatted using Go's http.TimeFormat (RFC1123
with GMT) so caches interpret it correctly, and add the time import if it isn't
already present.

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