currentSession saved on HttpServletRequest attribute

Previously, if the following happened:

* New Session Created
* Exception thrown
* Exception processed by error handler within Servlet
* Error Handler used a session

The result would be two sessions were created. This means the
data from the first session was also lost. This happend
because ERROR dispatch is a separate Filter invocation where
the request is no longer wrapped.

This commit ensures that currentSession is saved on a
HttpServletRequest attribute so that the ERROR dispatch sees
that a session was already created.

Fixes: gh-229

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

@@ -58,6 +58,7 @@ import org.springframework.session.SessionRepository;
  * @since 1.0
  * @author Rob Winch
  */
+@SuppressWarnings("deprecation")
 @Order(SessionRepositoryFilter.DEFAULT_ORDER)
 public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerRequestFilter {
 	public static final String SESSION_REPOSITORY_ATTR = SessionRepository.class.getName();
@@ -161,7 +162,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 	 * @since 1.0
 	 */
 	private final class SessionRepositoryRequestWrapper extends HttpServletRequestWrapper {
-		private HttpSessionWrapper currentSession;
+		private final String CURRENT_SESSION_ATTR = HttpServletRequestWrapper.class.getName();
 		private Boolean requestedSessionIdValid;
 		private boolean requestedSessionInvalidated;
 		private final HttpServletResponse response;
@@ -177,7 +178,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 		 * Uses the HttpSessionStrategy to write the session id tot he response and persist the Session.
 		 */
 		private void commitSession() {
-			HttpSessionWrapper wrappedSession = currentSession;
+			HttpSessionWrapper wrappedSession = getCurrentSession();
 			if(wrappedSession == null) {
 				if(isInvalidateClientSession()) {
 					httpSessionStrategy.onInvalidateSession(this, response);
@@ -191,6 +192,19 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 			}
 		}
 
+		@SuppressWarnings("unchecked")
+		private HttpSessionWrapper getCurrentSession() {
+			return (HttpSessionWrapper) getAttribute(CURRENT_SESSION_ATTR);
+		}
+
+		private void setCurrentSession(HttpSessionWrapper currentSession) {
+			if(currentSession == null) {
+				removeAttribute(CURRENT_SESSION_ATTR);
+			} else {
+				setAttribute(CURRENT_SESSION_ATTR, currentSession);
+			}
+		}
+
 		@SuppressWarnings({ "unused", "unchecked" })
 		public String changeSessionId() {
 			HttpSession session = getSession(false);
@@ -210,8 +224,8 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 			}
 
 			sessionRepository.delete(session.getId());
-			HttpSessionWrapper original = currentSession;
-			currentSession = null;
+			HttpSessionWrapper original = getCurrentSession();
+			setCurrentSession(null);
 
 			HttpSession newSession = getSession();
 			original.session = ((HttpSessionWrapper)newSession).session;
@@ -243,11 +257,12 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 		}
 
 		private boolean isInvalidateClientSession() {
-			return currentSession == null && requestedSessionInvalidated;
+			return getCurrentSession() == null && requestedSessionInvalidated;
 		}
 
 		@Override
 		public HttpSession getSession(boolean create) {
+			HttpSessionWrapper currentSession = getCurrentSession();
 			if(currentSession != null) {
 				return currentSession;
 			}
@@ -258,6 +273,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 					this.requestedSessionIdValid = true;
 					currentSession = new HttpSessionWrapper(session, getServletContext());
 					currentSession.setNew(false);
+					setCurrentSession(currentSession);
 					return currentSession;
 				}
 			}
@@ -266,6 +282,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 			}
 			S session = sessionRepository.createSession();
 			currentSession = new HttpSessionWrapper(session, getServletContext());
+			setCurrentSession(currentSession);
 			return currentSession;
 		}
 
@@ -377,7 +394,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession> extends OncePerR
 				checkState();
 				this.invalidated = true;
 				requestedSessionInvalidated = true;
-				currentSession = null;
+				setCurrentSession(null);
 				sessionRepository.delete(getId());
 			}
 

spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java

@@ -67,6 +67,8 @@ public class SessionRepositoryFilterTests {
 	@Mock
 	private HttpSessionStrategy strategy;
 
+	private Map<String, ExpiringSession> sessions;
+
 	private SessionRepository<ExpiringSession> sessionRepository;
 
 	private SessionRepositoryFilter<ExpiringSession> filter;
@@ -79,7 +81,8 @@ public class SessionRepositoryFilterTests {
 
 	@Before
 	public void setup() throws Exception {
-		sessionRepository = new MapSessionRepository();
+		sessions = new HashMap<String, ExpiringSession>();
+		sessionRepository = new MapSessionRepository(sessions);
 		filter = new SessionRepositoryFilter<ExpiringSession>(sessionRepository);
 		setupRequest();
 	}
@@ -558,6 +561,28 @@ public class SessionRepositoryFilterTests {
 		});
 	}
 
+	// gh-229
+	@Test
+	public void doFilterGetSessionGetSessionOnError() throws Exception {
+		doFilter(new DoInFilter() {
+			@Override
+			public void doFilter(HttpServletRequest wrappedRequest) {
+				wrappedRequest.getSession().setAttribute("a", "b");
+			}
+		});
+
+		// reuse the same request similar to processing an ERROR dispatch
+
+		doFilter(new DoInFilter() {
+			@Override
+			public void doFilter(HttpServletRequest wrappedRequest) {
+				assertThat(wrappedRequest.getSession(false)).isNotNull();
+			}
+		});
+
+		assertThat(this.sessions.size()).isEqualTo(1);
+	}
+
 	@Test
 	public void doFilterCookieSecuritySettings() throws Exception {
 		request.setSecure(true);