Skip to content

Commit baae780

Browse files
committed
Rewrite session(id) creation code. Fixes #22. Fixes #23.
* Fix possible infinite looping. * Fix unnecessary multiple saves to Redis. * Comply with Manager implementation by returning null if specific session ID is requested but is already taken. * Fix bug allowing duplicate session IDs.
1 parent 4587d19 commit baae780

File tree

1 file changed

+26
-20
lines changed

1 file changed

+26
-20
lines changed

src/main/java/com/radiadesign/catalina/session/RedisSessionManager.java

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -250,33 +250,32 @@ protected synchronized void stopInternal() throws LifecycleException {
250250
}
251251

252252
@Override
253-
public Session createSession(String sessionId) {
254-
RedisSession session = (RedisSession)createEmptySession();
255-
256-
// Initialize the properties of the new session and return it
257-
session.setNew(true);
258-
session.setValid(true);
259-
session.setCreationTime(System.currentTimeMillis());
260-
session.setMaxInactiveInterval(getMaxInactiveInterval());
261-
253+
public Session createSession(String requestedSessionId) {
254+
RedisSession session = null;
255+
String sessionId = null;
262256
String jvmRoute = getJvmRoute();
263257

264258
Boolean error = true;
265259
Jedis jedis = null;
266-
267260
try {
268261
jedis = acquireConnection();
269262

270263
// Ensure generation of a unique session identifier.
271-
do {
272-
if (null == sessionId) {
273-
sessionId = generateSessionId();
274-
}
275-
264+
if (null == requestedSessionId) {
276265
if (jvmRoute != null) {
277-
sessionId += '.' + jvmRoute;
266+
sessionId = requestedSessionId + '.' + jvmRoute;
267+
}
268+
if (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L) {
269+
sessionId = null;
278270
}
279-
} while (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 1L); // 1 = key set; 0 = key already existed
271+
} else {
272+
do {
273+
sessionId = generateSessionId();
274+
if (jvmRoute != null) {
275+
sessionId += '.' + jvmRoute;
276+
}
277+
} while (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L); // 1 = key set; 0 = key already existed
278+
}
280279

281280
/* Even though the key is set in Redis, we are not going to flag
282281
the current thread as having had the session persisted since
@@ -286,14 +285,21 @@ This ensures that the save(session) at the end of the request
286285

287286
error = false;
288287

289-
session.setId(sessionId);
290-
session.tellNew();
288+
if (null != sessionId) {
289+
session = (RedisSession)createEmptySession();
290+
session.setNew(true);
291+
session.setValid(true);
292+
session.setCreationTime(System.currentTimeMillis());
293+
session.setMaxInactiveInterval(getMaxInactiveInterval());
294+
session.setId(sessionId);
295+
session.tellNew();
296+
}
291297

292298
currentSession.set(session);
293299
currentSessionId.set(sessionId);
294300
currentSessionIsPersisted.set(false);
295301

296-
if (this.getSaveOnChange()) {
302+
if (null != session && this.getSaveOnChange()) {
297303
try {
298304
save(session);
299305
} catch (IOException ex) {

0 commit comments

Comments
 (0)