diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryKeyMissITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryKeyMissITests.java new file mode 100644 index 00000000..9b5dc26c --- /dev/null +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/ReactiveRedisSessionRepositoryKeyMissITests.java @@ -0,0 +1,150 @@ +/* + * Copyright 2014-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.session.data.redis; + +import java.time.Instant; +import java.util.Map; +import java.util.function.BiFunction; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import reactor.core.publisher.Mono; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.core.ReactiveHashOperations; +import org.springframework.data.redis.core.ReactiveRedisOperations; +import org.springframework.session.MapSession; +import org.springframework.session.config.ReactiveSessionRepositoryCustomizer; +import org.springframework.session.data.redis.ReactiveRedisSessionRepository.RedisSession; +import org.springframework.session.data.redis.config.annotation.web.server.EnableRedisWebSession; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; +import static org.mockito.Mockito.spy; + +/** + * Key miss error tests for {@link ReactiveRedisSessionRepository} + * + * @author Marcus da Coregio + * @see Related + * GitHub Issue + */ +@ExtendWith(SpringExtension.class) +class ReactiveRedisSessionRepositoryKeyMissITests extends AbstractRedisITests { + + private ReactiveRedisSessionRepository sessionRepository; + + private ReactiveRedisOperations spyOperations; + + AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaThenThrowIllegalStateException() { + this.context.register(Config.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + ReactiveHashOperations opsForHash = spy(this.spyOperations.opsForHash()); + given(this.spyOperations.opsForHash()).willReturn(opsForHash); + willAnswer((invocation) -> this.sessionRepository.deleteById(session.getId()) + .then((Mono) invocation.callRealMethod())).given(opsForHash).putAll(any(), any()); + + this.sessionRepository.save(session).block(); + assertThatIllegalStateException().isThrownBy(() -> this.sessionRepository.findById(session.getId()).block()) + .withMessage("creationTime key must not be null"); + } + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaAndSafeMapperThenSessionIsNull() { + this.context.register(RedisSessionMapperConfig.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + ReactiveHashOperations opsForHash = spy(this.spyOperations.opsForHash()); + given(this.spyOperations.opsForHash()).willReturn(opsForHash); + willAnswer((invocation) -> this.sessionRepository.deleteById(session.getId()) + .then((Mono) invocation.callRealMethod())).given(opsForHash).putAll(any(), any()); + + this.sessionRepository.save(session).block(); + assertThat(this.sessionRepository.findById(session.getId()).block()).isNull(); + } + + @SuppressWarnings("unchecked") + private void refreshAndPrepareFields() { + this.context.refresh(); + this.sessionRepository = this.context.getBean(ReactiveRedisSessionRepository.class); + ReactiveRedisOperations redisOperations = (ReactiveRedisOperations) ReflectionTestUtils + .getField(this.sessionRepository, "sessionRedisOperations"); + this.spyOperations = spy(redisOperations); + ReflectionTestUtils.setField(this.sessionRepository, "sessionRedisOperations", this.spyOperations); + } + + private RedisSession createAndSaveSession(Instant lastAccessedTime) { + RedisSession session = this.sessionRepository.createSession().block(); + session.setLastAccessedTime(lastAccessedTime); + session.setAttribute("attribute1", "value1"); + this.sessionRepository.save(session).block(); + return this.sessionRepository.findById(session.getId()).block(); + } + + @Configuration + @EnableRedisWebSession + static class Config extends BaseConfig { + + } + + @Configuration + @EnableRedisWebSession + static class RedisSessionMapperConfig extends BaseConfig { + + @Bean + ReactiveSessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository + .setRedisSessionMapper(new SafeRedisSessionMapper(redisSessionRepository)); + } + + } + + static class SafeRedisSessionMapper implements BiFunction, Mono> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final ReactiveRedisSessionRepository sessionRepository; + + SafeRedisSessionMapper(ReactiveRedisSessionRepository sessionRepository) { + this.sessionRepository = sessionRepository; + } + + @Override + public Mono apply(String sessionId, Map map) { + return Mono.fromSupplier(() -> this.delegate.apply(sessionId, map)).onErrorResume( + IllegalStateException.class, + (ex) -> this.sessionRepository.deleteById(sessionId).then(Mono.empty())); + } + + } + +} diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryKeyMissITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryKeyMissITests.java new file mode 100644 index 00000000..774c3d24 --- /dev/null +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisIndexedSessionRepositoryKeyMissITests.java @@ -0,0 +1,158 @@ +/* + * Copyright 2014-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.session.data.redis; + +import java.time.Instant; +import java.util.Map; +import java.util.function.BiFunction; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.core.BoundHashOperations; +import org.springframework.data.redis.core.RedisOperations; +import org.springframework.session.MapSession; +import org.springframework.session.config.SessionRepositoryCustomizer; +import org.springframework.session.data.redis.RedisIndexedSessionRepository.RedisSession; +import org.springframework.session.data.redis.config.annotation.web.http.EnableRedisIndexedHttpSession; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; +import static org.mockito.Mockito.spy; + +/** + * Key miss error tests for {@link RedisIndexedSessionRepository} + * + * @author Marcus da Coregio + * @see Related + * GitHub Issue + */ +@ExtendWith(SpringExtension.class) +class RedisIndexedSessionRepositoryKeyMissITests extends AbstractRedisITests { + + private RedisIndexedSessionRepository sessionRepository; + + private RedisOperations spyOperations; + + AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaThenThrowIllegalStateException() { + this.context.register(Config.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + BoundHashOperations opsForHash = spy(this.spyOperations.boundHashOps(anyString())); + given(this.spyOperations.boundHashOps(anyString())).willReturn(opsForHash); + willAnswer((invocation) -> { + this.sessionRepository.deleteById(session.getId()); + return invocation.callRealMethod(); + }).given(opsForHash).putAll(any()); + + this.sessionRepository.save(session); + assertThatIllegalStateException().isThrownBy(() -> this.sessionRepository.findById(session.getId())) + .withMessage("creationTime key must not be null"); + } + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaAndSafeMapperThenSessionIsNull() { + this.context.register(RedisSessionMapperConfig.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + BoundHashOperations opsForHash = spy(this.spyOperations.boundHashOps(anyString())); + given(this.spyOperations.boundHashOps(anyString())).willReturn(opsForHash); + willAnswer((invocation) -> { + this.sessionRepository.deleteById(session.getId()); + return invocation.callRealMethod(); + }).given(opsForHash).putAll(any()); + + this.sessionRepository.save(session); + assertThat(this.sessionRepository.findById(session.getId())).isNull(); + } + + @SuppressWarnings("unchecked") + private void refreshAndPrepareFields() { + this.context.refresh(); + this.sessionRepository = this.context.getBean(RedisIndexedSessionRepository.class); + RedisOperations redisOperations = (RedisOperations) ReflectionTestUtils + .getField(this.sessionRepository, "sessionRedisOperations"); + this.spyOperations = spy(redisOperations); + ReflectionTestUtils.setField(this.sessionRepository, "sessionRedisOperations", this.spyOperations); + } + + private RedisSession createAndSaveSession(Instant lastAccessedTime) { + RedisSession session = this.sessionRepository.createSession(); + session.setLastAccessedTime(lastAccessedTime); + session.setAttribute("attribute1", "value1"); + this.sessionRepository.save(session); + return this.sessionRepository.findById(session.getId()); + } + + @Configuration + @EnableRedisIndexedHttpSession + static class Config extends BaseConfig { + + } + + @Configuration + @EnableRedisIndexedHttpSession + static class RedisSessionMapperConfig extends BaseConfig { + + @Bean + SessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository.setRedisSessionMapper( + new SafeRedisSessionMapper(redisSessionRepository.getSessionRedisOperations())); + } + + } + + static class SafeRedisSessionMapper implements BiFunction, MapSession> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final RedisOperations redisOperations; + + SafeRedisSessionMapper(RedisOperations redisOperations) { + this.redisOperations = redisOperations; + } + + @Override + public MapSession apply(String sessionId, Map map) { + try { + return this.delegate.apply(sessionId, map); + } + catch (IllegalStateException ex) { + this.redisOperations.delete("spring:session:sessions:" + sessionId); + return null; + } + } + + } + +} diff --git a/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisSessionRepositoryKeyMissITests.java b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisSessionRepositoryKeyMissITests.java new file mode 100644 index 00000000..f636d620 --- /dev/null +++ b/spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisSessionRepositoryKeyMissITests.java @@ -0,0 +1,157 @@ +/* + * Copyright 2014-2023 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.session.data.redis; + +import java.time.Instant; +import java.util.Map; +import java.util.function.BiFunction; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.data.redis.core.HashOperations; +import org.springframework.data.redis.core.RedisOperations; +import org.springframework.session.MapSession; +import org.springframework.session.config.SessionRepositoryCustomizer; +import org.springframework.session.data.redis.RedisSessionRepository.RedisSession; +import org.springframework.session.data.redis.config.annotation.web.http.EnableRedisHttpSession; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; +import static org.mockito.Mockito.spy; + +/** + * Key miss error tests for {@link RedisSessionRepository} + * + * @author Marcus da Coregio + * @see Related + * GitHub Issue + */ +@ExtendWith(SpringExtension.class) +class RedisSessionRepositoryKeyMissITests extends AbstractRedisITests { + + private RedisSessionRepository sessionRepository; + + private RedisOperations spyOperations; + + AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaThenThrowIllegalStateException() { + this.context.register(Config.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + HashOperations opsForHash = spy(this.spyOperations.opsForHash()); + given(this.spyOperations.opsForHash()).willReturn(opsForHash); + willAnswer((invocation) -> { + this.sessionRepository.deleteById(session.getId()); + return invocation.callRealMethod(); + }).given(opsForHash).putAll(any(), any()); + + this.sessionRepository.save(session); + assertThatIllegalStateException().isThrownBy(() -> this.sessionRepository.findById(session.getId())) + .withMessage("creationTime key must not be null"); + } + + @Test + void findByIdWhenSessionDeletedWhileSavingDeltaAndSafeMapperThenSessionIsNull() { + this.context.register(RedisSessionMapperConfig.class); + refreshAndPrepareFields(); + RedisSession session = createAndSaveSession(Instant.now()); + session.setAttribute("new", "value"); + + HashOperations opsForHash = spy(this.spyOperations.opsForHash()); + given(this.spyOperations.opsForHash()).willReturn(opsForHash); + willAnswer((invocation) -> { + this.sessionRepository.deleteById(session.getId()); + return invocation.callRealMethod(); + }).given(opsForHash).putAll(any(), any()); + + this.sessionRepository.save(session); + assertThat(this.sessionRepository.findById(session.getId())).isNull(); + } + + @SuppressWarnings("unchecked") + private void refreshAndPrepareFields() { + this.context.refresh(); + this.sessionRepository = this.context.getBean(RedisSessionRepository.class); + RedisOperations redisOperations = (RedisOperations) ReflectionTestUtils + .getField(this.sessionRepository, "sessionRedisOperations"); + this.spyOperations = spy(redisOperations); + ReflectionTestUtils.setField(this.sessionRepository, "sessionRedisOperations", this.spyOperations); + } + + private RedisSession createAndSaveSession(Instant lastAccessedTime) { + RedisSession session = this.sessionRepository.createSession(); + session.setLastAccessedTime(lastAccessedTime); + session.setAttribute("attribute1", "value1"); + this.sessionRepository.save(session); + return this.sessionRepository.findById(session.getId()); + } + + @Configuration + @EnableRedisHttpSession + static class Config extends BaseConfig { + + } + + @Configuration + @EnableRedisHttpSession + static class RedisSessionMapperConfig extends BaseConfig { + + @Bean + SessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository + .setRedisSessionMapper(new SafeRedisSessionMapper(redisSessionRepository)); + } + + } + + static class SafeRedisSessionMapper implements BiFunction, MapSession> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final RedisSessionRepository sessionRepository; + + SafeRedisSessionMapper(RedisSessionRepository sessionRepository) { + this.sessionRepository = sessionRepository; + } + + @Override + public MapSession apply(String sessionId, Map map) { + try { + return this.delegate.apply(sessionId, map); + } + catch (IllegalStateException ex) { + this.sessionRepository.deleteById(sessionId); + return null; + } + } + + } + +} diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java index 84460599..a24db32d 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/ReactiveRedisSessionRepository.java @@ -21,6 +21,7 @@ import java.time.Instant; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.function.BiFunction; import org.reactivestreams.Publisher; import reactor.core.publisher.Mono; @@ -66,6 +67,8 @@ public class ReactiveRedisSessionRepository private SessionIdGenerator sessionIdGenerator = UuidSessionIdGenerator.getInstance(); + private BiFunction, Mono> redisSessionMapper = new RedisSessionMapperAdapter(); + /** * Create a new {@link ReactiveRedisSessionRepository} instance. * @param sessionRedisOperations the {@link ReactiveRedisOperations} to use for @@ -154,7 +157,7 @@ public class ReactiveRedisSessionRepository return this.sessionRedisOperations.opsForHash().entries(sessionKey) .collectMap((e) -> e.getKey().toString(), Map.Entry::getValue) .filter((map) -> !map.isEmpty()) - .map(new RedisSessionMapper(id)) + .flatMap((map) -> this.redisSessionMapper.apply(id, map)) .filter((session) -> !session.isExpired()) .map((session) -> new RedisSession(session, false)) .switchIfEmpty(Mono.defer(() -> deleteById(id).then(Mono.empty()))); @@ -186,6 +189,16 @@ public class ReactiveRedisSessionRepository this.sessionIdGenerator = sessionIdGenerator; } + /** + * Set the {@link BiFunction} used to convert a {@link Map} to a {@link MapSession}. + * @param redisSessionMapper the mapper to use, cannot be null + * @since 3.2 + */ + public void setRedisSessionMapper(BiFunction, Mono> redisSessionMapper) { + Assert.notNull(redisSessionMapper, "redisSessionMapper cannot be null"); + this.redisSessionMapper = redisSessionMapper; + } + /** * A custom implementation of {@link Session} that uses a {@link MapSession} as the * basis for its mapping. It keeps track of any attributes that have changed. When @@ -349,4 +362,16 @@ public class ReactiveRedisSessionRepository } + private static final class RedisSessionMapperAdapter + implements BiFunction, Mono> { + + private final RedisSessionMapper mapper = new RedisSessionMapper(); + + @Override + public Mono apply(String sessionId, Map map) { + return Mono.fromSupplier(() -> this.mapper.apply(sessionId, map)); + } + + } + } 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 70fe7982..9b08c482 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 @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.function.BiFunction; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -326,6 +327,8 @@ public class RedisIndexedSessionRepository private SessionIdGenerator sessionIdGenerator = UuidSessionIdGenerator.getInstance(); + private BiFunction, MapSession> redisSessionMapper = new RedisSessionMapper(); + /** * Creates a new instance. For an example, refer to the class level javadoc. * @param sessionRedisOperations the {@link RedisOperations} to use for managing the @@ -523,8 +526,8 @@ public class RedisIndexedSessionRepository if ((entries == null) || entries.isEmpty()) { return null; } - MapSession loaded = new RedisSessionMapper(id).apply(entries); - if (!allowExpired && loaded.isExpired()) { + MapSession loaded = this.redisSessionMapper.apply(id, entries); + if (loaded == null || (!allowExpired && loaded.isExpired())) { return null; } RedisSession result = new RedisSession(loaded, false); @@ -568,9 +571,11 @@ public class RedisIndexedSessionRepository String sessionId = channel.substring(channel.lastIndexOf(":") + 1); @SuppressWarnings("unchecked") Map entries = (Map) this.defaultSerializer.deserialize(message.getBody()); - MapSession loaded = new RedisSessionMapper(sessionId).apply(entries); - RedisSession session = new RedisSession(loaded, false); - handleCreated(session); + MapSession loaded = this.redisSessionMapper.apply(sessionId, entries); + if (loaded != null) { + RedisSession session = new RedisSession(loaded, false); + handleCreated(session); + } return; } @@ -730,6 +735,17 @@ public class RedisIndexedSessionRepository this.sessionIdGenerator = sessionIdGenerator; } + /** + * Set the {@link BiFunction} used to map {@link MapSession} to a + * {@link ReactiveRedisSessionRepository.RedisSession}. + * @param redisSessionMapper the mapper to use, cannot be null + * @since 3.2 + */ + public void setRedisSessionMapper(BiFunction, MapSession> redisSessionMapper) { + Assert.notNull(redisSessionMapper, "redisSessionMapper cannot be null"); + this.redisSessionMapper = redisSessionMapper; + } + /** * A custom implementation of {@link Session} that uses a {@link MapSession} as the * basis for its mapping. It keeps track of any attributes that have changed. When diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionMapper.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionMapper.java index 476d03d3..ab2f926c 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionMapper.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionMapper.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 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. @@ -19,6 +19,7 @@ package org.springframework.session.data.redis; import java.time.Duration; import java.time.Instant; import java.util.Map; +import java.util.function.BiFunction; import java.util.function.Function; import org.springframework.session.MapSession; @@ -30,9 +31,10 @@ import org.springframework.util.Assert; * {@link MapSession}. * * @author Vedran Pavic + * @author Marcus da Coregio * @since 2.2.0 */ -final class RedisSessionMapper implements Function, MapSession> { +public final class RedisSessionMapper implements BiFunction, MapSession> { /** * The key in the hash representing {@link Session#getCreationTime()}. @@ -56,17 +58,15 @@ final class RedisSessionMapper implements Function, MapSessi */ static final String ATTRIBUTE_PREFIX = "sessionAttr:"; - private final String sessionId; - - RedisSessionMapper(String sessionId) { - Assert.hasText(sessionId, "sessionId must not be empty"); - this.sessionId = sessionId; + private static void handleMissingKey(String key) { + throw new IllegalStateException(key + " key must not be null"); } @Override - public MapSession apply(Map map) { + public MapSession apply(String sessionId, Map map) { + Assert.hasText(sessionId, "sessionId must not be empty"); Assert.notEmpty(map, "map must not be empty"); - MapSession session = new MapSession(this.sessionId); + MapSession session = new MapSession(sessionId); Long creationTime = (Long) map.get(CREATION_TIME_KEY); if (creationTime == null) { handleMissingKey(CREATION_TIME_KEY); @@ -90,8 +90,4 @@ final class RedisSessionMapper implements Function, MapSessi return session; } - private static void handleMissingKey(String key) { - throw new IllegalStateException(key + " key must not be null"); - } - } diff --git a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java index a94629a2..e14c5cba 100644 --- a/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java +++ b/spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisSessionRepository.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2022 the original author or authors. + * Copyright 2014-2023 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. @@ -21,6 +21,7 @@ import java.time.Instant; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.function.BiFunction; import org.springframework.data.redis.core.RedisOperations; import org.springframework.session.FlushMode; @@ -60,6 +61,8 @@ public class RedisSessionRepository implements SessionRepository, MapSession> redisSessionMapper = new RedisSessionMapper(); + /** * Create a new {@link RedisSessionRepository} instance. * @param sessionRedisOperations the {@link RedisOperations} to use for managing @@ -136,8 +139,8 @@ public class RedisSessionRepository implements SessionRepository, MapSession> redisSessionMapper) { + Assert.notNull(redisSessionMapper, "redisSessionMapper cannot be null"); + this.redisSessionMapper = redisSessionMapper; + } + /** * An internal {@link Session} implementation used by this {@link SessionRepository}. */ diff --git a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionMapperTests.java b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionMapperTests.java index 3e345c59..2042bc16 100644 --- a/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionMapperTests.java +++ b/spring-session-data-redis/src/test/java/org/springframework/session/data/redis/RedisSessionMapperTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2019 the original author or authors. + * Copyright 2014-2023 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. @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.session.MapSession; @@ -38,34 +37,29 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; */ class RedisSessionMapperTests { - private RedisSessionMapper mapper; - - @BeforeEach - void setUp() { - this.mapper = new RedisSessionMapper("id"); - } + private RedisSessionMapper mapper = new RedisSessionMapper(); @Test - void constructor_NullId_ShouldThrowException() { - assertThatIllegalArgumentException().isThrownBy(() -> new RedisSessionMapper(null)) + void apply_NullId_ShouldThrowException() { + assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply(null, Collections.emptyMap())) .withMessage("sessionId must not be empty"); } @Test - void constructor_EmptyId_ShouldThrowException() { - assertThatIllegalArgumentException().isThrownBy(() -> new RedisSessionMapper(" ")) + void apply_EmptyId_ShouldThrowException() { + assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply(" ", Collections.emptyMap())) .withMessage("sessionId must not be empty"); } @Test void apply_NullMap_ShouldThrowException() { - assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply(null)) + assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply("1234", null)) .withMessage("map must not be empty"); } @Test void apply_EmptyMap_ShouldThrowException() { - assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply(Collections.emptyMap())) + assertThatIllegalArgumentException().isThrownBy(() -> this.mapper.apply("1234", Collections.emptyMap())) .withMessage("map must not be empty"); } @@ -74,7 +68,7 @@ class RedisSessionMapperTests { Map sessionMap = new HashMap<>(); sessionMap.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, 0L); sessionMap.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1800); - assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply(sessionMap)) + assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply("id", sessionMap)) .withMessage(RedisSessionMapper.CREATION_TIME_KEY + " key must not be null"); } @@ -83,7 +77,7 @@ class RedisSessionMapperTests { Map sessionMap = new HashMap<>(); sessionMap.put(RedisSessionMapper.CREATION_TIME_KEY, 0L); sessionMap.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1800); - assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply(sessionMap)) + assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply("id", sessionMap)) .withMessage(RedisSessionMapper.LAST_ACCESSED_TIME_KEY + " key must not be null"); } @@ -92,7 +86,7 @@ class RedisSessionMapperTests { Map sessionMap = new HashMap<>(); sessionMap.put(RedisSessionMapper.CREATION_TIME_KEY, 0L); sessionMap.put(RedisSessionMapper.LAST_ACCESSED_TIME_KEY, 0L); - assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply(sessionMap)) + assertThatIllegalStateException().isThrownBy(() -> this.mapper.apply("id", sessionMap)) .withMessage(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY + " key must not be null"); } @@ -104,7 +98,7 @@ class RedisSessionMapperTests { sessionMap.put(RedisSessionMapper.MAX_INACTIVE_INTERVAL_KEY, 1800); sessionMap.put(RedisSessionMapper.ATTRIBUTE_PREFIX + "existing", "value"); sessionMap.put(RedisSessionMapper.ATTRIBUTE_PREFIX + "missing", null); - MapSession session = this.mapper.apply(sessionMap); + MapSession session = this.mapper.apply("id", sessionMap); assertThat(session.getId()).isEqualTo("id"); assertThat(session.getCreationTime()).isEqualTo(Instant.ofEpochMilli(0)); assertThat(session.getLastAccessedTime()).isEqualTo(Instant.ofEpochMilli(0)); diff --git a/spring-session-docs/modules/ROOT/pages/configuration/redis.adoc b/spring-session-docs/modules/ROOT/pages/configuration/redis.adoc index c2474062..f29194d9 100644 --- a/spring-session-docs/modules/ROOT/pages/configuration/redis.adoc +++ b/spring-session-docs/modules/ROOT/pages/configuration/redis.adoc @@ -9,6 +9,7 @@ Now that you have your application configured, you might want to start customizi - I want to <>. - I want to <>. - I want to <> +- I want to <> [[serializing-session-using-json]] == Serializing the Session using JSON @@ -269,3 +270,138 @@ public void removeSession(Principal principal, String sessionIdToDelete) { ==== In the example above, you can use the `getSessions` method to find all sessions of a specific user, and the `removeSession` method to remove a specific session of a user. + +[[configuring-redis-session-mapper]] +== Configuring Redis Session Mapper + + +Spring Session Redis retrieves session information from Redis and stores it in a `Map`. +This map needs to undergo a mapping process to be transformed into a `MapSession` object, which is then utilized within `RedisSession`. + +The default mapper used for this purpose is called `RedisSessionMapper`. +If the session map doesn't contain the minimum necessary keys to construct the session, like `creationTime`, this mapper will throw an exception. +One possible scenario for the absence of required keys is when the session key is deleted concurrently, usually due to expiration, while the save process is in progress. +This occurs because the https://redis.io/commands/hset/[HSET command] is employed to set fields within the key, and if the key doesn't exist, this command will create it. + +If you want to customize the mapping process, you can create your implementation of `BiFunction, MapSession>` and set it into the session repository. +The following example shows how to delegate the mapping process to the default mapper, but if an exception is thrown, the session is deleted from Redis: + +[tabs] +====== +RedisSessionRepository:: ++ +[source,java,role="primary"] +---- +@Configuration +@EnableRedisHttpSession +public class SessionConfig { + + @Bean + SessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository + .setRedisSessionMapper(new SafeRedisSessionMapper(redisSessionRepository)); + } + + static class SafeRedisSessionMapper implements BiFunction, MapSession> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final RedisSessionRepository sessionRepository; + + SafeRedisSessionMapper(RedisSessionRepository sessionRepository) { + this.sessionRepository = sessionRepository; + } + + @Override + public MapSession apply(String sessionId, Map map) { + try { + return this.delegate.apply(sessionId, map); + } + catch (IllegalStateException ex) { + this.sessionRepository.deleteById(sessionId); + return null; + } + } + + } + +} +---- + +RedisIndexedSessionRepository:: ++ +[source,java,role="secondary"] +---- +@Configuration +@EnableRedisIndexedHttpSession +public class SessionConfig { + + @Bean + SessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository.setRedisSessionMapper( + new SafeRedisSessionMapper(redisSessionRepository.getSessionRedisOperations())); + } + + static class SafeRedisSessionMapper implements BiFunction, MapSession> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final RedisOperations redisOperations; + + SafeRedisSessionMapper(RedisOperations redisOperations) { + this.redisOperations = redisOperations; + } + + @Override + public MapSession apply(String sessionId, Map map) { + try { + return this.delegate.apply(sessionId, map); + } + catch (IllegalStateException ex) { + // if you use a different redis namespace, change the key accordingly + this.redisOperations.delete("spring:session:sessions:" + sessionId); // we do not invoke RedisIndexedSessionRepository#deleteById to avoid an infinite loop because the method also invokes this mapper + return null; + } + } + + } + +} +---- + +ReactiveRedisSessionRepository:: ++ +[source,java,role="tertiary"] +---- +@Configuration +@EnableRedisWebSession +public class SessionConfig { + + @Bean + ReactiveSessionRepositoryCustomizer redisSessionRepositoryCustomizer() { + return (redisSessionRepository) -> redisSessionRepository + .setRedisSessionMapper(new SafeRedisSessionMapper(redisSessionRepository)); + } + + static class SafeRedisSessionMapper implements BiFunction, Mono> { + + private final RedisSessionMapper delegate = new RedisSessionMapper(); + + private final ReactiveRedisSessionRepository sessionRepository; + + SafeRedisSessionMapper(ReactiveRedisSessionRepository sessionRepository) { + this.sessionRepository = sessionRepository; + } + + @Override + public Mono apply(String sessionId, Map map) { + return Mono.fromSupplier(() -> this.delegate.apply(sessionId, map)) + .onErrorResume(IllegalStateException.class, + (ex) -> this.sessionRepository.deleteById(sessionId).then(Mono.empty())); + } + + } + +} +---- +====== diff --git a/spring-session-docs/modules/ROOT/pages/whats-new.adoc b/spring-session-docs/modules/ROOT/pages/whats-new.adoc index a18e5da0..ea07f330 100644 --- a/spring-session-docs/modules/ROOT/pages/whats-new.adoc +++ b/spring-session-docs/modules/ROOT/pages/whats-new.adoc @@ -1,3 +1,4 @@ = What's New - xref:configuration/common.adoc#changing-how-session-ids-are-generated[docs] - https://github.com/spring-projects/spring-session/issues/11[gh-11] - Introduce `SessionIdGenerator` to allow custom session id generation +- xref:configuration/redis.adoc#configuring-redis-session-mapper[docs] - https://github.com/spring-projects/spring-session/issues/2021[gh-2021] - Allow safe deserialization of Redis sessions