Defer to Tomcat's default for use-relative-redirects

Spring Boot unconditionally set useRelativeRedirects on the Tomcat
Context, defaulting it to false. That overrode Tomcat's own default and
forced absolute Location headers on every sendRedirect.

Tomcat's default is not a constant. StandardContext declares it as
!Globals.STRICT_SERVLET_COMPLIANCE, so it is true normally and false
under strict servlet compliance. Simply flipping Boot's default to true
would still override Tomcat, just in the other direction, and precisely
for users who opted into strict compliance.

Make server.tomcat.use-relative-redirects a nullable Boolean that is
only applied when set, mirroring the sibling redirect-context-root
property. When it is left unset Boot no longer touches the setting and
Tomcat's own default wins in every mode. Setting the property explicitly
continues to work in both directions.

This changes the accessors from isUseRelativeRedirects()/
setUseRelativeRedirects(boolean) to getUseRelativeRedirects()/
setUseRelativeRedirects(Boolean).

Smoke tests that asserted a port-qualified absolute Location are updated
to the relative form, and the proxy tip in the reference documentation is
qualified since the context root redirect no longer carries a scheme.

See gh-50900

Signed-off-by: Tiziano Basile <tiziano.basile@nearform.com>

Author: basteez

documentation/spring-boot-docs/src/docs/antora/modules/how-to/pages/webserver.adoc

@@ -562,8 +562,9 @@ server:
 
 NOTE: You can trust all proxies by setting the `internal-proxies` to empty (but do not do so in production).
 
-TIP: If you are using Tomcat and terminating SSL at the proxy, configprop:server.tomcat.redirect-context-root[] should be set to `false`.
+TIP: If you are using Tomcat, terminating SSL at the proxy, and have set configprop:server.tomcat.use-relative-redirects[] to `false`, then configprop:server.tomcat.redirect-context-root[] should also be set to `false`.
 This allows the `X-Forwarded-Proto` header to be honored before any redirects are performed.
+When relative redirects are in use, which is Tomcat's default, the context root redirect carries no scheme so there is nothing for the header to correct.
 
 You can take complete control of the configuration of Tomcat's javadoc:org.apache.catalina.valves.RemoteIpValve[] by switching the automatic one off (to do so, set `server.forward-headers-strategy=NONE`) and adding a new valve instance using a javadoc:org.springframework.boot.web.server.WebServerFactoryCustomizer[] bean.
 

module/spring-boot-tomcat/src/main/java/org/springframework/boot/tomcat/autoconfigure/TomcatServerProperties.java

@@ -56,6 +56,7 @@
  * @author Florian Storz
  * @author Michael Weidmann
  * @author Lasse Wulff
+ * @author Tiziano Basile
  * @since 4.0.0
  */
 @ConfigurationProperties("server.tomcat")
@@ -103,9 +104,11 @@ public class TomcatServerProperties {
 
 	/**
 	 * Whether HTTP 1.1 and later location headers generated by a call to sendRedirect
-	 * will use relative or absolute redirects.
+	 * will use relative or absolute redirects. When not set, Tomcat's own default is
+	 * used, which is relative unless strict servlet compliance is enabled. Has no effect
+	 * on a reactive web server.
 	 */
-	private boolean useRelativeRedirects;
+	private @Nullable Boolean useRelativeRedirects;
 
 	/**
 	 * Character encoding to use to decode the URI.
@@ -235,11 +238,11 @@ public void setRedirectContextRoot(Boolean redirectContextRoot) {
 		this.redirectContextRoot = redirectContextRoot;
 	}
 
-	public boolean isUseRelativeRedirects() {
+	public @Nullable Boolean getUseRelativeRedirects() {
 		return this.useRelativeRedirects;
 	}
 
-	public void setUseRelativeRedirects(boolean useRelativeRedirects) {
+	public void setUseRelativeRedirects(@Nullable Boolean useRelativeRedirects) {
 		this.useRelativeRedirects = useRelativeRedirects;
 	}
 

module/spring-boot-tomcat/src/main/java/org/springframework/boot/tomcat/autoconfigure/servlet/TomcatServletWebServerFactoryCustomizer.java

@@ -29,6 +29,7 @@
  *
  * @author Brian Clozel
  * @author Phillip Webb
+ * @author Tiziano Basile
  */
 class TomcatServletWebServerFactoryCustomizer
 		implements WebServerFactoryCustomizer<TomcatServletWebServerFactory>, Ordered {
@@ -52,7 +53,9 @@ public void customize(TomcatServletWebServerFactory factory) {
 		if (this.tomcatProperties.getRedirectContextRoot() != null) {
 			customizeRedirectContextRoot(factory, this.tomcatProperties.getRedirectContextRoot());
 		}
-		customizeUseRelativeRedirects(factory, this.tomcatProperties.isUseRelativeRedirects());
+		if (this.tomcatProperties.getUseRelativeRedirects() != null) {
+			customizeUseRelativeRedirects(factory, this.tomcatProperties.getUseRelativeRedirects());
+		}
 	}
 
 	private void customizeRedirectContextRoot(ConfigurableTomcatWebServerFactory factory, boolean redirectContextRoot) {

module/spring-boot-tomcat/src/test/java/org/springframework/boot/tomcat/autoconfigure/TomcatServerPropertiesTests.java

@@ -45,6 +45,7 @@
  * Tests for {@link TomcatServerProperties}.
  *
  * @author Andy Wilkinson
+ * @author Tiziano Basile
  */
 class TomcatServerPropertiesTests {
 
@@ -95,7 +96,7 @@ void testTomcatBinding() {
 		assertThat(this.properties.getBackgroundProcessorDelay()).hasSeconds(10);
 		assertThat(this.properties.getRelaxedPathChars()).containsExactly('|', '<');
 		assertThat(this.properties.getRelaxedQueryChars()).containsExactly('^', '|');
-		assertThat(this.properties.isUseRelativeRedirects()).isTrue();
+		assertThat(this.properties.getUseRelativeRedirects()).isTrue();
 	}
 
 	@Test
@@ -235,8 +236,8 @@ void tomcatInternalProxiesMatchesDefault() {
 	}
 
 	@Test
-	void tomcatUseRelativeRedirectsDefaultsToFalse() {
-		assertThat(this.properties.isUseRelativeRedirects()).isFalse();
+	void tomcatUseRelativeRedirectsIsNotSetByDefault() {
+		assertThat(this.properties.getUseRelativeRedirects()).isNull();
 	}
 
 	@Test

module/spring-boot-tomcat/src/test/java/org/springframework/boot/tomcat/autoconfigure/servlet/TomcatServletWebServerFactoryCustomizerTests.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.tomcat.autoconfigure.servlet;
 
 import org.apache.catalina.Context;
+import org.apache.catalina.core.StandardContext;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -30,11 +31,16 @@
 import org.springframework.test.context.support.TestPropertySourceUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.BDDMockito.then;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 
 /**
  * Tests for {@link TomcatServletWebServerFactoryCustomizer}.
  *
  * @author Phillip Webb
+ * @author Tiziano Basile
  */
 class TomcatServletWebServerFactoryCustomizerTests {
 
@@ -80,14 +86,39 @@ void redirectContextRootCanBeConfigured() {
 	}
 
 	@Test
-	void useRelativeRedirectsCanBeConfigured() {
+	void useRelativeRedirectsWhenNotSetDoesNotCustomizeContext() {
+		TomcatServletWebServerFactory factory = customizeAndGetFactory();
+		Context context = mock(Context.class);
+		factory.getContextCustomizers().forEach((customizer) -> customizer.customize(context));
+		then(context).should(never()).setUseRelativeRedirects(anyBoolean());
+	}
+
+	@Test
+	void useRelativeRedirectsWhenNotSetUsesTomcatsDefault() {
+		assertThat(this.tomcatProperties.getUseRelativeRedirects()).isNull();
+		TomcatWebServer server = customizeAndGetServer();
+		Context context = (Context) server.getTomcat().getHost().findChildren()[0];
+		assertThat(context.getUseRelativeRedirects()).isEqualTo(new StandardContext().getUseRelativeRedirects());
+	}
+
+	@Test
+	void useRelativeRedirectsCanBeEnabled() {
 		bind("server.tomcat.use-relative-redirects=true");
-		assertThat(this.tomcatProperties.isUseRelativeRedirects()).isTrue();
+		assertThat(this.tomcatProperties.getUseRelativeRedirects()).isTrue();
 		TomcatWebServer server = customizeAndGetServer();
 		Context context = (Context) server.getTomcat().getHost().findChildren()[0];
 		assertThat(context.getUseRelativeRedirects()).isTrue();
 	}
 
+	@Test
+	void useRelativeRedirectsCanBeDisabled() {
+		bind("server.tomcat.use-relative-redirects=false");
+		assertThat(this.tomcatProperties.getUseRelativeRedirects()).isFalse();
+		TomcatWebServer server = customizeAndGetServer();
+		Context context = (Context) server.getTomcat().getHost().findChildren()[0];
+		assertThat(context.getUseRelativeRedirects()).isFalse();
+	}
+
 	private void bind(String... inlinedProperties) {
 		TestPropertySourceUtils.addInlinedPropertiesToEnvironment(this.environment, inlinedProperties);
 		new Binder(ConfigurationPropertySources.get(this.environment)).bind("server.tomcat",

smoke-test/spring-boot-smoke-test-oauth2-authorization-server/src/test/java/smoketest/oauth2/server/SampleOAuth2AuthorizationServerApplicationTests.java

@@ -112,7 +112,7 @@ void authServerMetadataShouldAllowAccess() {
 	void anonymousShouldRedirectToLogin() {
 		RestTestClient.ResponseSpec response = nonFollowingRedirect().get().uri("/").exchange();
 		response.expectStatus().isFound();
-		response.expectHeader().location("http://localhost:" + this.port + "/login");
+		response.expectHeader().location("/login");
 	}
 
 	@Test
@@ -181,7 +181,7 @@ void anonymousTokenRequestWithAcceptHeaderTextHtmlShouldRedirectToLogin() {
 			.body(body)
 			.exchange();
 		response.expectStatus().isFound();
-		response.expectHeader().location("http://localhost:" + this.port + "/login");
+		response.expectHeader().location("/login");
 	}
 
 }

smoke-test/spring-boot-smoke-test-oauth2-client/src/test/java/smoketest/oauth2/client/SampleOAuth2ClientApplicationTests.java

@@ -53,7 +53,7 @@ private RestTestClient nonFollowingRedirect() {
 	void everythingShouldRedirectToLogin() {
 		RestTestClient.ResponseSpec response = nonFollowingRedirect().get().uri("/").exchange();
 		response.expectStatus().isFound();
-		response.expectHeader().location("http://localhost:" + this.port + "/login");
+		response.expectHeader().location("/login");
 	}
 
 	@Test

smoke-test/spring-boot-smoke-test-saml2-service-provider/src/test/java/smoketest/saml2/serviceprovider/SampleSaml2RelyingPartyApplicationTests.java

@@ -51,7 +51,7 @@ private RestTestClient nonFollowingRedirect() {
 	void everythingShouldRedirectToLogin() {
 		RestTestClient.ResponseSpec response = nonFollowingRedirect().get().uri("/").exchange();
 		response.expectStatus().isFound();
-		response.expectHeader().location("http://localhost:" + this.port + "/login");
+		response.expectHeader().location("/login");
 	}
 
 	@Test

smoke-test/spring-boot-smoke-test-web-groovy-templates/src/test/java/smoketest/groovytemplates/SampleGroovyTemplateApplicationTests.java

@@ -22,7 +22,6 @@
 import org.springframework.boot.resttestclient.autoconfigure.AutoConfigureRestTestClient;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
-import org.springframework.boot.test.web.server.LocalServerPort;
 import org.springframework.test.web.servlet.client.RestTestClient;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
@@ -38,9 +37,6 @@
 @AutoConfigureRestTestClient
 class SampleGroovyTemplateApplicationTests {
 
-	@LocalServerPort
-	private int port;
-
 	@Autowired
 	private RestTestClient restTestClient;
 
@@ -62,7 +58,7 @@ void testCreate() {
 			.body(map)
 			.exchange()
 			.expectHeader()
-			.value("Location", (location) -> assertThat(location).contains("localhost:" + this.port));
+			.value("Location", (location) -> assertThat(location).matches("/\\d+(;jsessionid=[\\w.]+)?"));
 	}
 
 	@Test

smoke-test/spring-boot-smoke-test-web-method-security/src/test/java/smoketest/security/method/SampleMethodSecurityApplicationTests.java

@@ -91,7 +91,7 @@ void testLogin() {
 		assertThat(result.getStatus()).isEqualTo(HttpStatus.FOUND);
 		URI location = result.getResponseHeaders().getLocation();
 		assertThat(location).isNotNull();
-		assertThat(location.toString()).endsWith(this.port + "/");
+		assertThat(location.toString()).isEqualTo("/");
 	}
 
 	@Test