There is no standard governing the x-forwarded-proto header, so we may have guessed wrong. I can't exactly see how a csv list of hosts in x-forwarded-for makes sense without the protocols, but apparently that is what some backends expect.
Comment From: dsyer
There's no standard, but Spring treats all the x-forwarded-* headers as a CSV in UriComponentsBuilder. I don't see any pressing need to change this, but we can leave it open in case someone has some opinions.
Comment From: ryanjbaxter
I read through the Spring Boot issue and it just seemed like he made a request with a x-forwarded-poto header to zuul and zuul tacked on the same value to the already existing. Should it not be a csv list or is the problem that we are adding the same value twice?
Comment From: dsyer
We're adding additional entries so that the protos match the hosts. Seems logical to me, if the CSV means anything.
Comment From: ofaizulin
Hi @dsyer This issue is quite bad. Lot's of systems are expecting to have host and port and proto as a single value.
X-Forwarded-Proto is appended: -> http, https X-Forwarded-Host is appended and contains port: -> localhost, localhost:8080 X-Forwarded-Port is appended: -> 8080, 8080
But, if you look on, for instance, MDN the syntax for mentioned headers:
X-Forwarded-Proto: <protocol>
X-Forwarded-Host: <host>
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Proto https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host
Seems like that there is no entry in MDN for X-Forwarded-Proto, but I would expect behavior to match X-Forwarded-Host and X-Forwarded-Proto
I understand that X-* are experimental and are definitely not a standard (Forwarded header is), however this is breaking change for certain systems being working behind zuul.
IMHO, it worth to follow MDN here, lot's of developers are using it as documentation when building systems.
In future, to avoid such issues, maybe it's worth to drop experimental headers at all and support Forwarded header? https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded
Commit where changed: https://github.com/spring-cloud/spring-cloud-netflix/commit/a38b7b71ac8be9608ac2530dac41cd6298d696cf
Comment From: NetForce1
Can we at least have an option to configure this behavior? Tomcat's RemoteIpValve, for instance, only seems to support multiple values for X-Forwarded-For, not for proto and port. See https://github.com/apache/tomcat/blob/5d7a579366d48668dd81934a74478336dab06ac7/java/org/apache/catalina/valves/RemoteIpValve.java
Comment From: spencergibb
IMHO, it worth to follow MDN here, lot's of developers are using it as documentation when building systems. In future, to avoid such issues, maybe it's worth to drop experimental headers at all and support Forwarded header?
MDN states:
The alternative and de-facto standard versions of this header are the X-Forwarded-For, X-Forwarded-Host and X-Forwarded-Proto headers.
Comment From: Michael-Frank
The statement in commit message a38b7b71ac8be9608ac2530dac41cd6298d696cf is wrong:
[..]The problem is that "Forwarded" does not contain the ports, so Spring UriComponentsBuilder cannot use it to rewrite links to a specific port
The host value token of a Forwarded header can contain an port: <host>:<port>
The Forwarded rfc7239 describes this in section-5.3
The syntax for a "host" value, after potential quoted-string unescaping, MUST conform to the Host ABNF described in Section 5.4 of [RFC7230].
And RFC7230 Section 5.4 states:
The "Host" header field in a request provides the host and port information from the target URI, enabling the origin server to distinguish among resources while servicing requests for multiple host names on a single IP address. Host = uri-host [ ":" port ]
And the Spring UriComponentsBuilder also already supports this case: - https://github.com/spring-projects/spring-framework/blob/9c7de232b844816dd550130da720587cd9d23b6b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java#L742 - https://github.com/spring-projects/spring-framework/blob/9c7de232b844816dd550130da720587cd9d23b6b/spring-web/src/main/java/org/springframework/web/util/UriComponentsBuilder.java#L771
Therefore the following Forwardedheader values are all valid:
- For="[2001:db8:cafe::17]:4711; host=5.5.5.5:8080"
- for=192.0.2.60; proto=http; by=203.0.113.43; host=5.5.5.5:8080
- for=192.0.2.60; proto=https; by=203.0.113.43; host=[1234:db9:badd::16]:8080
- for=192.0.2.43; host=5.5.5.5:8080, for=198.51.100.17; host=7.7.7.7:8080
⚠️ For the future: pleas dont accept any PR that overwrites the headers. - If not header is found: may add - If a header exists: Either leave them and do nothying or append to at end of value as CSV Reasoning: If e.g. Zuul runs behind another proxy layer, blindly overwrinting would destroy the information for which forwarded headers where introduced in the first place - providing downstream services with information only available on the front facing proxy like the real user ip, the DNS name of the recieving host (aka. the URL parts as the clients browser sees it)
Comment From: menelaosbgr
I understand it might be standard to have a CSV list for hosts, but what about port? I've come across a situation where this was causing a problem.
Thanks, Menelaos
Comment From: OlgaMaciaszek
Closing issue, as Zuul is no longer supported.