From 5fd2c71559d420ffdb793ad1bf440b69edb300ad Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Wed, 19 Aug 2015 11:38:17 -0500 Subject: [PATCH] Minimize MapSession's use of Secure Random Previously when creating a MapSession from an existing session required that UUID.randomUUID() be invoked. This could slow down the system since it requires entropy. MapSession now has a constructor that accepts the id which prevents Secure random from being used when the session is already known. Fixes gh-271 --- .../org/springframework/session/MapSession.java | 16 ++++++++++++++-- .../redis/RedisOperationsSessionRepository.java | 3 +-- .../springframework/session/MapSessionTests.java | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/spring-session/src/main/java/org/springframework/session/MapSession.java b/spring-session/src/main/java/org/springframework/session/MapSession.java index b418d56..4fb709c 100644 --- a/spring-session/src/main/java/org/springframework/session/MapSession.java +++ b/spring-session/src/main/java/org/springframework/session/MapSession.java @@ -46,7 +46,7 @@ public final class MapSession implements ExpiringSession, Serializable { */ public static final int DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS = 1800; - private String id = UUID.randomUUID().toString(); + private String id; private Map sessionAttrs = new HashMap(); private long creationTime = System.currentTimeMillis(); private long lastAccessedTime = creationTime; @@ -57,9 +57,21 @@ public final class MapSession implements ExpiringSession, Serializable { private int maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL_SECONDS; /** - * Creates a new instance + * Creates a new instance with a secure randomly generated identifier. */ public MapSession() { + this(UUID.randomUUID().toString()); + } + + /** + * Creates a new instance with the specified id. This is preferred to the + * default constructor when the id is known to prevent unnecessary consumption on + * entropy which can be slow. + * + * @param id the identifier to use + */ + public MapSession(String id) { + this.id = id; } /** diff --git a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index c36d1eb..f8378d7 100644 --- a/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -398,8 +398,7 @@ public class RedisOperationsSessionRepository implements FindByPrincipalNameSess } private MapSession loadSession(String id, Map entries) { - MapSession loaded = new MapSession(); - loaded.setId(id); + MapSession loaded = new MapSession(id); for(Map.Entry entry : entries.entrySet()) { String key = (String) entry.getKey(); if(CREATION_TIME_ATTR.equals(key)) { diff --git a/spring-session/src/test/java/org/springframework/session/MapSessionTests.java b/spring-session/src/test/java/org/springframework/session/MapSessionTests.java index 9ccbc32..42873f5 100644 --- a/spring-session/src/test/java/org/springframework/session/MapSessionTests.java +++ b/spring-session/src/test/java/org/springframework/session/MapSessionTests.java @@ -34,7 +34,7 @@ public class MapSessionTests { @Test(expected = IllegalArgumentException.class) public void constructorNullSession() { - new MapSession(null); + new MapSession((ExpiringSession) null); } /**