SessionRepositoryFilter caches null session lookup
If a session cannot be found by id, we will cache that result for any subsequent calls to getSession(false) for the duration of this request. Fixes gh-423
This commit is contained in:
committed by
Rob Winch
parent
4f7728f5b5
commit
831d2f4152
@@ -87,6 +87,11 @@ public class SessionRepositoryFilter<S extends ExpiringSession>
|
||||
public static final String SESSION_REPOSITORY_ATTR = SessionRepository.class
|
||||
.getName();
|
||||
|
||||
/**
|
||||
* Invalid session id (not backed by the session repository) request attribute name.
|
||||
*/
|
||||
public static final String INVALID_SESSION_ID_ATTR = SESSION_REPOSITORY_ATTR + ".invalidSessionId";
|
||||
|
||||
/**
|
||||
* The default filter order.
|
||||
*/
|
||||
@@ -330,7 +335,7 @@ public class SessionRepositoryFilter<S extends ExpiringSession>
|
||||
return currentSession;
|
||||
}
|
||||
String requestedSessionId = getRequestedSessionId();
|
||||
if (requestedSessionId != null) {
|
||||
if (requestedSessionId != null && getAttribute(INVALID_SESSION_ID_ATTR) == null) {
|
||||
S session = getSession(requestedSessionId);
|
||||
if (session != null) {
|
||||
this.requestedSessionIdValid = true;
|
||||
@@ -338,6 +343,12 @@ public class SessionRepositoryFilter<S extends ExpiringSession>
|
||||
currentSession.setNew(false);
|
||||
setCurrentSession(currentSession);
|
||||
return currentSession;
|
||||
} else {
|
||||
// This is an invalid session id. No need to ask again if request.getSession is invoked for the duration of this request
|
||||
if (SESSION_LOGGER.isDebugEnabled()) {
|
||||
SESSION_LOGGER.debug("No session found by id: Caching result for getSession(false) for this HttpServletRequest.");
|
||||
}
|
||||
setAttribute(INVALID_SESSION_ID_ATTR, "true");
|
||||
}
|
||||
}
|
||||
if (!create) {
|
||||
|
||||
@@ -64,6 +64,7 @@ import static org.mockito.Mockito.reset;
|
||||
import static org.mockito.Mockito.spy;
|
||||
import static org.mockito.Mockito.verify;
|
||||
import static org.mockito.Mockito.verifyZeroInteractions;
|
||||
import static org.mockito.internal.verification.VerificationModeFactory.times;
|
||||
|
||||
@RunWith(MockitoJUnitRunner.class)
|
||||
@SuppressWarnings("deprecation")
|
||||
@@ -1341,6 +1342,35 @@ public class SessionRepositoryFilterTests {
|
||||
this.filter.setHttpSessionStrategy((MultiHttpSessionStrategy) null);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void getSessionFalseWithInvalidSessionIdShouldOnlyAskRepositoryOnce() throws ServletException, IOException {
|
||||
this.sessionRepository = spy(this.sessionRepository);
|
||||
this.filter = new SessionRepositoryFilter<ExpiringSession>(this.sessionRepository);
|
||||
|
||||
final String nonExistantSessionId = "nonExistantSessionId";
|
||||
setSessionCookie(nonExistantSessionId);
|
||||
|
||||
doFilter(new DoInFilter() {
|
||||
@Override
|
||||
public void doFilter(HttpServletRequest wrappedRequest) {
|
||||
// Before first invocation
|
||||
assertThat(SessionRepositoryFilterTests.this.request.getAttribute(SessionRepositoryFilter.INVALID_SESSION_ID_ATTR)).isNull();
|
||||
|
||||
// First call should go all the way through to the sessioRepository (it will not find the session)
|
||||
HttpSession session = wrappedRequest.getSession(false);
|
||||
verify(sessionRepository, times(1)).getSession(nonExistantSessionId);
|
||||
assertThat(session).isNull();
|
||||
assertThat(SessionRepositoryFilterTests.this.request.getAttribute(SessionRepositoryFilter.INVALID_SESSION_ID_ATTR)).isNotNull();
|
||||
|
||||
// Second call should not reach the sessionRepository
|
||||
session = wrappedRequest.getSession(false);
|
||||
verify(sessionRepository, times(1)).getSession(nonExistantSessionId); // still only called once
|
||||
assertThat(session).isNull();
|
||||
assertThat(SessionRepositoryFilterTests.this.request.getAttribute(SessionRepositoryFilter.INVALID_SESSION_ID_ATTR)).isNotNull();
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// --- helper methods
|
||||
|
||||
private void assertNewSession() {
|
||||
|
||||
Reference in New Issue
Block a user