Spring 4.3 added ForwardedHeaderFilter which provides a cross container independent way to support X-Forwarded-* headers.
This is nice because not all containers provide the same support. For example, Tomcat does not support X-Forwarded-Host. Another benefit is that ForwardedHeaderFilter allows overriding the context root of the application in the event the proxy uses a different context root than the application. This is helpful to ensure links are rendered correctly.
I'm not sure that we should change the current server.use-forward-headers property to use the filter. Since it's currently supported for Tomcat, Undertow and Jetty I'd rather just see the Tomcat bug fixed.
@philwebb Thanks for the feedback. I think there are a few advantages to using Spring's ForwardedHeaderFilter:
@rwinch Are there any downsides? Filter ordering for example? or performance?
I doubt that performance would be an issue.
The only limitation I can think of is that a Filter would not wrap a ServletRequestListener. However, we already have lots of limitations around listeners.
I am against this, for now at least. When Undertow's native support is used, the HttpServerExchange is marked as secure which has the knock-on effect of marking the request as secure. Anything in Undertow that looks at the exchange will get the wrong answer if we move to the filter-based approach.
@wilkinsona What is looking at the HttpExchange vs the HttpServletRequest (which is correctly overridden)?
Nothing in Undertow itself, other than its MarkSecureHandler which marks every request as secure by configuring an attachment on the exchange. The problem is that we have no way of knowing what other code might be looking in the "wrong" place. For that code, moving to ForwardedHeaderFilter would be a breaking change.
Sorry, but we feel the current approach is probably better overall.
Why not adding server.use-forward-headers-strategy with Enum (can find better name for value) like NATIVE (or CONTAINER), FILTER? As @rwinch explain ForwardedHeaderFilter can be useful for some use cases.
We've decided to reconsider this option and allow this use case with a enum, as suggested by @kakawait.
We should probably deprecate the existing server.use-forward-headers option and only use the enum option.
Most helpful comment
We've decided to reconsider this option and allow this use case with a enum, as suggested by @kakawait.
We should probably deprecate the existing
server.use-forward-headersoption and only use the enum option.