From 5ea8232f84c251b562eddc158a3d954a2598f680 Mon Sep 17 00:00:00 2001 From: Vishal Puri Date: Mon, 23 Jul 2007 07:58:31 +0000 Subject: [PATCH] SEC-484: fixed concurrency issue --- .../concurrent/SessionRegistryImpl.java | 169 +++++++------- .../concurrent/SessionRegistryImplTests.java | 212 ++++++++++-------- 2 files changed, 203 insertions(+), 178 deletions(-) diff --git a/core/src/main/java/org/acegisecurity/concurrent/SessionRegistryImpl.java b/core/src/main/java/org/acegisecurity/concurrent/SessionRegistryImpl.java index 6e2a69f0bf..d495905db4 100644 --- a/core/src/main/java/org/acegisecurity/concurrent/SessionRegistryImpl.java +++ b/core/src/main/java/org/acegisecurity/concurrent/SessionRegistryImpl.java @@ -34,7 +34,6 @@ import java.util.Set; import javax.servlet.http.HttpSession; - /** * Base implementation of {@link org.acegisecurity.concurrent.SessionRegistry} * which also listens for {@link @@ -47,113 +46,123 @@ import javax.servlet.http.HttpSession; * web.xml so that this class is notified of sessions that * expire. *

- * + * * @author Ben Alex - * @version $Id${date} + * @version $Id: SessionRegistryImpl.java 1862 2007-05-25 01:38:42Z benalex + * ${date} */ public class SessionRegistryImpl implements SessionRegistry, - ApplicationListener { - //~ Instance fields ======================================================== + ApplicationListener { + // ~ Instance fields + // ======================================================== - private Map principals = Collections.synchronizedMap(new HashMap()); // - private Map sessionIds = Collections.synchronizedMap(new HashMap()); // + private Map principals = Collections.synchronizedMap(new HashMap()); // + private Map sessionIds = Collections.synchronizedMap(new HashMap()); // - //~ Methods ================================================================ + // ~ Methods + // ================================================================ - public Object[] getAllPrincipals() { - return principals.keySet().toArray(); - } + public Object[] getAllPrincipals() { + return principals.keySet().toArray(); + } - public SessionInformation[] getAllSessions(Object principal, - boolean includeExpiredSessions) { - Set sessionsUsedByPrincipal = (Set) principals.get(principal); + public SessionInformation[] getAllSessions(Object principal, + boolean includeExpiredSessions) { + Set sessionsUsedByPrincipal = (Set) principals.get(principal); + if (sessionsUsedByPrincipal == null) { + return null; + } - if (sessionsUsedByPrincipal == null) { - return null; - } + List list = new ArrayList(); - List list = new ArrayList(); - Iterator iter = sessionsUsedByPrincipal.iterator(); + synchronized (sessionsUsedByPrincipal) { + for (Iterator iter = sessionsUsedByPrincipal.iterator(); iter + .hasNext();) { + String sessionId = (String) iter.next(); + SessionInformation sessionInformation = getSessionInformation(sessionId); - while (iter.hasNext()) { - synchronized (sessionsUsedByPrincipal) { - String sessionId = (String) iter.next(); - SessionInformation sessionInformation = getSessionInformation(sessionId); + if (includeExpiredSessions || !sessionInformation.isExpired()) { + list.add(sessionInformation); + } + } + } - if (includeExpiredSessions || !sessionInformation.isExpired()) { - list.add(sessionInformation); - } - } - } + return (SessionInformation[]) list.toArray(new SessionInformation[] {}); + } - return (SessionInformation[]) list.toArray(new SessionInformation[] {}); - } + public SessionInformation getSessionInformation(String sessionId) { + Assert.hasText(sessionId, + "SessionId required as per interface contract"); - public SessionInformation getSessionInformation(String sessionId) { - Assert.hasText(sessionId, "SessionId required as per interface contract"); + return (SessionInformation) sessionIds.get(sessionId); + } - return (SessionInformation) sessionIds.get(sessionId); - } + public void onApplicationEvent(ApplicationEvent event) { + if (event instanceof HttpSessionDestroyedEvent) { + String sessionId = ((HttpSession) event.getSource()).getId(); + removeSessionInformation(sessionId); + } + } - public void onApplicationEvent(ApplicationEvent event) { - if (event instanceof HttpSessionDestroyedEvent) { - String sessionId = ((HttpSession) event.getSource()).getId(); - removeSessionInformation(sessionId); - } - } + public void refreshLastRequest(String sessionId) { + Assert.hasText(sessionId, + "SessionId required as per interface contract"); - public void refreshLastRequest(String sessionId) { - Assert.hasText(sessionId, "SessionId required as per interface contract"); + SessionInformation info = getSessionInformation(sessionId); - SessionInformation info = getSessionInformation(sessionId); + if (info != null) { + info.refreshLastRequest(); + } + } - if (info != null) { - info.refreshLastRequest(); - } - } + public synchronized void registerNewSession(String sessionId, + Object principal) { + Assert.hasText(sessionId, + "SessionId required as per interface contract"); + Assert.notNull(principal, + "Principal required as per interface contract"); - public synchronized void registerNewSession(String sessionId, Object principal) { - Assert.hasText(sessionId, "SessionId required as per interface contract"); - Assert.notNull(principal, "Principal required as per interface contract"); + if (getSessionInformation(sessionId) != null) { + removeSessionInformation(sessionId); + } - if (getSessionInformation(sessionId) != null) { - removeSessionInformation(sessionId); - } + sessionIds.put(sessionId, new SessionInformation(principal, sessionId, + new Date())); - sessionIds.put(sessionId, - new SessionInformation(principal, sessionId, new Date())); + Set sessionsUsedByPrincipal = (Set) principals.get(principal); - Set sessionsUsedByPrincipal = (Set) principals.get(principal); + if (sessionsUsedByPrincipal == null) { + sessionsUsedByPrincipal = Collections + .synchronizedSet(new HashSet()); + } - if (sessionsUsedByPrincipal == null) { - sessionsUsedByPrincipal = Collections.synchronizedSet(new HashSet()); - } + sessionsUsedByPrincipal.add(sessionId); - sessionsUsedByPrincipal.add(sessionId); + principals.put(principal, sessionsUsedByPrincipal); + } - principals.put(principal, sessionsUsedByPrincipal); - } + public void removeSessionInformation(String sessionId) { + Assert.hasText(sessionId, + "SessionId required as per interface contract"); - public void removeSessionInformation(String sessionId) { - Assert.hasText(sessionId, "SessionId required as per interface contract"); + SessionInformation info = getSessionInformation(sessionId); - SessionInformation info = getSessionInformation(sessionId); + if (info != null) { + sessionIds.remove(sessionId); - if (info != null) { - sessionIds.remove(sessionId); + Set sessionsUsedByPrincipal = (Set) principals.get(info + .getPrincipal()); - Set sessionsUsedByPrincipal = (Set) principals.get(info.getPrincipal()); + if (sessionsUsedByPrincipal != null) { + synchronized (sessionsUsedByPrincipal) { + sessionsUsedByPrincipal.remove(sessionId); - if (sessionsUsedByPrincipal != null) { - synchronized (sessionsUsedByPrincipal) { - sessionsUsedByPrincipal.remove(sessionId); - - if (sessionsUsedByPrincipal.size() == 0) { - // No need to keep object in principals Map anymore - principals.remove(info.getPrincipal()); - } - } - } - } - } + if (sessionsUsedByPrincipal.size() == 0) { + // No need to keep object in principals Map anymore + principals.remove(info.getPrincipal()); + } + } + } + } + } } diff --git a/core/src/test/java/org/acegisecurity/concurrent/SessionRegistryImplTests.java b/core/src/test/java/org/acegisecurity/concurrent/SessionRegistryImplTests.java index 9cf3a49489..7f733b0ccc 100644 --- a/core/src/test/java/org/acegisecurity/concurrent/SessionRegistryImplTests.java +++ b/core/src/test/java/org/acegisecurity/concurrent/SessionRegistryImplTests.java @@ -23,137 +23,153 @@ import org.springframework.mock.web.MockHttpSession; import java.util.Date; - /** * Tests {@link SessionRegistryImpl}. - * + * * @author Ben Alex * @version $Id$ */ public class SessionRegistryImplTests extends TestCase { - //~ Methods ======================================================================================================== + // ~ Methods + // ======================================================================================================== - public void testEventPublishing() { - MockHttpSession httpSession = new MockHttpSession(); - Object principal = "Some principal object"; - String sessionId = httpSession.getId(); - assertNotNull(sessionId); + public void testEventPublishing() { + MockHttpSession httpSession = new MockHttpSession(); + Object principal = "Some principal object"; + String sessionId = httpSession.getId(); + assertNotNull(sessionId); - SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); + SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); - // Register new Session - sessionRegistry.registerNewSession(sessionId, principal); + // Register new Session + sessionRegistry.registerNewSession(sessionId, principal); - // Deregister session via an ApplicationEvent - sessionRegistry.onApplicationEvent(new HttpSessionDestroyedEvent(httpSession)); + // Deregister session via an ApplicationEvent + sessionRegistry.onApplicationEvent(new HttpSessionDestroyedEvent( + httpSession)); - // Check attempts to retrieve cleared session return null - assertNull(sessionRegistry.getSessionInformation(sessionId)); - } + // Check attempts to retrieve cleared session return null + assertNull(sessionRegistry.getSessionInformation(sessionId)); + } - public void testMultiplePrincipals() throws Exception { - Object principal1 = "principal_1"; - Object principal2 = "principal_2"; - String sessionId1 = "1234567890"; - String sessionId2 = "9876543210"; - String sessionId3 = "5432109876"; + public void testMultiplePrincipals() throws Exception { + Object principal1 = "principal_1"; + Object principal2 = "principal_2"; + String sessionId1 = "1234567890"; + String sessionId2 = "9876543210"; + String sessionId3 = "5432109876"; - SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); + SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); - sessionRegistry.registerNewSession(sessionId1, principal1); - sessionRegistry.registerNewSession(sessionId2, principal1); - sessionRegistry.registerNewSession(sessionId3, principal2); + sessionRegistry.registerNewSession(sessionId1, principal1); + sessionRegistry.registerNewSession(sessionId2, principal1); + sessionRegistry.registerNewSession(sessionId3, principal2); - assertEquals(principal1, sessionRegistry.getAllPrincipals()[0]); - assertEquals(principal2, sessionRegistry.getAllPrincipals()[1]); - } + assertEquals(principal1, sessionRegistry.getAllPrincipals()[0]); + assertEquals(principal2, sessionRegistry.getAllPrincipals()[1]); + } - public void testSessionInformationLifecycle() throws Exception { - Object principal = "Some principal object"; - String sessionId = "1234567890"; - SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); + public void testSessionInformationLifecycle() throws Exception { + Object principal = "Some principal object"; + String sessionId = "1234567890"; + SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); - // Register new Session - sessionRegistry.registerNewSession(sessionId, principal); + // Register new Session + sessionRegistry.registerNewSession(sessionId, principal); - // Retrieve existing session by session ID - Date currentDateTime = sessionRegistry.getSessionInformation(sessionId).getLastRequest(); - assertEquals(principal, sessionRegistry.getSessionInformation(sessionId).getPrincipal()); - assertEquals(sessionId, sessionRegistry.getSessionInformation(sessionId).getSessionId()); - assertNotNull(sessionRegistry.getSessionInformation(sessionId).getLastRequest()); + // Retrieve existing session by session ID + Date currentDateTime = sessionRegistry.getSessionInformation(sessionId) + .getLastRequest(); + assertEquals(principal, sessionRegistry + .getSessionInformation(sessionId).getPrincipal()); + assertEquals(sessionId, sessionRegistry + .getSessionInformation(sessionId).getSessionId()); + assertNotNull(sessionRegistry.getSessionInformation(sessionId) + .getLastRequest()); - // Retrieve existing session by principal - assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); + // Retrieve existing session by principal + assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); - // Sleep to ensure SessionRegistryImpl will update time - Thread.sleep(1000); + // Sleep to ensure SessionRegistryImpl will update time + Thread.sleep(1000); - // Update request date/time - sessionRegistry.refreshLastRequest(sessionId); + // Update request date/time + sessionRegistry.refreshLastRequest(sessionId); - Date retrieved = sessionRegistry.getSessionInformation(sessionId).getLastRequest(); - assertTrue(retrieved.after(currentDateTime)); + Date retrieved = sessionRegistry.getSessionInformation(sessionId) + .getLastRequest(); + assertTrue(retrieved.after(currentDateTime)); - // Check it retrieves correctly when looked up via principal - assertEquals(retrieved, sessionRegistry.getAllSessions(principal, false)[0].getLastRequest()); + // Check it retrieves correctly when looked up via principal + assertEquals(retrieved, sessionRegistry + .getAllSessions(principal, false)[0].getLastRequest()); - // Clear session information - sessionRegistry.removeSessionInformation(sessionId); + // Clear session information + sessionRegistry.removeSessionInformation(sessionId); - // Check attempts to retrieve cleared session return null - assertNull(sessionRegistry.getSessionInformation(sessionId)); - assertNull(sessionRegistry.getAllSessions(principal, false)); - } + // Check attempts to retrieve cleared session return null + assertNull(sessionRegistry.getSessionInformation(sessionId)); + assertNull(sessionRegistry.getAllSessions(principal, false)); + } - public void testTwoSessionsOnePrincipalExpiring() throws Exception { - Object principal = "Some principal object"; - String sessionId1 = "1234567890"; - String sessionId2 = "9876543210"; - SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); + public void testTwoSessionsOnePrincipalExpiring() throws Exception { + Object principal = "Some principal object"; + String sessionId1 = "1234567890"; + String sessionId2 = "9876543210"; + SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); - // Register new Session - sessionRegistry.registerNewSession(sessionId1, principal); - assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); - assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, false)[0].getSessionId()); + // Register new Session + sessionRegistry.registerNewSession(sessionId1, principal); + assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); + assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, + false)[0].getSessionId()); - // Register new Session - sessionRegistry.registerNewSession(sessionId2, principal); - assertEquals(2, sessionRegistry.getAllSessions(principal, false).length); - assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[1].getSessionId()); + // Register new Session + sessionRegistry.registerNewSession(sessionId2, principal); + assertEquals(2, sessionRegistry.getAllSessions(principal, false).length); + assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, + false)[1].getSessionId()); - // Expire one session - SessionInformation session = sessionRegistry.getSessionInformation(sessionId2); - session.expireNow(); + // Expire one session + SessionInformation session = sessionRegistry + .getSessionInformation(sessionId2); + session.expireNow(); - // Check retrieval still correct - assertTrue(sessionRegistry.getSessionInformation(sessionId2).isExpired()); - assertFalse(sessionRegistry.getSessionInformation(sessionId1).isExpired()); - } + // Check retrieval still correct + assertTrue(sessionRegistry.getSessionInformation(sessionId2) + .isExpired()); + assertFalse(sessionRegistry.getSessionInformation(sessionId1) + .isExpired()); + } - public void testTwoSessionsOnePrincipalHandling() throws Exception { - Object principal = "Some principal object"; - String sessionId1 = "1234567890"; - String sessionId2 = "9876543210"; - SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); + public void testTwoSessionsOnePrincipalHandling() throws Exception { + Object principal = "Some principal object"; + String sessionId1 = "1234567890"; + String sessionId2 = "9876543210"; + SessionRegistryImpl sessionRegistry = new SessionRegistryImpl(); - // Register new Session - sessionRegistry.registerNewSession(sessionId1, principal); - assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); - assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, false)[0].getSessionId()); + // Register new Session + sessionRegistry.registerNewSession(sessionId1, principal); + assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); + assertEquals(sessionId1, sessionRegistry.getAllSessions(principal, + false)[0].getSessionId()); - // Register new Session - sessionRegistry.registerNewSession(sessionId2, principal); - assertEquals(2, sessionRegistry.getAllSessions(principal, false).length); - assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[1].getSessionId()); + // Register new Session + sessionRegistry.registerNewSession(sessionId2, principal); + assertEquals(2, sessionRegistry.getAllSessions(principal, false).length); + assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, + false)[1].getSessionId()); - // Clear session information - sessionRegistry.removeSessionInformation(sessionId1); - assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); - assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, false)[0].getSessionId()); + // Clear session information + sessionRegistry.removeSessionInformation(sessionId1); + assertEquals(1, sessionRegistry.getAllSessions(principal, false).length); + assertEquals(sessionId2, sessionRegistry.getAllSessions(principal, + false)[0].getSessionId()); + + // Clear final session + sessionRegistry.removeSessionInformation(sessionId2); + assertNull(sessionRegistry.getSessionInformation(sessionId2)); + assertNull(sessionRegistry.getAllSessions(principal, false)); + } - // Clear final session - sessionRegistry.removeSessionInformation(sessionId2); - assertNull(sessionRegistry.getSessionInformation(sessionId2)); - assertNull(sessionRegistry.getAllSessions(principal, false)); - } }