From 1edce117aae86931b73cef7cb34df9a6b45474fd Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Fri, 5 Jan 2018 12:51:24 +0100 Subject: [PATCH] Fix Redis change session id handling This commit updates logic around changing session id in `RedisOperationsSessionRepository` to properly handle updates for new sessions i.e. ones that haven't been saved yet. Previously, the logic skipped both Redis rename operation and replacement of session id within the current session holder object, which led to no such key errors on subsequent save operation which still observed the session id as changed. Closes gh-962 --- .../RedisOperationsSessionRepositoryITests.java | 16 +++++++++++++++- .../redis/RedisOperationsSessionRepository.java | 12 +++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java index 57aae4ec..2aa61fb6 100644 --- a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -567,6 +567,20 @@ public class RedisOperationsSessionRepositoryITests extends AbstractRedisITests assertThat(this.repository.findById(originalId)).isNull(); } + // gh-962 + @Test + public void changeSessionIdSaveTwice() { + RedisSession toSave = this.repository.createSession(); + String originalId = toSave.getId(); + toSave.changeSessionId(); + + this.repository.save(toSave); + this.repository.save(toSave); + + assertThat(this.repository.findById(toSave.getId())).isNotNull(); + assertThat(this.repository.findById(originalId)).isNull(); + } + private String getSecurityName() { return this.context.getAuthentication().getName(); } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java index 654afcdd..fbf14168 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -804,10 +804,12 @@ public class RedisOperationsSessionRepository implements } private void saveChangeSessionId(String sessionId) { - if (!isNew() && !sessionId.equals(this.originalSessionId)) { - String originalSessionIdKey = getSessionKey(this.originalSessionId); - String sessionIdKey = getSessionKey(sessionId); - RedisOperationsSessionRepository.this.sessionRedisOperations.rename(originalSessionIdKey, sessionIdKey); + if (!sessionId.equals(this.originalSessionId)) { + if (!isNew()) { + String originalSessionIdKey = getSessionKey(this.originalSessionId); + String sessionIdKey = getSessionKey(sessionId); + RedisOperationsSessionRepository.this.sessionRedisOperations.rename(originalSessionIdKey, sessionIdKey); + } this.originalSessionId = sessionId; } }