From 1a7f71fc0faf024cf549d994fb486b9eb6ed6885 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Tue, 19 Jan 2010 01:07:33 +0000 Subject: [PATCH] SEC-1372: Return an empty list rather than null from SessionRegistryImpl.getAllSessions() If the principal has no sessions, null is returned which contradicts the interface contract. In practice it didn't matter as the null was checked for, but it is cleaner to disallow a null value. --- .../security/core/session/SessionRegistryImpl.java | 2 +- .../security/core/session/SessionRegistryImplTests.java | 4 ++-- .../session/ConcurrentSessionControlStrategy.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java b/core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java index 39e8f22c7a..2180a37e52 100644 --- a/core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java +++ b/core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java @@ -62,7 +62,7 @@ public class SessionRegistryImpl implements SessionRegistry, ApplicationListener final Set sessionsUsedByPrincipal = principals.get(principal); if (sessionsUsedByPrincipal == null) { - return null; + return Collections.emptyList(); } List list = new ArrayList(sessionsUsedByPrincipal.size()); diff --git a/core/src/test/java/org/springframework/security/core/session/SessionRegistryImplTests.java b/core/src/test/java/org/springframework/security/core/session/SessionRegistryImplTests.java index 28c7c5ce48..7949b31968 100644 --- a/core/src/test/java/org/springframework/security/core/session/SessionRegistryImplTests.java +++ b/core/src/test/java/org/springframework/security/core/session/SessionRegistryImplTests.java @@ -117,7 +117,7 @@ public class SessionRegistryImplTests { // Check attempts to retrieve cleared session return null assertNull(sessionRegistry.getSessionInformation(sessionId)); - assertNull(sessionRegistry.getAllSessions(principal, false)); + assertEquals(0, sessionRegistry.getAllSessions(principal, false).size()); } @Test @@ -168,7 +168,7 @@ public class SessionRegistryImplTests { sessionRegistry.removeSessionInformation(sessionId2); assertNull(sessionRegistry.getSessionInformation(sessionId2)); - assertNull(sessionRegistry.getAllSessions(principal, false)); + assertEquals(0, sessionRegistry.getAllSessions(principal, false).size()); } private boolean contains(String sessionId, Object principal) { diff --git a/web/src/main/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategy.java b/web/src/main/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategy.java index 73012a159a..513e120c56 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategy.java +++ b/web/src/main/java/org/springframework/security/web/authentication/session/ConcurrentSessionControlStrategy.java @@ -69,7 +69,7 @@ public class ConcurrentSessionControlStrategy extends SessionFixationProtectionS final List sessions = sessionRegistry.getAllSessions(authentication.getPrincipal(), false); - int sessionCount = sessions == null ? 0 : sessions.size(); + int sessionCount = sessions.size(); int allowedSessions = getMaximumSessionsForThisUser(authentication); if (sessionCount < allowedSessions) {