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