diff --git a/spring-session-core/src/main/java/org/springframework/session/web/http/OncePerRequestFilter.java b/spring-session-core/src/main/java/org/springframework/session/web/http/OncePerRequestFilter.java index ff1c147d..409f25d3 100644 --- a/spring-session-core/src/main/java/org/springframework/session/web/http/OncePerRequestFilter.java +++ b/spring-session-core/src/main/java/org/springframework/session/web/http/OncePerRequestFilter.java @@ -68,13 +68,14 @@ abstract class OncePerRequestFilter implements Filter { HttpServletRequest httpRequest = (HttpServletRequest) request; HttpServletResponse httpResponse = (HttpServletResponse) response; String alreadyFilteredAttributeName = this.alreadyFilteredAttributeName; - alreadyFilteredAttributeName = updateForErrorDispatch( - alreadyFilteredAttributeName, request); boolean hasAlreadyFilteredAttribute = request .getAttribute(alreadyFilteredAttributeName) != null; if (hasAlreadyFilteredAttribute) { - + if (DispatcherType.ERROR.equals(request.getDispatcherType())) { + doFilterNestedErrorDispatch(httpRequest, httpResponse, filterChain); + return; + } // Proceed without invoking this filter... filterChain.doFilter(request, response); } @@ -91,15 +92,39 @@ abstract class OncePerRequestFilter implements Filter { } } - private String updateForErrorDispatch(String alreadyFilteredAttributeName, - ServletRequest request) { - // Jetty does ERROR dispatch within sendError, so request attribute is still present - // Use a separate attribute for ERROR dispatches - if (DispatcherType.ERROR.equals(request.getDispatcherType()) - && request.getAttribute(alreadyFilteredAttributeName) != null) { - return alreadyFilteredAttributeName + ".ERROR"; - } - return alreadyFilteredAttributeName; + /** + * Return the name of the request attribute that identifies that a request is already + * filtered. + *

+ * The default implementation takes the configured name of the concrete filter + * instance and appends ".FILTERED". If the filter is not fully initialized, it falls + * back to its class name. + * @return the name of request attribute indicating already filtered request + * @see #ALREADY_FILTERED_SUFFIX + */ + protected String getAlreadyFilteredAttributeName() { + return this.alreadyFilteredAttributeName; + } + + /** + * Typically an ERROR dispatch happens after the REQUEST dispatch completes, and the + * filter chain starts anew. On some servers however the ERROR dispatch may be nested + * within the REQUEST dispatch, e.g. as a result of calling {@code sendError} on the + * response. In that case we are still in the filter chain, on the same thread, but + * the request and response have been switched to the original, unwrapped ones. + *

+ * Sub-classes may use this method to filter such nested ERROR dispatches and re-apply + * wrapping on the request or response. {@code ThreadLocal} context, if any, should + * still be active as we are still nested within the filter chain. + * @param request the request + * @param response the response + * @param filterChain the filter chain + * @throws ServletException if request is not HTTP request + * @throws IOException in case of I/O operation exception + */ + protected void doFilterNestedErrorDispatch(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + doFilter(request, response, filterChain); } /** diff --git a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java index 0c28d006..bcf68b05 100644 --- a/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java +++ b/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java @@ -159,6 +159,12 @@ public class SessionRepositoryFilter extends OncePerRequestFi this.servletContext = servletContext; } + @Override + protected void doFilterNestedErrorDispatch(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) throws ServletException, IOException { + doFilterInternal(request, response, filterChain); + } + /** * Allows ensuring that the session is saved if the response is committed. * diff --git a/spring-session-core/src/test/java/org/springframework/session/web/http/OncePerRequestFilterTests.java b/spring-session-core/src/test/java/org/springframework/session/web/http/OncePerRequestFilterTests.java index 52b200d2..88f6a118 100644 --- a/spring-session-core/src/test/java/org/springframework/session/web/http/OncePerRequestFilterTests.java +++ b/spring-session-core/src/test/java/org/springframework/session/web/http/OncePerRequestFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2019 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import javax.servlet.DispatcherType; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -32,6 +33,7 @@ import org.junit.Test; import org.springframework.mock.web.MockFilterChain; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.web.util.WebUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -95,4 +97,36 @@ public class OncePerRequestFilterTests { assertThat(this.invocations).containsOnly(this.filter, filter2); } + + @Test // gh-1470 + public void filterNestedErrorDispatch() throws ServletException, IOException { + TestOncePerRequestFilter filter = new TestOncePerRequestFilter(); + this.request.setAttribute(filter.getAlreadyFilteredAttributeName(), Boolean.TRUE); + this.request.setDispatcherType(DispatcherType.ERROR); + this.request.setAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE, "/error"); + filter.doFilter(this.request, new MockHttpServletResponse(), this.chain); + assertThat(filter.didFilter).isFalse(); + assertThat(filter.didFilterNestedErrorDispatch).isTrue(); + } + + private static class TestOncePerRequestFilter extends OncePerRequestFilter { + + private boolean didFilter; + + private boolean didFilterNestedErrorDispatch; + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) { + this.didFilter = true; + } + + @Override + protected void doFilterNestedErrorDispatch(HttpServletRequest request, HttpServletResponse response, + FilterChain filterChain) { + this.didFilterNestedErrorDispatch = true; + } + + } + }