From 7bbf5f83b5194501a3c9f84a25f17d753205e4a6 Mon Sep 17 00:00:00 2001 From: Ruslan Molchanov Date: Tue, 28 Dec 2021 16:46:32 +0300 Subject: [PATCH] Fix memory leak with null principal in Redis Closes gh-1987 --- .../RedisIndexedSessionRepositoryITests.java | 24 +++++++++++++++++++ .../redis/RedisIndexedSessionRepository.java | 12 ++++++---- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java index e1665aeeb..08745fab1 100644 --- a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryITests.java @@ -527,6 +527,30 @@ void changeSessionIdWhenPrincipalNameChangesThenNewPrincipalMapsToNewSessionId() assertThat(findByPrincipalName.keySet()).containsOnly(changeSessionId); } + @Test // gh-1987 + void changeSessionIdWhenPrincipalNameChangesFromNullThenIndexShouldNotBeCreated() { + String principalName = null; + String principalNameChanged = "findByChangedPrincipalName" + UUID.randomUUID(); + RedisSession toSave = this.repository.createSession(); + toSave.setAttribute(INDEX_NAME, principalName); + + this.repository.save(toSave); + + RedisSession findById = this.repository.findById(toSave.getId()); + String changeSessionId = findById.changeSessionId(); + findById.setAttribute(INDEX_NAME, principalNameChanged); + this.repository.save(findById); + + Map findByPrincipalName = this.repository.findByIndexNameAndIndexValue(INDEX_NAME, + principalName); + assertThat(findByPrincipalName).isEmpty(); + + findByPrincipalName = this.repository.findByIndexNameAndIndexValue(INDEX_NAME, principalNameChanged); + + assertThat(findByPrincipalName).hasSize(1); + assertThat(findByPrincipalName.keySet()).containsOnly(changeSessionId); + } + @Test void changeSessionIdWhenOnlyChangeId() { String attrName = "changeSessionId"; diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java index e1cd731dd..19f3e0a25 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java @@ -858,11 +858,13 @@ private void saveChangeSessionId() { catch (NonTransientDataAccessException ex) { handleErrNoSuchKeyError(ex); } - String originalPrincipalRedisKey = getPrincipalKey(this.originalPrincipalName); - RedisIndexedSessionRepository.this.sessionRedisOperations.boundSetOps(originalPrincipalRedisKey) - .remove(this.originalSessionId); - RedisIndexedSessionRepository.this.sessionRedisOperations.boundSetOps(originalPrincipalRedisKey) - .add(sessionId); + if (this.originalPrincipalName != null) { + String originalPrincipalRedisKey = getPrincipalKey(this.originalPrincipalName); + RedisIndexedSessionRepository.this.sessionRedisOperations.boundSetOps(originalPrincipalRedisKey) + .remove(this.originalSessionId); + RedisIndexedSessionRepository.this.sessionRedisOperations.boundSetOps(originalPrincipalRedisKey) + .add(sessionId); + } } this.originalSessionId = sessionId; }