Don't forward proto if we receive the header #131
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
If we receive a header for X-Forwarded-For and we have the parameter forwardIP, we add the IP of this machine to the list of IPs. However for X-Forwarded-Proto it is not a list. We send the proto of the existing request (if forwardIP is enabled). This change honors the header if we receive it
dsmiley
reviewed
Feb 1, 2018
| @@ -446,6 +446,10 @@ private void setXForwardedForHeader(HttpServletRequest servletRequest, | |||
|
|
|||
| String protoHeaderName = "X-Forwarded-Proto"; | |||
| String protoHeader = servletRequest.getScheme(); | |||
| String existingProtoHeader = servletRequest.getHeader(protoHeaderName); | |||
| if (existingProtoHeader != null) { | |||
There was a problem hiding this comment.
Looks fine. Please add a comment that this header is not a list.
|
Can you confirm this PR is still valid? I can commit it. |
|
Ping |
|
I'm sorry for the (VERY!) late reply. We use the modified code in a forked version, but we haven't keep up with most recent versions and we're moving away from the code that uses this library. |
|
I understand; I'm a lazy steward here :-) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This change is a bit more controversial than the previous one. Feel free to decline if it doesn't suit.
When upgrading from version 1.7 to version 1.10, we found the X-Forwarded-Proto header was no longer being honoured when passing through the proxy. The header was being replaced.
For X-Forwarded-For, the behaviour was easier since we can add the IP to the list. However my understanding is that X-Forwarded-Proto it is not a list. We could disable forwardip, or just ignore current protocol if we have a header.