Spring Support Forwarded with fallback to X-Forwarded-Host

The ForwardedHeaderFilter supports already Forwarded headers as defined in: https://tools.ietf.org/html/rfc7239#section-5.2

Improvement: Implement fallback to X-Forwarded-Host and X-Forwarded-Port header if host is not defined in Forward header.

Background: The Forwarded header can contain some or all of the parameters: by, for, host, proto. ForwardedHeaderFilter uses UriComponentsBuilder to adjust URI parts. The implementation in UriComponentsBuilder#adaptFromForwardedHeaders right now is not really symetric. First it checks if there is a Forwarded header, if yes it tries to take the parameters proto, host from this.
Only for proto there is a fallback implemented. Meaning that if proto is not found in Forwarded header, the X-Forwarded-Ssl is checked additionally. This is not done for host. So if there is a Forwarded header but if this does not contain host-parameter, host and port are not extracted at all. Wouldn't it be better to also fallback to the X-Forwarded-Host and X-Forwarded-Port header if they exist?

Scenario: When using spring application behind a fabio proxy this problem occurs. The headers that fabio sets are: forwarded: "for=100.93.64.162; proto=http; by=192.168.30.12; httpproto=http/1.1" x-forwarded-host: "10.65.250.31:9999" host is not defined in Forwarded, so the host is not extracted in current implementation.

Comment From: bclozel

It might be hard to cover both Forwarded and X-Forwarded-* headers consistently, since values are ordered and there would be no way to know which header value came first if both Forwarded: host= and X-Forwarded-Host are there. Our current approach is to either process Forwarded or X-Forwarded-*, but not both - and I think this is a sane approach and this avoids many corner cases and potential security issues.

In general, I tend to agree with the spec: during the transition to proper Forwarded headers, proxies are in the best position to make that call and translate X-Forwarded-* to Forwarded values. Also, the headers set in this example are wrong, since httpproto is not defined in the spec.

Could you report those issues to the proxy?

Comment From: hlang

My proposal is to keep the current logic. Always use parameters provided in Forwarded first. Only if a parameter is not provided by Forwarded fallback to related X-Forwarded-*. This is already implemented for proto. If proto is not in Forwarded, it is taken from X-Forwarded-Ssl.

                Matcher matcher = FORWARDED_PROTO_PATTERN.matcher(forwardedToUse);
                if (matcher.find()) {
                    scheme(matcher.group(1).trim());
                    port(null);
                }
                else if (isForwardedSslOn(headers)) {
                    scheme("https");
                    port(null);
                }

Same thing could be done for host. Something like:

                matcher = FORWARDED_HOST_PATTERN.matcher(forwardedToUse);
                if (matcher.find()) {
                    adaptForwardedHost(matcher.group(1).trim());
                } else {
                    adaptHostAndPortFromXForwardedHeader(headers);
                }

Comment From: rstoyanchev

@hlang it is a fair question, why would a proxy do one Forwarded and one X-Forwarded? If there is a reasonable explanation we could consider the change, but otherwise I would advise asking for this to be fixed in the proxy.

Comment From: hlang

Opened a related Fabio issue: https://github.com/fabiolb/fabio/issues/713