-
-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update how request.url is derived #506
Conversation
Signed-off-by: Logan McAnsh <[email protected]>
Signed-off-by: Logan McAnsh <[email protected]>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates how the request URL is derived by introducing a port-resolution logic that considers the "X-Forwarded-Host" and "Host" headers and updates the URL construction accordingly.
- Updated URL derivation logic to include port extraction
- Modified URL construction using the new port information
- Added tests to validate the correct parsing of the URL and port
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
packages/remix-fastify/src/shared.ts | Updated URL construction to resolve the port from headers |
packages/remix-fastify/tests/server.test.ts | Added test to validate correct URL formation when a port is provided |
packages/remix-fastify/src/shared.ts
Outdated
// req.hostname doesn't include port information so grab that from | ||
// `X-Forwarded-Host` or `Host` | ||
let [, hostnamePortStr] = req.get("X-Forwarded-Host")?.split(":") ?? []; | ||
let [, hostPortStr] = req.get("host")?.split(":") ?? []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable 'req' is used instead of the function parameter 'request'. Use 'request.get(...)' to maintain consistency with the function signature.
// req.hostname doesn't include port information so grab that from | |
// `X-Forwarded-Host` or `Host` | |
let [, hostnamePortStr] = req.get("X-Forwarded-Host")?.split(":") ?? []; | |
let [, hostPortStr] = req.get("host")?.split(":") ?? []; | |
// request.hostname doesn't include port information so grab that from | |
// `X-Forwarded-Host` or `Host` | |
let [, hostnamePortStr] = request.get("X-Forwarded-Host")?.split(":") ?? []; | |
let [, hostPortStr] = request.get("host")?.split(":") ?? []; |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Signed-off-by: Logan McAnsh <[email protected]>
Signed-off-by: Logan McAnsh <[email protected]>
remix-run/react-router#13309