From 3d93f2cf56ad9b9562ceeb3ffda75715b113de4f Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Mon, 27 Jul 2015 16:52:33 -0500 Subject: [PATCH] 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 --- .../web/http/SessionRepositoryFilter.java | 29 +++++++++++++++---- .../http/SessionRepositoryFilterTests.java | 27 ++++++++++++++++- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java b/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java index 88700c89..72039a11 100644 --- a/spring-session/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java +++ b/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 extends OncePerRequestFilter { public static final String SESSION_REPOSITORY_ATTR = SessionRepository.class.getName(); @@ -161,7 +162,7 @@ public class SessionRepositoryFilter 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 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 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 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 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 extends OncePerR this.requestedSessionIdValid = true; currentSession = new HttpSessionWrapper(session, getServletContext()); currentSession.setNew(false); + setCurrentSession(currentSession); return currentSession; } } @@ -266,6 +282,7 @@ public class SessionRepositoryFilter extends OncePerR } S session = sessionRepository.createSession(); currentSession = new HttpSessionWrapper(session, getServletContext()); + setCurrentSession(currentSession); return currentSession; } @@ -377,7 +394,7 @@ public class SessionRepositoryFilter extends OncePerR checkState(); this.invalidated = true; requestedSessionInvalidated = true; - currentSession = null; + setCurrentSession(null); sessionRepository.delete(getId()); } diff --git a/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java b/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java index aab9a019..b6ad17c4 100644 --- a/spring-session/src/test/java/org/springframework/session/web/http/SessionRepositoryFilterTests.java +++ b/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 sessions; + private SessionRepository sessionRepository; private SessionRepositoryFilter filter; @@ -79,7 +81,8 @@ public class SessionRepositoryFilterTests { @Before public void setup() throws Exception { - sessionRepository = new MapSessionRepository(); + sessions = new HashMap(); + sessionRepository = new MapSessionRepository(sessions); filter = new SessionRepositoryFilter(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);