Fixes gh-27801
Comment From: wilkinsona
Thanks for the proposal, @terminux, but I think it may be a little too broad. I believe we should only configure the filter to use relative redirects when Tomcat's in use and it has been configured to use relative redirects. If another container's in use or Tomcat's using absolute redirects, the filter's configuration should be unchanged.
Unfortunately, I haven't had time to verify that what I believe is required actually is required or to think about how best to implement it. If you have time to do so, that'd be great. Please don't worry if you don't though and thanks again for your efforts thus far.
Comment From: terminux
Thank you very much for your reply @wilkinsona , nice suggestions. I will reconsider how to implement it based on your suggestion.
Comment From: terminux
Hi @wilkinsona , my idea is to add a static config class specifically for ForwardedHeaderFilter
and we don't automatically enable relative redirects for tomcat containers, only if server.tomcat.use-relative-redirects
is configured to true
, It might be better. I updated the PR and it's ready for your review again.
Comment From: wilkinsona
Thanks, @terminux. When initially thinking about this, I had rejected something along these lines as I didn't want the configuration of the ForwardedHeaderFilter
to know about Tomcat directly. However, having seen your proposal it's quite an elegant and pragmatic solution. It does need to be refined a little as there's no guarantee about the processing order of the two @Bean
methods for the filter. The ordering can be controlled by putting each method in a separate class and @Import
ing those two classes in the desired order.
The alternative to coupling to ForwardedHeaderFilter
configuration directly to Tomcat would be a general callback interface for customizing the ForwardedHeaderFilter
and a Tomcat-specific implementation of it. That would remove the need for multiple @Bean
methods, but would require the new callback interface.
I'll flag this for a team meeting so that we can discuss these options and any others that come to mind.
Comment From: terminux
Thanks @wilkinsona , I originally thought about using @Order
to control the processing order of two @Bean
methods, but it's less elegant. However, when I saw your comment, I found that @Import
is indeed a good choice !
I also thought about using the callback interface way, for example:
interface ForwardedHeaderFilterCustomizer {
void customize(ForwardedHeaderFilter filter);
}
But actually ForwardedHeaderFilter
has only two methods (setRemoveOnly
and setRelativeRedirects
) that may be called. I'm not sure if adding a new callback interface for this would be a bit wasteful, but it scales well.
Comment From: wilkinsona
Thanks very much, @terminux. I went for a customizer callback in the end, but as in implementation detail that isn't part of Boot's public API.