x-forwarded-proto appended in zuul (should be replaced)

[dsyer]

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.

[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.

[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?

[dsyer]

We're adding additional entries so that the protos match the hosts. Seems logical to me, if the CSV means anything.

[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

[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

[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.

[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 Forwarded header 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)

[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

[OlgaMaciaszek]

Closing issue, as Zuul is no longer supported.