Support selective filtering of error dispatches

OncePerRequestFilter now allows sub-classes to choose whether they
should ever get involved in processing an error dispatch.

Issue: SPR-9895

spring-orm/src/main/java/org/springframework/orm/hibernate3/support/OpenSessionInViewFilter.java

@@ -178,6 +178,15 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 		return false;
 	}
 
+	/**
+	 * Returns "false" so that the filter may provide a Hibernate
+	 * {@code Session} to each error dispatches.
+	 */
+	@Override
+	protected boolean shouldNotFilterErrorDispatch() {
+		return false;
+	}
+
 	@Override
 	protected void doFilterInternal(
 			HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
@@ -187,7 +196,6 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 		boolean participate = false;
 
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean isFirstRequest = !asyncManager.hasConcurrentResult();
 		String key = getAlreadyFilteredAttributeName();
 
 		if (isSingleSession()) {
@@ -197,6 +205,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 				participate = true;
 			}
 			else {
+				boolean isFirstRequest = !isAsyncDispatch(request);
 				if (isFirstRequest || !applySessionBindingInterceptor(asyncManager, key)) {
 					logger.debug("Opening single Hibernate Session in OpenSessionInViewFilter");
 					Session session = getSession(sessionFactory);
@@ -210,8 +219,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 		}
 		else {
 			// deferred close mode
-			Assert.state(!asyncManager.isConcurrentHandlingStarted(),
-					"Deferred close mode is not supported on async dispatches");
+			Assert.state(!isAsyncStarted(request), "Deferred close mode is not supported on async dispatches");
 			if (SessionFactoryUtils.isDeferredCloseActive(sessionFactory)) {
 				// Do not modify deferred close: just set the participate flag.
 				participate = true;
@@ -230,7 +238,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 					// single session mode
 					SessionHolder sessionHolder =
 							(SessionHolder) TransactionSynchronizationManager.unbindResource(sessionFactory);
-					if (!asyncManager.isConcurrentHandlingStarted()) {
+					if (!isAsyncStarted(request)) {
 						logger.debug("Closing single Hibernate Session in OpenSessionInViewFilter");
 						closeSession(sessionHolder.getSession(), sessionFactory);
 					}

spring-orm/src/main/java/org/springframework/orm/hibernate4/support/OpenSessionInViewFilter.java

@@ -102,7 +102,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 	}
 
 	/**
-	 * The default value is "false" so that the filter may re-bind the opened
+	 * Returns "false" so that the filter may re-bind the opened Hibernate
 	 * {@code Session} to each asynchronously dispatched thread and postpone
 	 * closing it until the very last asynchronous dispatch.
 	 */
@@ -111,6 +111,15 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 		return false;
 	}
 
+	/**
+	 * Returns "false" so that the filter may provide a Hibernate
+	 * {@code Session} to each error dispatches.
+	 */
+	@Override
+	protected boolean shouldNotFilterErrorDispatch() {
+		return false;
+	}
+
 	@Override
 	protected void doFilterInternal(
 			HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
@@ -120,7 +129,6 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 		boolean participate = false;
 
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean isFirstRequest = !asyncManager.hasConcurrentResult();
 		String key = getAlreadyFilteredAttributeName();
 
 		if (TransactionSynchronizationManager.hasResource(sessionFactory)) {
@@ -128,6 +136,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 			participate = true;
 		}
 		else {
+			boolean isFirstRequest = !isAsyncDispatch(request);
 			if (isFirstRequest || !applySessionBindingInterceptor(asyncManager, key)) {
 				logger.debug("Opening Hibernate Session in OpenSessionInViewFilter");
 				Session session = openSession(sessionFactory);
@@ -147,7 +156,7 @@ public class OpenSessionInViewFilter extends OncePerRequestFilter {
 			if (!participate) {
 				SessionHolder sessionHolder =
 						(SessionHolder) TransactionSynchronizationManager.unbindResource(sessionFactory);
-				if (!asyncManager.isConcurrentHandlingStarted()) {
+				if (!isAsyncStarted(request)) {
 					logger.debug("Closing Hibernate Session in OpenSessionInViewFilter");
 					SessionFactoryUtils.closeSession(sessionHolder.getSession());
 				}

spring-orm/src/main/java/org/springframework/orm/jpa/support/OpenEntityManagerInViewFilter.java

@@ -126,7 +126,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter {
 	}
 
 	/**
-	 * The default value is "false" so that the filter may re-bind the opened
+	 * Returns "false" so that the filter may re-bind the opened
 	 * {@code EntityManager} to each asynchronously dispatched thread and postpone
 	 * closing it until the very last asynchronous dispatch.
 	 */
@@ -135,6 +135,15 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter {
 		return false;
 	}
 
+	/**
+	 * Returns "false" so that the filter may provide an {@code EntityManager}
+	 * to each error dispatches.
+	 */
+	@Override
+	protected boolean shouldNotFilterErrorDispatch() {
+		return false;
+	}
+
 	@Override
 	protected void doFilterInternal(
 			HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
@@ -144,7 +153,6 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter {
 		boolean participate = false;
 
 		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean isFirstRequest = !asyncManager.hasConcurrentResult();
 		String key = getAlreadyFilteredAttributeName();
 
 		if (TransactionSynchronizationManager.hasResource(emf)) {
@@ -152,6 +160,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter {
 			participate = true;
 		}
 		else {
+			boolean isFirstRequest = !isAsyncDispatch(request);
 			if (isFirstRequest || !applyEntityManagerBindingInterceptor(asyncManager, key)) {
 				logger.debug("Opening JPA EntityManager in OpenEntityManagerInViewFilter");
 				try {
@@ -175,7 +184,7 @@ public class OpenEntityManagerInViewFilter extends OncePerRequestFilter {
 			if (!participate) {
 				EntityManagerHolder emHolder = (EntityManagerHolder)
 						TransactionSynchronizationManager.unbindResource(emf);
-				if (!asyncManager.isConcurrentHandlingStarted()) {
+				if (!isAsyncStarted(request)) {
 					logger.debug("Closing JPA EntityManager in OpenEntityManagerInViewFilter");
 					EntityManagerFactoryUtils.closeEntityManager(emHolder.getEntityManager());
 				}

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

@@ -32,8 +32,6 @@ import javax.servlet.http.HttpSession;
 
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
-import org.springframework.web.context.request.async.WebAsyncManager;
-import org.springframework.web.context.request.async.WebAsyncUtils;
 import org.springframework.web.util.WebUtils;
 
 /**
@@ -200,8 +198,7 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter
 	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
 			throws ServletException, IOException {
 
-		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean isFirstRequest = !asyncManager.hasConcurrentResult();
+		boolean isFirstRequest = !isAsyncDispatch(request);
 
 		if (isIncludePayload()) {
 			if (isFirstRequest) {
@@ -216,7 +213,7 @@ public abstract class AbstractRequestLoggingFilter extends OncePerRequestFilter
 			filterChain.doFilter(request, response);
 		}
 		finally {
-			if (!asyncManager.isConcurrentHandlingStarted()) {
+			if (!isAsyncStarted(request)) {
 				afterRequest(request, getAfterMessage(request));
 			}
 		}

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

@@ -27,21 +27,39 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.web.context.request.async.WebAsyncManager;
 import org.springframework.web.context.request.async.WebAsyncUtils;
+import org.springframework.web.util.WebUtils;
 
 /**
- * Filter base class that guarantees to be just executed once per request,
- * on any servlet container. It provides a {@link #doFilterInternal}
+ * Filter base class that aims to guarantee a single execution per request
+ * dispatch, on any servlet container. It provides a {@link #doFilterInternal}
  * method with HttpServletRequest and HttpServletResponse arguments.
  *
- * <p>In an async scenario a filter may be invoked again in additional threads
- * as part of an {@linkplain javax.servlet.DispatcherType.ASYNC ASYNC} dispatch.
- * Sub-classes may decide whether to be invoked once per request or once per
- * request thread for as long as the same request is being processed.
- * See {@link #shouldNotFilterAsyncDispatch()}.
+ * <p>As of Servlet 3.0, a filter may be invoked as part of a
+ * {@link javax.servlet.DispatcherType.REQUEST REQUEST} or
+ * {@link javax.servlet.DispatcherType.ASYNC ASYNC} dispatches that occur in
+ * separate threads. A filter can be configured in {@code web.xml} whether it
+ * should be involved in async dispatches. However, in some cases servlet
+ * containers assume different default configuration. Therefore sub-classes can
+ * override the method {@link #shouldNotFilterAsyncDispatch()} to declare
+ * statically if they shouuld indeed be invoked, <em>once</em>, during both types
+ * of dispatches in order to provide thread initialization, logging, security,
+ * and so on. This mechanism complements and does not replace the need to
+ * configure a filter in {@code web.xml} with dispatcher types.
  *
- * <p>The {@link #getAlreadyFilteredAttributeName} method determines how
- * to identify that a request is already filtered. The default implementation
- * is based on the configured name of the concrete filter instance.
+ * <p>Sub-classes may use {@link #isAsyncDispatch(HttpServletRequest)} to
+ * determine when a filter is invoked as part of an async dispatch, and
+ * use {@link #isAsyncStarted(HttpServletRequest)} to determine when the
+ * request has been placed in async mode and therefore the current dispatch
+ * won't be the last one.
+ *
+ * <p>Yet another dispatch type that also occurs in its own thread is
+ * {@link javax.servlet.DispatcherType.ERROR ERROR}. Sub-classes can override
+ * {@link #shouldNotFilterErrorDispatch()} if they wish to declare statically
+ * if they should be invoked <em>once</em> during error dispatches.
+ *
+ * <p>The {@link #getAlreadyFilteredAttributeName} method determines how to
+ * identify that a request is already filtered. The default implementation is
+ * based on the configured name of the concrete filter instance.
  *
  * @author Juergen Hoeller
  * @author Rossen Stoyanchev
@@ -74,13 +92,10 @@ public abstract class OncePerRequestFilter extends GenericFilterBean {
 		HttpServletRequest httpRequest = (HttpServletRequest) request;
 		HttpServletResponse httpResponse = (HttpServletResponse) response;
 
-		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean processAsyncDispatch = asyncManager.hasConcurrentResult() && !shouldNotFilterAsyncDispatch();
-
 		String alreadyFilteredAttributeName = getAlreadyFilteredAttributeName();
 		boolean hasAlreadyFilteredAttribute = request.getAttribute(alreadyFilteredAttributeName) != null;
 
-		if ((hasAlreadyFilteredAttribute && (!processAsyncDispatch)) || shouldNotFilter(httpRequest)) {
+		if (hasAlreadyFilteredAttribute || skipDispatch(httpRequest) || shouldNotFilter(httpRequest)) {
 
 			// Proceed without invoking this filter...
 			filterChain.doFilter(request, response);
@@ -92,14 +107,46 @@ public abstract class OncePerRequestFilter extends GenericFilterBean {
 				doFilterInternal(httpRequest, httpResponse, filterChain);
 			}
 			finally {
-				if (!asyncManager.isConcurrentHandlingStarted()) {
-					// Remove the "already filtered" request attribute for this request.
-					request.removeAttribute(alreadyFilteredAttributeName);
-				}
+				// Remove the "already filtered" request attribute for this request.
+				request.removeAttribute(alreadyFilteredAttributeName);
 			}
 		}
 	}
 
+	private boolean skipDispatch(HttpServletRequest request) {
+		if (isAsyncDispatch(request) && shouldNotFilterAsyncDispatch()) {
+			return true;
+		}
+		if ((request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE) != null) && shouldNotFilterErrorDispatch()) {
+			return true;
+		}
+		return false;
+	}
+
+	/**
+	 * The dispatcher type {@code javax.servlet.DispatcherType.ASYNC} introduced
+	 * in Servlet 3.0 means a filter can be invoked in more than one thread over
+	 * the course of a single request. This method returns {@code true} if the
+	 * filter is currently executing within an asynchronous dispatch.
+	 *
+	 * @param request the current request
+	 * @see WebAsyncManager#hasConcurrentResult()
+	 */
+	protected boolean isAsyncDispatch(HttpServletRequest request) {
+		return WebAsyncUtils.getAsyncManager(request).hasConcurrentResult();
+	}
+
+	/**
+	 * Whether request processing is in asynchronous mode meaning that the
+	 * response will not be committed after the current thread is exited.
+	 *
+	 * @param request the current request
+	 * @see WebAsyncManager#isConcurrentHandlingStarted()
+	 */
+	protected boolean isAsyncStarted(HttpServletRequest request) {
+		return WebAsyncUtils.getAsyncManager(request).isConcurrentHandlingStarted();
+	}
+
 	/**
 	 * Return the name of the request attribute that identifies that a request
 	 * is already filtered.
@@ -130,18 +177,18 @@ public abstract class OncePerRequestFilter extends GenericFilterBean {
 	}
 
 	/**
-	 * Whether to filter async dispatches, which occur in a different thread.
 	 * The dispatcher type {@code javax.servlet.DispatcherType.ASYNC} introduced
-	 * in Servlet 3.0 means a filter can be invoked in more than one thread (and
-	 * exited) over the course of a single request. Some filters only need to
-	 * filter the initial thread (e.g. request wrapping) while others may need
+	 * in Servlet 3.0 means a filter can be invoked in more than one thread
+	 * over the course of a single request. Some filters only need to filter
+	 * the initial thread (e.g. request wrapping) while others may need
 	 * to be invoked at least once in each additional thread for example for
 	 * setting up thread locals or to perform final processing at the very end.
 	 * <p>Note that although a filter can be mapped to handle specific dispatcher
 	 * types via {@code web.xml} or in Java through the {@code ServletContext},
 	 * servlet containers may enforce different defaults with regards to
 	 * dispatcher types. This flag enforces the design intent of the filter.
-	 * <p>The default setting is "true", which means the filter will not be
+	 *
+	 * <p>The default return value is "true", which means the filter will not be
 	 * invoked during subsequent async dispatches. If "false", the filter will
 	 * be invoked during async dispatches with the same guarantees of being
 	 * invoked only once during a request within a single thread.
@@ -150,6 +197,16 @@ public abstract class OncePerRequestFilter extends GenericFilterBean {
 		return true;
 	}
 
+	/**
+	 * Whether to filter error dispatches such as when the servlet container
+	 * processes and error mapped in {@code web.xml}. The default return value
+	 * is "true", which means the filter will not be invoked in case of an error
+	 * dispatch.
+	 */
+	protected boolean shouldNotFilterErrorDispatch() {
+		return true;
+	}
+
 	/**
 	 * Same contract as for <code>doFilter</code>, but guaranteed to be
 	 * just invoked once per request within a single request thread.

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

@@ -69,16 +69,24 @@ public class RequestContextFilter extends OncePerRequestFilter {
 		this.threadContextInheritable = threadContextInheritable;
 	}
 
-
 	/**
-	 * The default value is "false" in which case the filter will set up the request
-	 * context in each asynchronously dispatched thread.
+	 * Returns "false" so that the filter may set up the request context in each
+	 * asynchronously dispatched thread.
 	 */
 	@Override
 	protected boolean shouldNotFilterAsyncDispatch() {
 		return false;
 	}
 
+	/**
+	 * Returns "false" so that the filter may set up the request context in an
+	 * error dispatch.
+	 */
+	@Override
+	protected boolean shouldNotFilterErrorDispatch() {
+		return false;
+	}
+
 	@Override
 	protected void doFilterInternal(
 			HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)

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

@@ -68,16 +68,13 @@ public class ShallowEtagHeaderFilter extends OncePerRequestFilter {
 	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
 			throws ServletException, IOException {
 
-		WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request);
-		boolean isFirstRequest = !asyncManager.hasConcurrentResult();
-
-		if (isFirstRequest) {
+		if (!isAsyncDispatch(request)) {
 			response = new ShallowEtagResponseWrapper(response);
 		}
 
 		filterChain.doFilter(request, response);
 
-		if (!asyncManager.isConcurrentHandlingStarted()) {
+		if (!isAsyncStarted(request)) {
 			updateResponse(request, response);
 		}
 	}

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

@@ -34,6 +34,7 @@ import org.springframework.mock.web.MockFilterConfig;
 import org.springframework.mock.web.MockHttpServletResponse;
 import org.springframework.mock.web.MockServletContext;
 import org.springframework.web.context.request.async.WebAsyncUtils;
+import org.springframework.web.util.WebUtils;
 
 /**
  * @author Rick Evans
@@ -50,6 +51,7 @@ public class CharacterEncodingFilterTests extends TestCase {
 		HttpServletRequest request = createMock(HttpServletRequest.class);
 		addAsyncManagerExpectations(request);
 		request.setCharacterEncoding(ENCODING);
+		expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null);
 		expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null);
 		request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE);
 		request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);
@@ -79,6 +81,7 @@ public class CharacterEncodingFilterTests extends TestCase {
 		addAsyncManagerExpectations(request);
 		expect(request.getCharacterEncoding()).andReturn(null);
 		request.setCharacterEncoding(ENCODING);
+		expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null);
 		expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null);
 		request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE);
 		request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);
@@ -104,6 +107,7 @@ public class CharacterEncodingFilterTests extends TestCase {
 		HttpServletRequest request = createMock(HttpServletRequest.class);
 		addAsyncManagerExpectations(request);
 		expect(request.getCharacterEncoding()).andReturn(ENCODING);
+		expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null);
 		expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null);
 		request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE);
 		request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);
@@ -129,6 +133,7 @@ public class CharacterEncodingFilterTests extends TestCase {
 		addAsyncManagerExpectations(request);
 		expect(request.getCharacterEncoding()).andReturn(null);
 		request.setCharacterEncoding(ENCODING);
+		expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null);
 		expect(request.getAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null);
 		request.setAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE);
 		request.removeAttribute(FILTER_NAME + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);
@@ -155,6 +160,7 @@ public class CharacterEncodingFilterTests extends TestCase {
 		addAsyncManagerExpectations(request);
 		expect(request.getCharacterEncoding()).andReturn(null);
 		request.setCharacterEncoding(ENCODING);
+		expect(request.getAttribute(WebUtils.ERROR_REQUEST_URI_ATTRIBUTE)).andReturn(null);
 		expect(request.getAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX)).andReturn(null);
 		request.setAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX, Boolean.TRUE);
 		request.removeAttribute(CharacterEncodingFilter.class.getName() + OncePerRequestFilter.ALREADY_FILTERED_SUFFIX);