Unify ServerProperties X-Forwarded-For support

Add a new `server.use-forward-headers` property which can be used to
switch on X-Forwarded-For header support in all supported embedded
servlet containers.

This commit reverts the decision to enable `RemoteIpValve` with Tomcat
by default (gh-3782) and requires that either `user-forward-headers` is
set to true or that `server.tomcat.protocol-header` or
`server.tomcat.remote-ip-header` are set.

See gh-4018
See gh-3782

spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/ServerProperties.java

@@ -47,6 +47,7 @@ import org.springframework.boot.context.embedded.InitParameterConfiguringServlet
 import org.springframework.boot.context.embedded.JspServlet;
 import org.springframework.boot.context.embedded.ServletContextInitializer;
 import org.springframework.boot.context.embedded.Ssl;
+import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory;
 import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer;
 import org.springframework.boot.context.embedded.tomcat.TomcatContextCustomizer;
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory;
@@ -102,6 +103,11 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 	 */
 	private final Map<String, String> contextParameters = new HashMap<String, String>();
 
+	/**
+	 * If X-Forwarded-* headers should be applied to the HttpRequest.
+	 */
+	private boolean useForwardHeaders;
+
 	private Session session = new Session();
 
 	@NestedConfigurationProperty
@@ -115,6 +121,8 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 
 	private final Tomcat tomcat = new Tomcat();
 
+	private final Jetty jetty = new Jetty();
+
 	private final Undertow undertow = new Undertow();
 
 	@Override
@@ -150,11 +158,16 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 			container.setCompression(getCompression());
 		}
 		if (container instanceof TomcatEmbeddedServletContainerFactory) {
-			getTomcat()
-					.customizeTomcat((TomcatEmbeddedServletContainerFactory) container);
+			getTomcat().customizeTomcat(this,
+					(TomcatEmbeddedServletContainerFactory) container);
+		}
+		if (container instanceof JettyEmbeddedServletContainerFactory) {
+			getJetty().customizeJetty(this,
+					(JettyEmbeddedServletContainerFactory) container);
 		}
+
 		if (container instanceof UndertowEmbeddedServletContainerFactory) {
-			getUndertow().customizeUndertow(
+			getUndertow().customizeUndertow(this,
 					(UndertowEmbeddedServletContainerFactory) container);
 		}
 		container.addInitializers(new SessionConfiguringInitializer(this.session));
@@ -267,6 +280,14 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 		return this.contextParameters;
 	}
 
+	public boolean isUseForwardHeaders() {
+		return this.useForwardHeaders;
+	}
+
+	public void setUseForwardHeaders(boolean useForwardHeaders) {
+		this.useForwardHeaders = useForwardHeaders;
+	}
+
 	/**
 	 * Get the session timeout.
 	 * @return the session timeout
@@ -320,6 +341,10 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 		return this.tomcat;
 	}
 
+	private Jetty getJetty() {
+		return this.jetty;
+	}
+
 	public Undertow getUndertow() {
 		return this.undertow;
 	}
@@ -488,9 +513,8 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 
 		/**
 		 * Header that holds the incoming protocol, usually named "X-Forwarded-Proto".
-		 * Configures a RemoteIpValve only if remoteIpHeader is also set.
 		 */
-		private String protocolHeader = "x-forwarded-proto";
+		private String protocolHeader;
 
 		/**
 		 * Value of the protocol header that indicates that the incoming request uses SSL.
@@ -500,13 +524,12 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 		/**
 		 * Name of the HTTP header used to override the original port value.
 		 */
-		private String portHeader = "x-forwarded-port";
+		private String portHeader = "X-Forwarded-Port";
 
 		/**
-		 * Name of the http header from which the remote ip is extracted. Configures a
-		 * RemoteIpValve only if protocolHeader is also set.
+		 * Name of the http header from which the remote ip is extracted..
 		 */
-		private String remoteIpHeader = "x-forwarded-for";
+		private String remoteIpHeader;
 
 		/**
 		 * Tomcat base directory. If not specified a temporary directory will be used.
@@ -659,12 +682,13 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 			this.uriEncoding = uriEncoding;
 		}
 
-		void customizeTomcat(TomcatEmbeddedServletContainerFactory factory) {
+		void customizeTomcat(ServerProperties serverProperties,
+				TomcatEmbeddedServletContainerFactory factory) {
 			if (getBasedir() != null) {
 				factory.setBaseDirectory(getBasedir());
 			}
 			customizeBackgroundProcessorDelay(factory);
-			customizeHeaders(factory);
+			customizeRemoteIpValve(serverProperties, factory);
 			if (this.maxThreads > 0) {
 				customizeMaxThreads(factory);
 			}
@@ -691,14 +715,20 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 			});
 		}
 
-		private void customizeHeaders(TomcatEmbeddedServletContainerFactory factory) {
-			String remoteIpHeader = getRemoteIpHeader();
+		private void customizeRemoteIpValve(ServerProperties properties,
+				TomcatEmbeddedServletContainerFactory factory) {
 			String protocolHeader = getProtocolHeader();
-			if (StringUtils.hasText(remoteIpHeader)
-					&& StringUtils.hasText(protocolHeader)) {
+			String remoteIpHeader = getRemoteIpHeader();
+			// For back compatibility the valve is also enabled if protocol-header is set
+			if (StringUtils.hasText(protocolHeader)
+					|| StringUtils.hasText(remoteIpHeader)
+					|| properties.isUseForwardHeaders()) {
 				RemoteIpValve valve = new RemoteIpValve();
-				valve.setRemoteIpHeader(remoteIpHeader);
-				valve.setProtocolHeader(protocolHeader);
+				valve.setProtocolHeader(StringUtils.hasLength(protocolHeader) ? protocolHeader
+						: "X-Forwarded-Proto");
+				if (StringUtils.hasLength(remoteIpHeader)) {
+					valve.setRemoteIpHeader(remoteIpHeader);
+				}
 				// The internal proxies default to a white list of "safe" internal IP
 				// addresses
 				valve.setInternalProxies(getInternalProxies());
@@ -822,6 +852,15 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 
 	}
 
+	private static class Jetty {
+
+		void customizeJetty(ServerProperties serverProperties,
+				JettyEmbeddedServletContainerFactory factory) {
+			factory.setUseForwardHeaders(serverProperties.isUseForwardHeaders());
+		}
+
+	}
+
 	public static class Undertow {
 
 		/**
@@ -958,7 +997,8 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 			getAccesslog().setDir(accessLogDir);
 		}
 
-		void customizeUndertow(UndertowEmbeddedServletContainerFactory factory) {
+		void customizeUndertow(ServerProperties serverProperties,
+				UndertowEmbeddedServletContainerFactory factory) {
 			factory.setBufferSize(this.bufferSize);
 			factory.setBuffersPerRegion(this.buffersPerRegion);
 			factory.setIoThreads(this.ioThreads);
@@ -967,6 +1007,7 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer, Ord
 			factory.setAccessLogDirectory(this.accesslog.dir);
 			factory.setAccessLogPattern(this.accesslog.pattern);
 			factory.setAccessLogEnabled(this.accesslog.enabled);
+			factory.setUseForwardHeaders(serverProperties.isUseForwardHeaders());
 		}
 
 		public static class Accesslog {

spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesTests.java

@@ -39,7 +39,9 @@ import org.springframework.beans.MutablePropertyValues;
 import org.springframework.boot.bind.RelaxedDataBinder;
 import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainer;
 import org.springframework.boot.context.embedded.ServletContextInitializer;
+import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory;
 import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory;
+import org.springframework.boot.context.embedded.undertow.UndertowEmbeddedServletContainerFactory;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -50,6 +52,7 @@ import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
 /**
@@ -252,27 +255,38 @@ public class ServerPropertiesTests {
 		map.put("server.tomcat.remote_ip_header", "");
 		map.put("server.tomcat.protocol_header", "");
 		bindProperties(map);
-
 		TomcatEmbeddedServletContainerFactory container = new TomcatEmbeddedServletContainerFactory();
 		this.properties.customize(container);
-
 		assertEquals(0, container.getValves().size());
 	}
 
 	@Test
 	public void defaultTomcatRemoteIpValve() throws Exception {
-		// Since 1.3.0 no need to explicitly set header names
+		Map<String, String> map = new HashMap<String, String>();
+		// Since 1.1.7 you need to specify at least the protocol
+		map.put("server.tomcat.protocol_header", "X-Forwarded-Proto");
+		map.put("server.tomcat.remote_ip_header", "X-Forwarded-For");
+		bindProperties(map);
+		testRemoteIpValveConfigured();
+	}
+
+	@Test
+	public void setUseForwardHeadersTomcat() throws Exception {
+		// Since 1.3.0 no need to explicitly set header names if use-forward-header=true
+		this.properties.setUseForwardHeaders(true);
+		testRemoteIpValveConfigured();
+	}
+
+	private void testRemoteIpValveConfigured() {
 		TomcatEmbeddedServletContainerFactory container = new TomcatEmbeddedServletContainerFactory();
 		this.properties.customize(container);
-
 		assertEquals(1, container.getValves().size());
 		Valve valve = container.getValves().iterator().next();
 		assertThat(valve, instanceOf(RemoteIpValve.class));
 		RemoteIpValve remoteIpValve = (RemoteIpValve) valve;
-		assertEquals("x-forwarded-proto", remoteIpValve.getProtocolHeader());
+		assertEquals("X-Forwarded-Proto", remoteIpValve.getProtocolHeader());
 		assertEquals("https", remoteIpValve.getProtocolHeaderHttpsValue());
-		assertEquals("x-forwarded-for", remoteIpValve.getRemoteIpHeader());
-
+		assertEquals("X-Forwarded-For", remoteIpValve.getRemoteIpHeader());
 		String expectedInternalProxies = "10\\.\\d{1,3}\\.\\d{1,3}\\.\\d{1,3}|" // 10/8
 				+ "192\\.168\\.\\d{1,3}\\.\\d{1,3}|" // 192.168/16
 				+ "169\\.254\\.\\d{1,3}\\.\\d{1,3}|" // 169.254/16
@@ -280,7 +294,6 @@ public class ServerPropertiesTests {
 				+ "172\\.1[6-9]{1}\\.\\d{1,3}\\.\\d{1,3}|" // 172.16/12
 				+ "172\\.2[0-9]{1}\\.\\d{1,3}\\.\\d{1,3}|"
 				+ "172\\.3[0-1]{1}\\.\\d{1,3}\\.\\d{1,3}";
-
 		assertEquals(expectedInternalProxies, remoteIpValve.getInternalProxies());
 	}
 
@@ -308,6 +321,22 @@ public class ServerPropertiesTests {
 		assertEquals("192.168.0.1", remoteIpValve.getInternalProxies());
 	}
 
+	@Test
+	public void setUseForwardHeadersUndertow() throws Exception {
+		this.properties.setUseForwardHeaders(true);
+		UndertowEmbeddedServletContainerFactory container = spy(new UndertowEmbeddedServletContainerFactory());
+		this.properties.customize(container);
+		verify(container).setUseForwardHeaders(true);
+	}
+
+	@Test
+	public void setUseForwardHeadersJetty() throws Exception {
+		this.properties.setUseForwardHeaders(true);
+		JettyEmbeddedServletContainerFactory container = spy(new JettyEmbeddedServletContainerFactory());
+		this.properties.customize(container);
+		verify(container).setUseForwardHeaders(true);
+	}
+
 	private void bindProperties(Map<String, String> map) {
 		new RelaxedDataBinder(this.properties, "server").bind(new MutablePropertyValues(
 				map));