Reject invalid forwarded requests in ForwardedHeaderFilter

Prior to this commit, the `ForwardedHeaderFilter` and the forwarded
header utils would throw `IllegalArgumentException` and
`IllegalStateException` when request headers are invalid and cannot be
parsed for Forwarded handling.

This commit aligns the behavior with the WebFlux counterpart by
rejecting such requests with HTTP 400 responses directly.

Fixes gh-31894

spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java

@@ -30,6 +30,8 @@
 import jakarta.servlet.http.HttpServletRequestWrapper;
 import jakarta.servlet.http.HttpServletResponse;
 import jakarta.servlet.http.HttpServletResponseWrapper;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.http.HttpStatus;
 import org.springframework.http.server.ServerHttpRequest;
@@ -64,11 +66,14 @@
  * @author Rossen Stoyanchev
  * @author Eddú Meléndez
  * @author Rob Winch
+ * @author Brian Clozel
  * @since 4.3
  * @see <a href="https://tools.ietf.org/html/rfc7239">https://tools.ietf.org/html/rfc7239</a>
  */
 public class ForwardedHeaderFilter extends OncePerRequestFilter {
 
+	private static final Log logger = LogFactory.getLog(ForwardedHeaderFilter.class);
+
 	private static final Set<String> FORWARDED_HEADER_NAMES =
 			Collections.newSetFromMap(new LinkedCaseInsensitiveMap<>(10, Locale.ENGLISH));
 
@@ -143,17 +148,34 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 			filterChain.doFilter(wrappedRequest, response);
 		}
 		else {
-			HttpServletRequest wrappedRequest =
-					new ForwardedHeaderExtractingRequest(request);
-
-			HttpServletResponse wrappedResponse = this.relativeRedirects ?
-					RelativeRedirectResponseWrapper.wrapIfNecessary(response, HttpStatus.SEE_OTHER) :
-					new ForwardedHeaderExtractingResponse(response, wrappedRequest);
-
+			HttpServletRequest wrappedRequest = null;
+			HttpServletResponse wrappedResponse = null;
+			try {
+				wrappedRequest = new ForwardedHeaderExtractingRequest(request);
+				wrappedResponse = this.relativeRedirects ?
+						RelativeRedirectResponseWrapper.wrapIfNecessary(response, HttpStatus.SEE_OTHER) :
+						new ForwardedHeaderExtractingResponse(response, wrappedRequest);
+			}
+			catch (Throwable ex) {
+				if (logger.isDebugEnabled()) {
+					logger.debug("Failed to apply forwarded headers to " + formatRequest(request), ex);
+				}
+				response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+				return;
+			}
 			filterChain.doFilter(wrappedRequest, wrappedResponse);
 		}
 	}
 
+	/**
+	 * Format the request for logging purposes including HTTP method and URL.
+	 * @param request the request to format
+	 * @return the String to display, never empty or {@code null}
+	 */
+	protected String formatRequest(HttpServletRequest request) {
+		return "HTTP " + request.getMethod() + " \"" + request.getRequestURI() + "\"";
+	}
+
 	@Override
 	protected void doFilterNestedErrorDispatch(HttpServletRequest request, HttpServletResponse response,
 			FilterChain filterChain) throws ServletException, IOException {

spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java

@@ -32,12 +32,12 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import org.springframework.http.HttpStatus;
 import org.springframework.web.testfixture.servlet.MockFilterChain;
 import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
 import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -46,6 +46,7 @@
  * @author Rossen Stoyanchev
  * @author Eddú Meléndez
  * @author Rob Winch
+ * @author Brian Clozel
  */
 public class ForwardedHeaderFilterTests {
 
@@ -208,6 +209,38 @@ public void forwardedRequestWithServletForward() throws Exception {
 		assertThat(actual.getRequestURL().toString()).isEqualTo("https://www.mycompany.example/bar");
 	}
 
+	@Nested // gh-31842
+	class InvalidRequests {
+
+		@Test
+		void shouldRejectInvalidForwardedForIpv4() throws Exception {
+			request.addHeader(FORWARDED, "for=127.0.0.1:");
+
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			filter.doFilter(request, response, filterChain);
+			assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
+		}
+
+		@Test
+		void shouldRejectInvalidForwardedForIpv6() throws Exception {
+			request.addHeader(FORWARDED, "for=\"2a02:918:175:ab60:45ee:c12c:dac1:808b\"");
+
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			filter.doFilter(request, response, filterChain);
+			assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
+		}
+
+		@Test
+		void shouldRejectInvalidForwardedPort() throws Exception {
+			request.addHeader(X_FORWARDED_PORT, "invalid");
+
+			MockHttpServletResponse response = new MockHttpServletResponse();
+			filter.doFilter(request, response, filterChain);
+			assertThat(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value());
+		}
+
+	}
+
 	@Nested
 	class ForwardedPrefix {
 
@@ -474,12 +507,6 @@ public void forwardedForMultipleIdentifiers() throws Exception {
 			assertThat(actual.getRemotePort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
 		}
 
-		@Test  // gh-26748
-		public void forwardedForInvalidIpV6Address() {
-			request.addHeader(FORWARDED, "for=\"2a02:918:175:ab60:45ee:c12c:dac1:808b\"");
-			assertThatIllegalArgumentException().isThrownBy(
-					ForwardedHeaderFilterTests.this::filterAndGetWrappedRequest);
-		}
 	}
 
 	@Nested