chore: Add optional 103 Early Hints support to Caddy proxy #155
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe pull request introduces an HTTP 103 Early Hints response mechanism to Caddy configurations. When the EARLY_HINTS_LINKS environment variable is set and the request method is GET or HEAD, Caddy emits a 103 response with a Link header before proxying the request upstream, enabling clients to preemptively fetch referenced assets. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Caddy
participant Upstream
Client->>Caddy: GET /path
rect rgb(200, 220, 255)
Note over Caddy: @early_hints match:<br/>GET/HEAD + EARLY_HINTS_LINKS set
Caddy->>Caddy: Evaluate matcher
end
alt Early Hints Eligible
rect rgb(220, 240, 220)
Note over Caddy: Emit Early Hints
Caddy->>Client: 103 Early Hints<br/>(Link header)
Client->>Client: Start fetching<br/>linked resources
end
end
Caddy->>Upstream: Forward request
Upstream->>Caddy: Response
Caddy->>Client: Final response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes The changes introduce a consistent pattern (Early Hints mechanism) applied homogeneously across two configuration files, with supporting documentation. The underlying logic is straightforward—matcher conditions and response header injection—making this a low-complexity review focused on verifying configuration syntax and documentation accuracy. Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @gocanto, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for HTTP 103 Early Hints within the Caddy proxy setup. By leveraging an environment variable, Caddy can now send preliminary Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for HTTP 103 Early Hints in both local and production Caddy configurations, controlled by the EARLY_HINTS_LINKS environment variable. The implementation in the Caddyfiles is sound, correctly using a named matcher and a route block to conditionally send the 103 response before proxying the request. The new documentation in caddy/readme.md clearly explains the feature.
I have a couple of suggestions for the documentation to improve clarity and add an important security consideration. Overall, this is a great addition for improving frontend performance.
caddy/readme.md
Outdated
| When this variable is defined Caddy will emit an HTTP `103 Early Hints` | ||
| response for eligible GET and HEAD requests before proxying traffic, allowing | ||
| clients to start fetching the referenced assets while the upstream response is | ||
| prepared. |
There was a problem hiding this comment.
Since the value of the EARLY_HINTS_LINKS environment variable is directly injected into the Link response header, it's important to be aware of potential security risks like HTTP response splitting if the variable could be controlled by an untrusted source. While environment variables are generally considered trusted, it would be good practice to add a security note to the documentation to warn users about this.
For example, you could add:
> **Security Note:** The value of this variable is used directly in the HTTP response. Ensure it is a properly formatted `Link` header value and does not contain untrusted data or newline characters to prevent security vulnerabilities like HTTP response splitting.
caddy/readme.md
Outdated
| Set the `EARLY_HINTS_LINKS` environment variable to a comma-separated list of | ||
| [`Link` header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link) | ||
| values (for example: `</assets/app.css>; rel=preload; as=style, </assets/app.js>; rel=modulepreload`). |
There was a problem hiding this comment.
The current phrasing "a comma-separated list of Link header values" could be slightly ambiguous. A user might interpret this as providing values that will be individually formatted, rather than providing the complete, single string for the Link header value. To improve clarity and prevent potential misconfiguration, I suggest rephrasing to make it explicit that the environment variable's content is used directly as the header's value.
| Set the `EARLY_HINTS_LINKS` environment variable to a comma-separated list of | |
| [`Link` header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link) | |
| values (for example: `</assets/app.css>; rel=preload; as=style, </assets/app.js>; rel=modulepreload`). | |
| Set the `EARLY_HINTS_LINKS` environment variable to the full string for the | |
| [`Link` header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link) | |
| value (for example: `</assets/app.css>; rel=preload; as=style, </assets/app.js>; rel=modulepreload`). |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
caddy/Caddyfile.prod
Outdated
| @early_hints { | ||
| method GET HEAD | ||
| expression {env.EARLY_HINTS_LINKS} != "" | ||
| } | ||
|
|
||
| # --- Default handler: optional Early Hints before proxying to web | ||
| route { | ||
| respond @early_hints 103 { | ||
| header Link {env.EARLY_HINTS_LINKS} | ||
| } | ||
|
|
||
| reverse_proxy web:80 { | ||
| header_up Host {host} |
There was a problem hiding this comment.
The same respond @early_hints 103 pattern appears in the production configuration, which means that as soon as EARLY_HINTS_LINKS is defined, any eligible GET or HEAD request will terminate with status 103 and the reverse_proxy web:80 block is skipped. This breaks all normal traffic behind the proxy whenever early hints are enabled. The early-hints response should be emitted without aborting the route so the request can still be proxied.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
caddy/Caddyfile.prod (1)
94-105: Early Hints (103) responses are not being sent in production; verify the @early_hints matcher configuration.The verification reveals that Early Hints are not being sent to the frontend path, despite the Caddyfile configuration. The expected 103 informational response before the final 200 response is absent. This indicates the
@early_hintsmatcher is either undefined, misconfigured, or not matching requests.Verify that:
- The
@early_hintsmatcher is properly defined elsewhere in the Caddyfile- The matcher correctly identifies requests that should receive Early Hints
- The
{env.EARLY_HINTS_LINKS}environment variable is set and contains valid Link header valuesThe local/prod architectural inconsistency noted earlier is secondary; resolve the Early Hints deployment issue first.
🧹 Nitpick comments (1)
caddy/readme.md (1)
10-18: Documentation is clear; consider minor enhancements.The Early Hints documentation effectively explains the feature and provides a helpful example.
For additional clarity, you might consider:
- Explicitly stating that "eligible" means GET/HEAD requests when
EARLY_HINTS_LINKSis set and non-empty.- Noting that in production, Early Hints currently apply only to web frontend traffic (not
/api/*paths), while in local development they apply to all traffic.- Mentioning that multiple Link header values should be comma-separated in the single env var string (as shown in the example).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
caddy/Caddyfile.local(1 hunks)caddy/Caddyfile.prod(1 hunks)caddy/readme.md(1 hunks)
🔇 Additional comments (3)
caddy/Caddyfile.local (2)
38-41: Matcher syntax looks correct.The
@early_hintsmatcher appropriately checks for GET/HEAD methods and a non-emptyEARLY_HINTS_LINKSenvironment variable. The expression syntax is valid for Caddy.
46-52: The configuration is correct; Caddy properly handles 103 Early Hints in route blocks.When Caddy responds with status 103 (Early Hints), the response is written without a body and the handler chain will continue. This means the
respond @early_hints 103directive sends the informational response with the Link header, then thereverse_proxydirective executes normally as the final handler in the chain.The pattern you've implemented—combining
respondwith 103 andreverse_proxyin the same route—is the documented and supported way to use Early Hints in Caddy. No changes are needed.caddy/Caddyfile.prod (1)
88-91: Matcher definition is consistent with local config.The
@early_hintsmatcher uses the same logic as inCaddyfile.local, ensuring consistency across environments.
gocanto
changed the title
1 participant
Summary
EARLY_HINTS_LINKSis definedTesting
Summary by CodeRabbit
New Features
Documentation