hotfix: host header fix#136
Conversation
…bound host header fix.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 54 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe changes modify how the proxy handles the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/adapter/proxy/core/common.go`:
- Around line 63-68: Fix the goimports lint error and tighten the comment in the
comment block referencing SCOUT-581 / OLLA-135 (the comment immediately above
the code that sets X-Forwarded-Host from originalReq.Host). Correct the typo
"superseeded" -> "superseded", rephrase the last sentence to say that
req.URL.Host is authoritative for the backend and that X-Forwarded-Host is set
from originalReq.Host only when that header is absent to preserve the original
host for backends that need it, and then run goimports (or apply import
grouping/formatting) so the file passes the linter.
In `@internal/adapter/proxy/proxy_headers_test.go`:
- Around line 196-214: The test writes handler-captured variables (e.g.,
capturedHost, capturedXFH, capturedHost1, capturedHost2) from the httptest
server goroutine and reads them from the test goroutine, causing races and also
ignores ProxyRequest errors; fix by making each handler send its captured value
into a channel (e.g., hostCh, xfhCh) and have the test receive from that channel
(with a timeout/select) after calling ProxyRequest, and change the discarded
call `_ = proxy.ProxyRequest(...)` to capture and assert/check the returned
error before reading from the channels; update all referenced tests that use
createProxy, ProxyRequest, and RequestStats accordingly (apply same
channel/select pattern to the other cases noted).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4bddae1-1813-4696-b22e-967efac70ddb
📒 Files selected for processing (3)
internal/adapter/proxy/core/common.gointernal/adapter/proxy/core/common_test.gointernal/adapter/proxy/proxy_headers_test.go
resolves #135
Fix: incorrect Host header on proxied requests (#135)
Olla was forwarding the inbound client's Host header to the backend instead of using the backend's own hostname. HTTPS backends behind nginx rejected this because server_name matching failed; plain HTTP often tolerated it via catch-all vhosts, which is why the bug looked HTTPS-specific.
Root cauase: CopyHeaders in internal/adapter/proxy/core/common.go explicitly copied originalReq.Host onto the outbound request. Both proxy engines build outbound requests from absolute URLs, so letting req.gost stay empty makes Go's transport correctly use req.URL.Host.
The original host is still available to backends via X-Forwarded-Host, which was already being set.
Also fixes, as a side effect really:
Summary by CodeRabbit
Bug Fixes
Tests