From cec69feb95893bda37c07e3e227879735c9d24dc Mon Sep 17 00:00:00 2001 From: Guirong Hu Date: Wed, 12 Jan 2022 19:53:23 +0800 Subject: [PATCH 1/2] Configure ForwardedHeaderFilter with Tomcat's use relative redirects Previously, when Tomcat was configured to use relative redirects and the ForwardedHeaderFilter is in use, the filter would ignore the use of the relative redirects. This commit corrects this misalignment by applying Tomcat's use relative redirects setting to the filter, but only when Tomcat is being used as the servlet container. See gh-29333 --- ...vletWebServerFactoryAutoConfiguration.java | 36 ++++++++++++++----- ...ebServerFactoryAutoConfigurationTests.java | 31 ++++++++++++++++ 2 files changed, 58 insertions(+), 9 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java index fc3d9c1380..a7d18cddf8 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2020 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -85,15 +85,33 @@ public class ServletWebServerFactoryAutoConfiguration { return new TomcatServletWebServerFactoryCustomizer(serverProperties); } - @Bean - @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) + @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(value = "server.forward-headers-strategy", havingValue = "framework") - public FilterRegistrationBean forwardedHeaderFilter() { - ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); - FilterRegistrationBean registration = new FilterRegistrationBean<>(filter); - registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC, DispatcherType.ERROR); - registration.setOrder(Ordered.HIGHEST_PRECEDENCE); - return registration; + static class ForwardedHeaderFilterConfiguration { + + @Bean + @ConditionalOnClass(name = "org.apache.catalina.startup.Tomcat") + @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) + public FilterRegistrationBean tomcatForwardedHeaderFilter( + ServerProperties serverProperties) { + return createForwardedHeaderFilter(serverProperties.getTomcat().isUseRelativeRedirects()); + } + + @Bean + @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) + public FilterRegistrationBean defaultForwardedHeaderFilter() { + return createForwardedHeaderFilter(false); + } + + private FilterRegistrationBean createForwardedHeaderFilter(boolean relativeRedirects) { + ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); + filter.setRelativeRedirects(relativeRedirects); + FilterRegistrationBean registration = new FilterRegistrationBean<>(filter); + registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC, DispatcherType.ERROR); + registration.setOrder(Ordered.HIGHEST_PRECEDENCE); + return registration; + } + } /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java index 0338bcdd16..ffdc3071a1 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java @@ -57,6 +57,7 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.stereotype.Component; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.filter.ForwardedHeaderFilter; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FrameworkServlet; @@ -368,6 +369,36 @@ class ServletWebServerFactoryAutoConfigurationTests { .run((context) -> assertThat(context).hasSingleBean(FilterRegistrationBean.class)); } + @Test + void relativeRedirectsShouldBeEnabledWhenUsingTomcatContainerAndUseRelativeRedirects() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withPropertyValues("server.forward-headers-strategy=framework", + "server.tomcat.use-relative-redirects=true"); + + runner.run((context) -> { + Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); + Boolean relativeRedirects = (Boolean) ReflectionTestUtils.getField(filter, "relativeRedirects"); + assertThat(relativeRedirects).isTrue(); + }); + } + + @Test + void relativeRedirectsShouldNotBeEnabledWhenNotUsingTomcatContainer() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withClassLoader(new FilteredClassLoader(Tomcat.class)) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withPropertyValues("server.forward-headers-strategy=framework"); + + runner.run((context) -> { + Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); + Boolean relativeRedirects = (Boolean) ReflectionTestUtils.getField(filter, "relativeRedirects"); + assertThat(relativeRedirects).isFalse(); + }); + } + private ContextConsumer verifyContext() { return this::verifyContext; } From 64ee54423a4e1b4e08e0e482ef6eede9dd82b0aa Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 9 Feb 2022 17:27:13 +0000 Subject: [PATCH 2/2] Polish "Configure ForwardedHeaderFilter with Tomcat's use relative redirects" See gh-29333 --- ...vletWebServerFactoryAutoConfiguration.java | 22 +++++++++-------- ...ebServerFactoryAutoConfigurationTests.java | 24 ++++++++++++++----- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java index a7d18cddf8..0537653e99 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfiguration.java @@ -87,25 +87,21 @@ public class ServletWebServerFactoryAutoConfiguration { @Configuration(proxyBeanMethods = false) @ConditionalOnProperty(value = "server.forward-headers-strategy", havingValue = "framework") + @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) static class ForwardedHeaderFilterConfiguration { @Bean @ConditionalOnClass(name = "org.apache.catalina.startup.Tomcat") @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) - public FilterRegistrationBean tomcatForwardedHeaderFilter( - ServerProperties serverProperties) { - return createForwardedHeaderFilter(serverProperties.getTomcat().isUseRelativeRedirects()); + ForwardedHeaderFilterCustomizer tomcatForwardedHeaderFilterCustomizer(ServerProperties serverProperties) { + return (filter) -> filter.setRelativeRedirects(serverProperties.getTomcat().isUseRelativeRedirects()); } @Bean - @ConditionalOnMissingFilterBean(ForwardedHeaderFilter.class) - public FilterRegistrationBean defaultForwardedHeaderFilter() { - return createForwardedHeaderFilter(false); - } - - private FilterRegistrationBean createForwardedHeaderFilter(boolean relativeRedirects) { + FilterRegistrationBean forwardedHeaderFilter( + ObjectProvider customizerProvider) { ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); - filter.setRelativeRedirects(relativeRedirects); + customizerProvider.ifAvailable((customizer) -> customizer.customize(filter)); FilterRegistrationBean registration = new FilterRegistrationBean<>(filter); registration.setDispatcherTypes(DispatcherType.REQUEST, DispatcherType.ASYNC, DispatcherType.ERROR); registration.setOrder(Ordered.HIGHEST_PRECEDENCE); @@ -114,6 +110,12 @@ public class ServletWebServerFactoryAutoConfiguration { } + interface ForwardedHeaderFilterCustomizer { + + void customize(ForwardedHeaderFilter filter); + + } + /** * Registers a {@link WebServerFactoryCustomizerBeanPostProcessor}. Registered via * {@link ImportBeanDefinitionRegistrar} for early registration. diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java index ffdc3071a1..c30bc1c0cf 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/ServletWebServerFactoryAutoConfigurationTests.java @@ -57,7 +57,6 @@ import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.stereotype.Component; -import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.filter.ForwardedHeaderFilter; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FrameworkServlet; @@ -353,6 +352,7 @@ class ServletWebServerFactoryAutoConfigurationTests { assertThat(context).hasSingleBean(FilterRegistrationBean.class); Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); assertThat(filter).isInstanceOf(ForwardedHeaderFilter.class); + assertThat(filter).extracting("relativeRedirects").isEqualTo(false); }); } @@ -376,11 +376,24 @@ class ServletWebServerFactoryAutoConfigurationTests { .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) .withPropertyValues("server.forward-headers-strategy=framework", "server.tomcat.use-relative-redirects=true"); + runner.run((context) -> { + Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); + assertThat(filter).isInstanceOf(ForwardedHeaderFilter.class); + assertThat(filter).extracting("relativeRedirects").isEqualTo(true); + }); + } + @Test + void relativeRedirectsShouldNotBeEnabledWhenUsingTomcatContainerAndNotUsingRelativeRedirects() { + WebApplicationContextRunner runner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) + .withPropertyValues("server.forward-headers-strategy=framework", + "server.tomcat.use-relative-redirects=false"); runner.run((context) -> { Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); - Boolean relativeRedirects = (Boolean) ReflectionTestUtils.getField(filter, "relativeRedirects"); - assertThat(relativeRedirects).isTrue(); + assertThat(filter).isInstanceOf(ForwardedHeaderFilter.class); + assertThat(filter).extracting("relativeRedirects").isEqualTo(false); }); } @@ -391,11 +404,10 @@ class ServletWebServerFactoryAutoConfigurationTests { .withClassLoader(new FilteredClassLoader(Tomcat.class)) .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class)) .withPropertyValues("server.forward-headers-strategy=framework"); - runner.run((context) -> { Filter filter = context.getBean(FilterRegistrationBean.class).getFilter(); - Boolean relativeRedirects = (Boolean) ReflectionTestUtils.getField(filter, "relativeRedirects"); - assertThat(relativeRedirects).isFalse(); + assertThat(filter).isInstanceOf(ForwardedHeaderFilter.class); + assertThat(filter).extracting("relativeRedirects").isEqualTo(false); }); }