From c2280ff69154edb2dc648feb12c6bdcc08247cf6 Mon Sep 17 00:00:00 2001 From: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com> Date: Thu, 13 Aug 2026 23:23:52 +0900 Subject: [PATCH] Prevent re-adding deleted sessions to Redis principal index RedisSession#saveDelta() unconditionally re-adds the session id to the resolved principal's index set whenever the delta contains the principal/security-context attribute key. RedisIndexedSessionRepository #deleteById() first removes the session id via cleanupPrincipalIndex(), then sets maxInactiveInterval to Duration.ZERO and calls save(session), which invokes saveDelta(). Under SaveMode.ALWAYS, the RedisSession constructor unconditionally copies every current attribute (including the principal-index attribute) into the delta, so saveDelta() always takes the principal-index branch on this path. Since the session's own attributes are untouched by deleteById(), resolveIndexesFor() still resolves the same principal, and saveDelta() re-adds the id to the index set that cleanupPrincipalIndex() had just removed it from. The principal's Redis set (e.g. "spring:session:index:...:") then keeps growing with ids of sessions that were explicitly invalidated, leaking memory. Guard the re-add with RedisSession#isExpired(), which is already true at this point in deleteById() (maxInactiveInterval is set to Duration .ZERO, not a negative "never expires" value). The unconditional removal of the id from the previous principal's set is left untouched, since removal is always correct regardless of expiration state. Closes gh-1843 Signed-off-by: seonwoo_jung <79202163+seonwooj0810@users.noreply.github.com> --- .../redis/RedisIndexedSessionRepository.java | 2 +- .../RedisIndexedSessionRepositoryTests.java | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) 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 891ff88f9..30e7eec63 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 @@ -913,7 +913,7 @@ private void saveDelta() { Map indexes = RedisIndexedSessionRepository.this.indexResolver.resolveIndexesFor(this); String principal = indexes.get(PRINCIPAL_NAME_INDEX_NAME); this.originalPrincipalName = principal; - if (principal != null) { + if (principal != null && !isExpired()) { String principalRedisKey = getPrincipalKey(principal); RedisIndexedSessionRepository.this.sessionRedisOperations.boundSetOps(principalRedisKey) .add(sessionId); diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java index 1ac304e65..fee6b9c53 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryTests.java @@ -334,6 +334,31 @@ void delete() { verify(this.redisOperations, never()).boundValueOps(getKey("expires:" + id)); } + // gh-1843 + @Test + void deleteWhenSaveModeAlwaysThenPrincipalIndexNotReAdded() { + String principalName = "principal"; + MapSession expected = new MapSession(); + expected.setLastAccessedTime(Instant.now().minusSeconds(60)); + expected.setAttribute(FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME, principalName); + given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations); + given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations); + Map map = map( + RedisIndexedSessionRepository + .getSessionAttrNameKey(FindByIndexNameSessionRepository.PRINCIPAL_NAME_INDEX_NAME), + principalName, RedisSessionMapper.CREATION_TIME_KEY, expected.getCreationTime().toEpochMilli(), + RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, (int) expected.getMaxInactiveInterval().getSeconds(), + RedisSessionMapper.LAST_ACCESSED_TIME_KEY, expected.getLastAccessedTime().toEpochMilli()); + given(this.boundHashOperations.entries()).willReturn(map); + this.redisRepository.setSaveMode(SaveMode.ALWAYS); + + String id = expected.getId(); + this.redisRepository.deleteById(id); + + verify(this.boundSetOperations, atLeastOnce()).remove(id); + verify(this.boundSetOperations, never()).add(id); + } + @Test void deleteNullSession() { given(this.redisOperations.boundHashOps(anyString())).willReturn(this.boundHashOperations);