Filtering for nested ERROR dispatch

Resolves: #1470
See: spring-projects/spring-framework#23196

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.
+	 * <p>
+	 * 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.
+	 * <p>
+	 * 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);
 	}
 
 	/**

spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java

@@ -159,6 +159,12 @@ public class SessionRepositoryFilter<S extends Session> 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.
 	 *

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;
+		}
+
+	}
+
 }