Skip to content

Commit

Permalink
Reject invalid forwarded requests in ForwardedHeaderFilter
Browse files Browse the repository at this point in the history
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-31842
  • Loading branch information
bclozel committed Dec 22, 2023
1 parent 8bd8c4f commit 4afac17
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,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.HttpHeaders;
import org.springframework.http.HttpStatus;
Expand Down Expand Up @@ -70,12 +72,15 @@
* @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>
* @see <a href="https://docs.spring.io/spring-framework/reference/web/webmvc/filters.html#filters-forwarded-headers">Forwarded Headers</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));

Expand Down Expand Up @@ -150,17 +155,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 {
Expand Down

0 comments on commit 4afac17

Please sign in to comment.