From 381a07cb8c903df8099a0a141f9d054ebb13aaf4 Mon Sep 17 00:00:00 2001 From: Vedran Pavic Date: Tue, 29 Jan 2019 01:38:54 +0100 Subject: [PATCH] Ignore failed rename operation for deleted session In scenario with concurrent requests attempting to change session id, the "ERR no such key" error will occur for a thread that comes in second. This commit addresses the problem by ignoring the aforementioned error. Resolves: #1270 --- ...edisOperationsSessionRepositoryITests.java | 21 +++++++++++++- .../RedisOperationsSessionRepository.java | 28 +++++++++++++------ 2 files changed, 39 insertions(+), 10 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 8e95658f..d8d22072 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-2018 the original author or authors. + * Copyright 2014-2019 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. @@ -600,6 +600,25 @@ public class RedisOperationsSessionRepositoryITests extends AbstractRedisITests assertThat(this.repository.findById(sessionId)).isNull(); } + @Test // gh-1270 + public void changeSessionIdSaveConcurrently() { + RedisSession toSave = this.repository.createSession(); + String originalId = toSave.getId(); + this.repository.save(toSave); + + RedisSession copy1 = this.repository.findById(originalId); + RedisSession copy2 = this.repository.findById(originalId); + + copy1.changeSessionId(); + this.repository.save(copy1); + copy2.changeSessionId(); + this.repository.save(copy2); + + assertThat(this.repository.findById(originalId)).isNull(); + assertThat(this.repository.findById(copy1.getId())).isNotNull(); + assertThat(this.repository.findById(copy2.getId())).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 042813b9..ba556fb2 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-2018 the original author or authors. + * Copyright 2014-2019 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. @@ -864,24 +864,34 @@ public class RedisOperationsSessionRepository implements if (!isNew()) { String originalSessionIdKey = getSessionKey(this.originalSessionId); String sessionIdKey = getSessionKey(sessionId); - RedisOperationsSessionRepository.this.sessionRedisOperations.rename( - originalSessionIdKey, sessionIdKey); + try { + RedisOperationsSessionRepository.this.sessionRedisOperations + .rename(originalSessionIdKey, sessionIdKey); + } + catch (NonTransientDataAccessException ex) { + handleErrNoSuchKeyError(ex); + } String originalExpiredKey = getExpiredKey(this.originalSessionId); String expiredKey = getExpiredKey(sessionId); try { - RedisOperationsSessionRepository.this.sessionRedisOperations.rename( - originalExpiredKey, expiredKey); + RedisOperationsSessionRepository.this.sessionRedisOperations + .rename(originalExpiredKey, expiredKey); } catch (NonTransientDataAccessException ex) { - if (!"ERR no such key".equals(NestedExceptionUtils - .getMostSpecificCause(ex).getMessage())) { - throw ex; - } + handleErrNoSuchKeyError(ex); } } this.originalSessionId = sessionId; } } + + private void handleErrNoSuchKeyError(NonTransientDataAccessException ex) { + if (!"ERR no such key" + .equals(NestedExceptionUtils.getMostSpecificCause(ex).getMessage())) { + throw ex; + } + } + } /**