Commit c12a3f41 authored by Madhura Bhave's avatar Madhura Bhave

Support explicitly setting forward headers strategy to NONE

Prior to this commit, there was no distinction between explicitly setting
forward headers strategy to a value of NONE and not setting it at all.
This meant that in a cloud environment, a cloud provider was always checked
to see if it was active and using forward headers and there was no way to
prevent that.

This commit changes the default value of the property to null so that there
is a way to determine if the property was explicitly set to NONE.

Fixes gh-19333
parent 8e285a43
......@@ -84,7 +84,7 @@ public class ServerProperties {
/**
* Strategy for handling X-Forwarded-* headers.
*/
private ForwardHeadersStrategy forwardHeadersStrategy = ForwardHeadersStrategy.NONE;
private ForwardHeadersStrategy forwardHeadersStrategy;
/**
* Value to use for the Server response header (if empty, no header is sent).
......
......@@ -102,7 +102,7 @@ public class JettyWebServerFactoryCustomizer
}
private boolean getOrDeduceUseForwardHeaders() {
if (this.serverProperties.getForwardHeadersStrategy().equals(ServerProperties.ForwardHeadersStrategy.NONE)) {
if (this.serverProperties.getForwardHeadersStrategy() == null) {
CloudPlatform platform = CloudPlatform.getActive(this.environment);
return platform != null && platform.isUsingForwardHeaders();
}
......
......@@ -68,7 +68,7 @@ public class NettyWebServerFactoryCustomizer
}
private boolean getOrDeduceUseForwardHeaders() {
if (this.serverProperties.getForwardHeadersStrategy().equals(ServerProperties.ForwardHeadersStrategy.NONE)) {
if (this.serverProperties.getForwardHeadersStrategy() == null) {
CloudPlatform platform = CloudPlatform.getActive(this.environment);
return platform != null && platform.isUsingForwardHeaders();
}
......
......@@ -201,7 +201,7 @@ public class TomcatWebServerFactoryCustomizer
}
private boolean getOrDeduceUseForwardHeaders() {
if (this.serverProperties.getForwardHeadersStrategy().equals(ServerProperties.ForwardHeadersStrategy.NONE)) {
if (this.serverProperties.getForwardHeadersStrategy() == null) {
CloudPlatform platform = CloudPlatform.getActive(this.environment);
return platform != null && platform.isUsingForwardHeaders();
}
......
......@@ -123,7 +123,7 @@ public class UndertowWebServerFactoryCustomizer
}
private boolean getOrDeduceUseForwardHeaders() {
if (this.serverProperties.getForwardHeadersStrategy().equals(ServerProperties.ForwardHeadersStrategy.NONE)) {
if (this.serverProperties.getForwardHeadersStrategy() == null) {
CloudPlatform platform = CloudPlatform.getActive(this.environment);
return platform != null && platform.isUsingForwardHeaders();
}
......
......@@ -86,6 +86,23 @@ class JettyWebServerFactoryCustomizerTests {
verify(factory).setUseForwardHeaders(false);
}
@Test
void forwardHeadersWhenStrategyIsNativeShouldConfigureValve() {
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NATIVE);
ConfigurableJettyWebServerFactory factory = mock(ConfigurableJettyWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(true);
}
@Test
void forwardHeadersWhenStrategyIsNoneShouldNotConfigureValve() {
this.environment.setProperty("DYNO", "-");
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NONE);
ConfigurableJettyWebServerFactory factory = mock(ConfigurableJettyWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(false);
}
@Test
void accessLogCanBeCustomized() throws IOException {
File logFile = File.createTempFile("jetty_log", ".log");
......
......@@ -92,6 +92,23 @@ class NettyWebServerFactoryCustomizerTests {
verify(factory).setUseForwardHeaders(true);
}
@Test
void forwardHeadersWhenStrategyIsNativeShouldConfigureValve() {
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NATIVE);
NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(true);
}
@Test
void forwardHeadersWhenStrategyIsNoneShouldNotConfigureValve() {
this.environment.setProperty("DYNO", "-");
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NONE);
NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(false);
}
@Test
void setServerConnectionTimeoutAsZero() {
setupServerConnectionTimeout(Duration.ZERO);
......
......@@ -234,6 +234,26 @@ class TomcatWebServerFactoryCustomizerTests {
testRemoteIpValveConfigured();
}
@Test
void defaultUseForwardHeaders() {
TomcatServletWebServerFactory factory = customizeAndGetFactory();
assertThat(factory.getEngineValves()).hasSize(0);
}
@Test
void forwardHeadersWhenStrategyIsNativeShouldConfigureValve() {
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NATIVE);
testRemoteIpValveConfigured();
}
@Test
void forwardHeadersWhenStrategyIsNoneShouldNotConfigureValve() {
this.environment.setProperty("DYNO", "-");
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NONE);
TomcatServletWebServerFactory factory = customizeAndGetFactory();
assertThat(factory.getEngineValves()).hasSize(0);
}
@Test
void defaultRemoteIpValve() {
// Since 1.1.7 you need to specify at least the protocol
......
......@@ -202,6 +202,23 @@ class UndertowWebServerFactoryCustomizerTests {
verify(factory).setUseForwardHeaders(true);
}
@Test
void forwardHeadersWhenStrategyIsNativeShouldConfigureValve() {
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NATIVE);
ConfigurableUndertowWebServerFactory factory = mock(ConfigurableUndertowWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(true);
}
@Test
void forwardHeadersWhenStrategyIsNoneShouldNotConfigureValve() {
this.environment.setProperty("DYNO", "-");
this.serverProperties.setForwardHeadersStrategy(ServerProperties.ForwardHeadersStrategy.NONE);
ConfigurableUndertowWebServerFactory factory = mock(ConfigurableUndertowWebServerFactory.class);
this.customizer.customize(factory);
verify(factory).setUseForwardHeaders(false);
}
private <T> T boundServerOption(Option<T> option) {
Builder builder = Undertow.builder();
ConfigurableUndertowWebServerFactory factory = mockFactory(builder);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment