Repository navigation
fix(proxy): preserve client ports in forwarded Host headers - #5934
Open
lennondotw wants to merge 1 commit into
Open
lennondotw wants to merge 1 commit into
lennondotw wants to merge 1 commit into
Conversation
Keep the incoming Host authority for default and custom locations, with the existing hostname fallback when Host is absent. Add a built-image regression that validates Host against Origin and Referer and run it after the CI image build.
7 of 10 tasks
|
CI Error: |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -1,5 +1,5 @@ | |||
| add_header X-Served-By $host; | |||
| proxy_set_header Host $host; | |||
| proxy_set_header Host $forward_host; | |||
There was a problem hiding this comment.
With asset caching enabled, this include now forwards a port-specific Host, but assets.conf still uses proxy_cache_key $host$request_uri. Requests for the same JavaScript file at example.test:232 and example.test:233 therefore share a cache entry. If the upstream returns port-specific links, users on one port can receive links pointing to the other.
Include the forwarded authority in the asset cache key and add a two-port cache test.
Comment on lines
+115
to
+116
| docker("exec", name, "nginx", "-s", "reload") | ||
| time.sleep(1) |
There was a problem hiding this comment.
The test waits one second after signaling an Nginx reload, then rejects the first unexpected response. Reloading does not wait for the new workers to serve the generated host, so a busy CI machine can still return the old default page and fail a correct build.
Replace the fixed sleep with a bounded readiness check for the generated host and upstream before running the assertions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
NPM currently forwards
Host: $host, which strips the client-facing port. A browser visitinghttps://router.example.test:233therefore sends a Host, Origin and Referer containing:233, but the upstream receives a Host without it. Applications that compare these values can reject authenticated requests even though the proxy connects successfully.This was reproduced on an ASUS router: read/save endpoints returned HTTP 200 with an HTML script redirecting to the login page, instead of reading or persisting settings. Preserving
$http_hostfixed the requests; restoring the old header in the same session reproduced the failure.Changes
$http_hostwhen present and falls back to$hostwhen absent.scripts/ci/test-and-build. It renders an actual proxy host through the backend generator and sends GET/POST requests to a synthetic HTTPS upstream that validates Host against Origin and Referer.The upstream connection address/port remains independently configured. This forwards the port actually present in the request; it does not append a configured public port to every Host. No UI option, API field or database migration is added.
Compatibility
This changes the default forwarded Host for requests with explicit ports; original hostname casing is preserved as well. Host values without a port remain without a port, and HTTP/1.0 requests without Host retain a nonempty
$hostfallback. Existing fulllocation /overrides continue to control their own headers. Generated configurations pick up the change when created or regenerated.Related work
develop.$http_host; feat: add use_http_host option to preserve port in Host header #5551 proposes a per-host opt-in setting. This version implements a consistent default for both default and custom locations, preserves the missing-Host fallback, and includes built-image regression coverage in CI.$http_hostfor passing the original Host unchanged.Validation
nginx -t, Python syntax, CI shell syntax andgit diff --checkpassed. The new built-image check is wired into upstream CI; upstream Jenkins results remain separate from local validation.Type of Change
AI Usage