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:...:<principal>")
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>

Author: seonwooj0810

spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisIndexedSessionRepository.java

@@ -913,7 +913,7 @@ private void saveDelta() {
 				Map<String, String> 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);

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.<String, Object>boundHashOps(anyString())).willReturn(this.boundHashOperations);
+		given(this.redisOperations.boundSetOps(anyString())).willReturn(this.boundSetOperations);
+		Map<String, Object> 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.<String, Object>boundHashOps(anyString())).willReturn(this.boundHashOperations);