From 959e161555a147cac812915b1677abb46e3a860b Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Wed, 10 Apr 2019 11:28:37 -0700 Subject: [PATCH] Provide an option to use Spring's forwarded header support Previously, if the `server.use-forward-headers` property was set to true, X-Forwarded-* headers support was provided at the server level. The property has been deprecated in favor of `server.forward-headers-strategy` which can be also be configured to use Spring's forwarded header support apart from native server support. Closes gh-5677 --- .../autoconfigure/web/ServerProperties.java | 43 ++++++++++++++++++- .../JettyWebServerFactoryCustomizer.java | 16 +++---- .../NettyWebServerFactoryCustomizer.java | 16 +++---- .../TomcatWebServerFactoryCustomizer.java | 10 +++-- .../UndertowWebServerFactoryCustomizer.java | 10 +++-- ...tiveWebServerFactoryAutoConfiguration.java | 9 ++++ ...vletWebServerFactoryAutoConfiguration.java | 17 ++++++++ ...ebServerFactoryAutoConfigurationTests.java | 18 ++++++++ ...ebServerFactoryAutoConfigurationTests.java | 22 ++++++++++ .../src/main/asciidoc/howto.adoc | 8 ++-- 10 files changed, 139 insertions(+), 30 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java index 86916226fd..5fac543a08 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.TimeZone; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.boot.convert.DurationUnit; import org.springframework.boot.web.server.Compression; @@ -79,6 +80,11 @@ public class ServerProperties { */ private Boolean useForwardHeaders; + /** + * Strategy for handling X-Forwarded-* headers. + */ + private ForwardHeadersStrategy forwardHeadersStrategy = ForwardHeadersStrategy.NONE; + /** * Value to use for the Server response header (if empty, no header is sent). */ @@ -129,12 +135,18 @@ public class ServerProperties { this.address = address; } + @DeprecatedConfigurationProperty public Boolean isUseForwardHeaders() { - return this.useForwardHeaders; + return ForwardHeadersStrategy.NATIVE.equals(this.forwardHeadersStrategy); } public void setUseForwardHeaders(Boolean useForwardHeaders) { - this.useForwardHeaders = useForwardHeaders; + if (useForwardHeaders != null && useForwardHeaders) { + this.forwardHeadersStrategy = ForwardHeadersStrategy.NATIVE; + } + else { + this.forwardHeadersStrategy = ForwardHeadersStrategy.NONE; + } } public String getServerHeader() { @@ -197,6 +209,14 @@ public class ServerProperties { return this.undertow; } + public ForwardHeadersStrategy getForwardHeadersStrategy() { + return this.forwardHeadersStrategy; + } + + public void setForwardHeadersStrategy(ForwardHeadersStrategy forwardHeadersStrategy) { + this.forwardHeadersStrategy = forwardHeadersStrategy; + } + /** * Servlet properties. */ @@ -1208,4 +1228,23 @@ public class ServerProperties { } + public enum ForwardHeadersStrategy { + + /** + * Use the underlying container's native support for forwarded headers. + */ + NATIVE, + + /** + * Use Spring's support for handling forwarded headers. + */ + FRAMEWORK, + + /** + * Ignore X-Forwarded-* headers. + */ + NONE + + } + } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java index 555868ff0f..f5fda1d7ac 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/JettyWebServerFactoryCustomizer.java @@ -68,8 +68,7 @@ public class JettyWebServerFactoryCustomizer implements public void customize(ConfigurableJettyWebServerFactory factory) { ServerProperties properties = this.serverProperties; ServerProperties.Jetty jettyProperties = properties.getJetty(); - factory.setUseForwardHeaders( - getOrDeduceUseForwardHeaders(properties, this.environment)); + factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders()); PropertyMapper propertyMapper = PropertyMapper.get(); propertyMapper.from(jettyProperties::getAcceptors).whenNonNull() .to(factory::setAcceptors); @@ -95,13 +94,14 @@ public class JettyWebServerFactoryCustomizer implements return value > 0; } - private boolean getOrDeduceUseForwardHeaders(ServerProperties serverProperties, - Environment environment) { - if (serverProperties.isUseForwardHeaders() != null) { - return serverProperties.isUseForwardHeaders(); + private boolean getOrDeduceUseForwardHeaders() { + if (this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NONE)) { + CloudPlatform platform = CloudPlatform.getActive(this.environment); + return platform != null && platform.isUsingForwardHeaders(); } - CloudPlatform platform = CloudPlatform.getActive(environment); - return platform != null && platform.isUsingForwardHeaders(); + return this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NATIVE); } private void customizeConnectionTimeout(ConfigurableJettyWebServerFactory factory, diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java index 549d7244a4..a93ff79742 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java @@ -58,8 +58,7 @@ public class NettyWebServerFactoryCustomizer @Override public void customize(NettyReactiveWebServerFactory factory) { - factory.setUseForwardHeaders( - getOrDeduceUseForwardHeaders(this.serverProperties, this.environment)); + factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders()); PropertyMapper propertyMapper = PropertyMapper.get(); propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize).whenNonNull() .asInt(DataSize::toBytes) @@ -70,13 +69,14 @@ public class NettyWebServerFactoryCustomizer .addServerCustomizers(getConnectionTimeOutCustomizer(duration))); } - private boolean getOrDeduceUseForwardHeaders(ServerProperties serverProperties, - Environment environment) { - if (serverProperties.isUseForwardHeaders() != null) { - return serverProperties.isUseForwardHeaders(); + private boolean getOrDeduceUseForwardHeaders() { + if (this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NONE)) { + CloudPlatform platform = CloudPlatform.getActive(this.environment); + return platform != null && platform.isUsingForwardHeaders(); } - CloudPlatform platform = CloudPlatform.getActive(environment); - return platform != null && platform.isUsingForwardHeaders(); + return this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NATIVE); } private void customizeMaxHttpHeaderSize(NettyReactiveWebServerFactory factory, diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java index 2583bae330..0eb58a7864 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/TomcatWebServerFactoryCustomizer.java @@ -188,11 +188,13 @@ public class TomcatWebServerFactoryCustomizer implements } private boolean getOrDeduceUseForwardHeaders() { - if (this.serverProperties.isUseForwardHeaders() != null) { - return this.serverProperties.isUseForwardHeaders(); + if (this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NONE)) { + CloudPlatform platform = CloudPlatform.getActive(this.environment); + return platform != null && platform.isUsingForwardHeaders(); } - CloudPlatform platform = CloudPlatform.getActive(this.environment); - return platform != null && platform.isUsingForwardHeaders(); + return this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NATIVE); } @SuppressWarnings("rawtypes") diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java index cfb8214011..3859d4e71f 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/UndertowWebServerFactoryCustomizer.java @@ -124,11 +124,13 @@ public class UndertowWebServerFactoryCustomizer implements } private boolean getOrDeduceUseForwardHeaders() { - if (this.serverProperties.isUseForwardHeaders() != null) { - return this.serverProperties.isUseForwardHeaders(); + if (this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NONE)) { + CloudPlatform platform = CloudPlatform.getActive(this.environment); + return platform != null && platform.isUsingForwardHeaders(); } - CloudPlatform platform = CloudPlatform.getActive(this.environment); - return platform != null && platform.isUsingForwardHeaders(); + return this.serverProperties.getForwardHeadersStrategy() + .equals(ServerProperties.ForwardHeadersStrategy.NATIVE); } } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfiguration.java index 3fe0896daa..72ead1b2cf 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfiguration.java @@ -25,6 +25,7 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigureOrder; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -37,6 +38,7 @@ import org.springframework.core.Ordered; import org.springframework.core.type.AnnotationMetadata; import org.springframework.http.ReactiveHttpInputMessage; import org.springframework.util.ObjectUtils; +import org.springframework.web.server.adapter.ForwardedHeaderTransformer; /** * {@link EnableAutoConfiguration Auto-configuration} for a reactive web server. @@ -62,6 +64,13 @@ public class ReactiveWebServerFactoryAutoConfiguration { return new ReactiveWebServerFactoryCustomizer(serverProperties); } + @Bean + @ConditionalOnProperty(value = "server.forward-headers-strategy", + havingValue = "framework") + public ForwardedHeaderTransformer forwardedHeaderTransformer() { + return new ForwardedHeaderTransformer(); + } + /** * Registers a {@link WebServerFactoryCustomizerBeanPostProcessor}. Registered via * {@link ImportBeanDefinitionRegistrar} for early registration. 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 743c107285..642b427cfd 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 @@ -16,6 +16,7 @@ package org.springframework.boot.autoconfigure.web.servlet; +import javax.servlet.DispatcherType; import javax.servlet.ServletRequest; import org.springframework.beans.BeansException; @@ -27,12 +28,14 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigureOrder; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.web.server.ErrorPageRegistrarBeanPostProcessor; import org.springframework.boot.web.server.WebServerFactoryCustomizerBeanPostProcessor; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -40,6 +43,7 @@ import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; import org.springframework.core.Ordered; import org.springframework.core.type.AnnotationMetadata; import org.springframework.util.ObjectUtils; +import org.springframework.web.filter.ForwardedHeaderFilter; /** * {@link EnableAutoConfiguration Auto-configuration} for servlet web servers. @@ -74,6 +78,19 @@ public class ServletWebServerFactoryAutoConfiguration { return new TomcatServletWebServerFactoryCustomizer(serverProperties); } + @Bean + @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; + } + /** * 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/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java index f116f96174..fb71b10c9a 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/ReactiveWebServerFactoryAutoConfigurationTests.java @@ -34,6 +34,7 @@ import org.springframework.context.ApplicationContextException; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.server.reactive.HttpHandler; +import org.springframework.web.server.adapter.ForwardedHeaderTransformer; import static org.assertj.core.api.Assertions.assertThat; @@ -42,6 +43,7 @@ import static org.assertj.core.api.Assertions.assertThat; * * @author Brian Clozel * @author Raheela Aslam + * @author Madhura Bhave */ public class ReactiveWebServerFactoryAutoConfigurationTests { @@ -147,6 +149,22 @@ public class ReactiveWebServerFactoryAutoConfigurationTests { }); } + @Test + public void forwardedHeaderTransformerShouldBeConfigured() { + this.contextRunner.withUserConfiguration(HttpHandlerConfiguration.class) + .withPropertyValues("server.forward-headers-strategy=framework") + .run((context) -> assertThat(context) + .hasSingleBean(ForwardedHeaderTransformer.class)); + } + + @Test + public void forwardedHeaderTransformerWhenStrategyNotFilterShouldNotBeConfigured() { + this.contextRunner.withUserConfiguration(HttpHandlerConfiguration.class) + .withPropertyValues("server.forward-headers-strategy=native") + .run((context) -> assertThat(context) + .doesNotHaveBean(ForwardedHeaderTransformer.class)); + } + @Configuration(proxyBeanMethods = false) protected static class HttpHandlerConfiguration { 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 33b11dbd8b..d070725c8b 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 @@ -16,6 +16,7 @@ package org.springframework.boot.autoconfigure.web.servlet; +import javax.servlet.Filter; import javax.servlet.Servlet; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; @@ -35,6 +36,7 @@ import org.springframework.boot.web.embedded.tomcat.TomcatContextCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatProtocolHandlerCustomizer; import org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory; import org.springframework.boot.web.server.WebServerFactoryCustomizer; +import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.springframework.boot.web.servlet.server.ConfigurableServletWebServerFactory; @@ -43,6 +45,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.web.filter.ForwardedHeaderFilter; import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.FrameworkServlet; @@ -56,6 +59,7 @@ import static org.mockito.Mockito.verify; * @author Phillip Webb * @author Stephane Nicoll * @author Raheela Aslam + * @author Madhura Bhave */ public class ServletWebServerFactoryAutoConfigurationTests { @@ -186,6 +190,24 @@ public class ServletWebServerFactoryAutoConfigurationTests { }); } + @Test + public void forwardedHeaderFilterShouldBeConfigured() { + this.contextRunner.withPropertyValues("server.forward-headers-strategy=framework") + .run((context) -> { + assertThat(context).hasSingleBean(FilterRegistrationBean.class); + Filter filter = context.getBean(FilterRegistrationBean.class) + .getFilter(); + assertThat(filter).isInstanceOf(ForwardedHeaderFilter.class); + }); + } + + @Test + public void forwardedHeaderFilterWhenStrategyNotFilterShouldNotBeConfigured() { + this.contextRunner.withPropertyValues("server.forward-headers-strategy=native") + .run((context) -> assertThat(context) + .doesNotHaveBean(FilterRegistrationBean.class)); + } + private ContextConsumer verifyContext() { return this::verifyContext; } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc index 5c19037004..78d609bbe8 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc @@ -969,11 +969,11 @@ construct links to itself. If the proxy adds conventional `X-Forwarded-For` and `X-Forwarded-Proto` headers (most proxy servers do so), the absolute links should be rendered correctly, provided -`server.use-forward-headers` is set to `true` in your `application.properties`. +`server.forward-headers-strategy` is set to `NATIVE` or `FRAMEWORK` in your `application.properties`. NOTE: If your application runs in Cloud Foundry or Heroku, the -`server.use-forward-headers` property defaults to `true`. In all -other instances, it defaults to `false`. +`server.forward-headers-strategy` property defaults to `NATIVE`. In all +other instances, it defaults to `NONE`. @@ -1006,7 +1006,7 @@ NOTE: You can trust all proxies by setting the `internal-proxies` to empty (but so in production). You can take complete control of the configuration of Tomcat's `RemoteIpValve` by -switching the automatic one off (to do so, set `server.use-forward-headers=false`) and +switching the automatic one off (to do so, set `server.forward-headers-strategy=NONE`) and adding a new valve instance in a `TomcatServletWebServerFactory` bean.