Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix race condition in SessionRegistryImpl 

Adding/removing sessions from principals wasn't atomic. If one thread
removed the last session from a principal while another thread added a
new one, the addition could be lost.

Fixes gh-3189
This commit is contained in:
Jeffrey Morlan 2019-08-02 16:40:27 -07:00 committed by Rob Winch
parent da62c31fdc
commit a17d66463d
1 changed files with 31 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
core/src/main/java/org/springframework/security/core/session/SessionRegistryImpl.java
View File

@ -132,13 +132,18 @@ public class SessionRegistryImpl implements SessionRegistry,
sessionIds.put(sessionId, sessionIds.put(sessionId,
new SessionInformation(principal, sessionId, new Date())); new SessionInformation(principal, sessionId, new Date()));
Set<String> sessionsUsedByPrincipal = principals.computeIfAbsent(principal, key -> new CopyOnWriteArraySet<>()); principals.compute(principal, (key, sessionsUsedByPrincipal) -> {
sessionsUsedByPrincipal.add(sessionId); if (sessionsUsedByPrincipal == null) {
sessionsUsedByPrincipal = new CopyOnWriteArraySet<>();
}
sessionsUsedByPrincipal.add(sessionId);
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
logger.trace("Sessions used by '" + principal + "' : " logger.trace("Sessions used by '" + principal + "' : "
+ sessionsUsedByPrincipal); + sessionsUsedByPrincipal);
} }
return sessionsUsedByPrincipal;
});
} }
public void removeSessionInformation(String sessionId) { public void removeSessionInformation(String sessionId) {
@ -157,32 +162,29 @@ public class SessionRegistryImpl implements SessionRegistry,
sessionIds.remove(sessionId); sessionIds.remove(sessionId);
Set<String> sessionsUsedByPrincipal = principals.get(info.getPrincipal()); principals.computeIfPresent(info.getPrincipal(), (key, sessionsUsedByPrincipal) -> {
if (sessionsUsedByPrincipal == null) {
return;
}
if (logger.isDebugEnabled()) {
logger.debug("Removing session " + sessionId
+ " from principal's set of registered sessions");
}
sessionsUsedByPrincipal.remove(sessionId);
if (sessionsUsedByPrincipal.isEmpty()) {
// No need to keep object in principals Map anymore
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug("Removing principal " + info.getPrincipal() logger.debug("Removing session " + sessionId
+ " from registry"); + " from principal's set of registered sessions");
} }
principals.remove(info.getPrincipal());
}
if (logger.isTraceEnabled()) { sessionsUsedByPrincipal.remove(sessionId);
logger.trace("Sessions used by '" + info.getPrincipal() + "' : "
+ sessionsUsedByPrincipal); if (sessionsUsedByPrincipal.isEmpty()) {
} // No need to keep object in principals Map anymore
if (logger.isDebugEnabled()) {
logger.debug("Removing principal " + info.getPrincipal()
+ " from registry");
}
sessionsUsedByPrincipal = null;
}
if (logger.isTraceEnabled()) {
logger.trace("Sessions used by '" + info.getPrincipal() + "' : "
+ sessionsUsedByPrincipal);
}
return sessionsUsedByPrincipal;
});
} }
} }