This repository has been archived by the owner on Jan 31, 2020. It is now read-only.
Use the Host port when port forwarding is detected #4
Conversation
This file contains 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
In vagrant and other systems, you often have a port on the host
forwarding to a port on the client box. In terms of ServerUrl, the port
on the host server is the public one that should be used.
Prior to this patch ServerUrl was detecting both the port from the Host
header, as well as SERVER_PORT, and using *both* in generated URLs.
As an example, given:
HTTP_HOST: localhost:10088
SERVER_PORT: 10081
ServerUrl would generate the host "localhost:10088:10081", when it
should be generating "localhost:10088". This patch corrects the
situation.
|
👍 |
| // If the Host header contains a port, and it differs from the | ||
| // SERVER_PORT, use the port from the Host header. Typically | ||
| // this is a situation where port-forwarding is in use, and you | ||
| // want to present a URI using the public port. |
There was a problem hiding this comment.
Comment should be:
// If the Host header contains a port, then use this port. Typically
// this is a situation where port-forwarding is in use, and you
// want to present a URI using the public port.
or similar as we don't actually check that it differs from SERVER_PORT (or need to).
There was a problem hiding this comment.
or similar as we don't actually check that it differs from SERVER_PORT (or need to).
There's a block immediately before it that does, actually. :) This is the condition that we hit if the SERVER_PORT and the HTTP_HOST's port are not equal.
There was a problem hiding this comment.
Hmm... we should probably extract the port from HTTP_HOST once then :)
- Per @akrabat, the logic for detecting the port based on the combination of the Host header and SERVER_PORT values needed to be cleaned up to make it more clear what was being done. The new approach tests for the combination of SERVER_PORT and a host header in the `<domain>:<port>` format, and uses PCRE matched groups to set the host and/or port.
|
As this was communicated from the Zend Server team, I'm also going to backport it to 2.4.3 as a critical fix for the LTS series. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
In vagrant and other systems, you often have a port on the host forwarding to a port on the client box. In terms of ServerUrl, the port on the host server is the public one that should be used.
Prior to this patch ServerUrl was detecting both the port from the Host header, as well as SERVER_PORT, and using both in generated URLs.
As an example, given:
ServerUrl would generate the host "localhost:10088:10081", when it should be generating "localhost:10088". This patch corrects the situation.