Fix port handling in HtmlUnitRequestBuilder

Prior to this commit, HtmlUnitRequestBuilder set the server port in the
MockHttpServletRequest to -1 if the URL did not contain an explicit
port. However, that can lead to errors in consumers of the request that
do not expect an invalid port number.

In addition, HtmlUnitRequestBuilder always set the remote port in the
MockHttpServletRequest to the value of the server port, which does not
make sense, since the remote port of the client has nothing to do with
the port on the server.

To address those issues, this commit revises HtmlUnitRequestBuilder so
that it:

- Does not set the server and local ports if the explicit or derived
  default value is -1.
- Consistently sets the server and local ports to the same valid value.
- Does not set the remote port.

Closes gh-35709

spring-test/src/main/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilder.java

@@ -211,14 +211,12 @@ final class HtmlUnitRequestBuilder implements RequestBuilder, Mergeable {
 
 	private void ports(UriComponents uriComponents, MockHttpServletRequest request) {
 		int serverPort = uriComponents.getPort();
-		request.setServerPort(serverPort);
 		if (serverPort == -1) {
-			int portConnection = this.webRequest.getUrl().getDefaultPort();
-			request.setLocalPort(serverPort);
-			request.setRemotePort(portConnection);
+			serverPort = this.webRequest.getUrl().getDefaultPort();
 		}
-		else {
-			request.setRemotePort(serverPort);
+		if (serverPort != -1) {
+			request.setServerPort(serverPort);
+			request.setLocalPort(serverPort);
 		}
 	}
 

spring-test/src/test/java/org/springframework/test/web/servlet/htmlunit/HtmlUnitRequestBuilderTests.java

@@ -26,6 +26,7 @@ import java.util.Map;
 
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.http.Cookie;
+import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpSession;
 import org.apache.commons.io.IOUtils;
 import org.apache.http.auth.UsernamePasswordCredentials;
@@ -339,22 +340,6 @@ public class HtmlUnitRequestBuilderTests {
 		assertThat(actualRequest.getLocalName()).isEqualTo("localhost");
 	}
 
-	@Test
-	void buildRequestLocalPortMatchingDefault() throws Exception {
-		webRequest.setUrl(new URL("http://localhost:80/test/this/here"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getLocalPort()).isEqualTo(-1);
-	}
-
-	@Test
-	void buildRequestLocalMissing() throws Exception {
-		webRequest.setUrl(new URL("http://localhost/test/this"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getLocalPort()).isEqualTo(-1);
-	}
-
 	@Test
 	void buildRequestMethods() {
 		for (HttpMethod expectedMethod : HttpMethod.values()) {
@@ -534,42 +519,11 @@ public class HtmlUnitRequestBuilderTests {
 	}
 
 	@Test
-	void buildRequestRemoteAddr() {
+	void buildRequestRemoteAddressHostAndPort() {
 		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
 
 		assertThat(actualRequest.getRemoteAddr()).isEqualTo("127.0.0.1");
-	}
-
-	@Test
-	void buildRequestRemoteHost() {
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getRemoteAddr()).isEqualTo("127.0.0.1");
-	}
-
-	@Test
-	void buildRequestRemotePort() throws Exception {
-		webRequest.setUrl(new URL("http://localhost:80/test/this/here"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getRemotePort()).isEqualTo(80);
-	}
-
-	@Test
-	void buildRequestRemotePort8080() throws Exception {
-		webRequest.setUrl(new URL("https://example.com:8080/"));
-
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getRemotePort()).isEqualTo(8080);
-	}
-
-	@Test
-	void buildRequestRemotePort80WithDefault() throws Exception {
-		webRequest.setUrl(new URL("http://company.example/"));
-
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
+		assertThat(actualRequest.getRemoteHost()).isEqualTo("localhost");
 		assertThat(actualRequest.getRemotePort()).isEqualTo(80);
 	}
 
@@ -596,9 +550,63 @@ public class HtmlUnitRequestBuilderTests {
 	}
 
 	@Test
-	void buildRequestUrl() {
-		String uri = requestBuilder.buildRequest(servletContext).getRequestURL().toString();
-		assertThat(uri).isEqualTo("https://example.com/test/this/here");
+	void buildRequestWithSchemeHttpAndDefaultPort() throws Exception {
+		webRequest.setUrl(new URL("http://localhost/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "http://localhost/test", 80, false);
+	}
+
+	@Test
+	void buildRequestWithSchemeHttpAndExplicitDefaultPort() throws Exception {
+		webRequest.setUrl(new URL("http://localhost:80/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "http://localhost/test", 80, false);
+	}
+
+	@Test
+	void buildRequestWithSchemeHttpAndExplicitPort() throws Exception {
+		webRequest.setUrl(new URL("http://localhost:8081/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "http://localhost:8081/test", 8081, false);
+
+		// Unlikely scheme/port combination:
+		webRequest.setUrl(new URL("http://localhost:443/test"));
+		request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "http://localhost:443/test", 443, false);
+	}
+
+	@Test
+	void buildRequestWithSchemeHttpsAndDefaultPort() throws Exception {
+		webRequest.setUrl(new URL("https://localhost/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "https://localhost/test", 443, true);
+	}
+
+	@Test
+	void buildRequestWithSchemeHttpsAndExplicitDefaultPort() throws Exception {
+		webRequest.setUrl(new URL("https://localhost:443/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "https://localhost/test", 443, true);
+	}
+
+	@Test
+	void buildRequestWithSchemeHttpsAndExplicitPort() throws Exception {
+		webRequest.setUrl(new URL("https://localhost:8443/test"));
+		var request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "https://localhost:8443/test", 8443, true);
+
+		// Unlikely scheme/port combination:
+		webRequest.setUrl(new URL("https://localhost:80/test"));
+		request = requestBuilder.buildRequest(servletContext);
+
+		assertUrlAndPorts(request, "https://localhost:80/test", 80, true);
 	}
 
 	@Test
@@ -624,30 +632,6 @@ public class HtmlUnitRequestBuilderTests {
 		assertThat(actualRequest.getServerName()).isEqualTo("example.com");
 	}
 
-	@Test
-	void buildRequestServerPort() throws Exception {
-		webRequest.setUrl(new URL("http://localhost:8080/test/this/here"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getServerPort()).isEqualTo(8080);
-	}
-
-	@Test
-	void buildRequestServerPortMatchingDefault() throws Exception {
-		webRequest.setUrl(new URL("http://localhost/test/this/here"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getServerPort()).isEqualTo(-1);
-	}
-
-	@Test
-	void buildRequestServerPortDefault() throws Exception {
-		webRequest.setUrl(new URL("https://example.com/"));
-		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
-
-		assertThat(actualRequest.getServerPort()).isEqualTo(-1);
-	}
-
 	@Test
 	void buildRequestServletContext() {
 		MockHttpServletRequest actualRequest = requestBuilder.buildRequest(servletContext);
@@ -910,4 +894,21 @@ public class HtmlUnitRequestBuilderTests {
 		return (String) ReflectionTestUtils.getField(requestBuilder, "contextPath");
 	}
 
+	private static void assertUrlAndPorts(HttpServletRequest request, String url, int serverPort, boolean secure) {
+		assertThat(request.getRequestURL()).asString().as("url").isEqualTo(url);
+		assertThat(request.getServerPort()).as("server port").isEqualTo(serverPort);
+		// In a mocked request, the local post is always the same as the server port.
+		assertThat(request.getLocalPort()).as("local port").isEqualTo(serverPort);
+		// Remote Port is always 80 (MockHttpServletRequest.DEFAULT_SERVER_PORT),
+		// since a mocked request does not influence the remote host, port, or address.
+		assertThat(request.getRemotePort()).as("remote port").isEqualTo(80);
+
+		if (secure) {
+			assertThat(request.isSecure()).as("secure").isTrue();
+		}
+		else {
+			assertThat(request.isSecure()).as("secure").isFalse();
+		}
+	}
+
 }