`HtmlUnitRequestBuilder` Creates `ServletRequet.getServerPort() = -1`

[rwinch]

Summary

I'm getting an error in Spring Security when using HtmlUnitRequestBuilder to request a URL that does not contain an explicit port (e.g. http://localhost/path) because HtmlUnitRequestBuilder will set the ServeltRequest.serverPort to -1 and Spring Security creates a URL with -1 as the port.

Please update Spring Framework to no longer set the ServeltRequest.serverPort to -1.

Details

The problem is that HtmlUnitRequestBuilder is setting the ServletRequest.serverPort property to -1 and Spring Security's UrlUtils.buildFullRequestUrl adds the serverPort as -1.

One could possibly argue that UrlUtils should handle ServletRequest.getServerPort() returning -1. After all, URL.getPort() returns -1 if the port is not set. However, I do not think that -1 is a valid value for ServletRequest.getServerPort() because:

1. The ServletRequest.getServerPort() Javadoc is defined as the value after : in the Host header value, if any, or the server port where the client connection was accepted on. It does not state anything about returning -1.
2. When running a Tomcat server locally on port 80 and requesting http://localhost/ then ServletRequest.getServerPort() returns 80 and not -1.
3. UrlUtils has been around since Acegi Security 1.0.0 (2006) and never handled the serverPort = -1 and as far as I know we have not received any reports from users.

Why Now?

You might wonder why this issue was just now discovered. The reason is that Spring Boot now autowires a UriBuilderFactoryWebClient which does not provide a port in its URLs. Previously, Spring Boot used an instance of LocalHostWebClient which always provided a port when using a URL starting with /.

Related spring-projects/spring-boot#47857

HtmlUnitRequestBuilder and remotePort and localPort

I'm wondering if these need to be corrected as well.

ServeltRequest.serverPort in other parts of Spring Framework

It is probably worth mentioning that while this is the specific issue causing Spring Security samples tests to fail. However, it seems the assumption that serverPort can be -1 exists in other places within Spring. For example, CorsUtils checks to see if the serverPort == -1. Perhaps this is an artifact of WebUtils.isSameOrigin(HttpRequest) needing to support checking the origin from HttpRequest.getURI() where the port could be -1?

Interestingly, ForwardedHeaderExtractingRequest prevents this.port, which is used for ServeltRequet.getServerPort(), from being set to -1.

Perhaps these additional areas are not problematic since it is being defensive about reading ServeltRequest.serverPort but not setting it to -1. More concretely, I don't think it is as problematic to be defensive when reading ServeltRequest.serverPort, but please do not set the value to -1.