diff --git a/spring-web/src/main/java/org/springframework/web/server/WebSession.java b/spring-web/src/main/java/org/springframework/web/server/WebSession.java index 6fe7e1cc2a..b35d0b82b1 100644 --- a/spring-web/src/main/java/org/springframework/web/server/WebSession.java +++ b/spring-web/src/main/java/org/springframework/web/server/WebSession.java @@ -116,13 +116,19 @@ public interface WebSession { Mono invalidate(); /** - * Save the session persisting attributes (e.g. if stored remotely) and also - * sending the session id to the client if the session is new. - *

Note that a session must be started explicitly via {@link #start()} or - * implicitly by adding attributes or otherwise this method has no effect. + * Save the session through the {@code WebSessionStore} as follows: + *

+ *

Note that this method is not intended for direct use by applications. + * Instead it is automatically invoked just before the response is + * committed is committed. * @return {@code Mono} to indicate completion with success or error - *

Typically this method should be automatically invoked just before the - * response is committed so applications don't have to by default. */ Mono save(); diff --git a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java index 130d140fb1..3ba0bcde2a 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java @@ -240,19 +240,37 @@ public class InMemoryWebSessionStore implements WebSessionStore { @Override public Mono save() { - if (sessions.size() >= maxSessions) { - expiredSessionChecker.removeExpiredSessions(clock.instant()); - if (sessions.size() >= maxSessions) { - return Mono.error(new IllegalStateException("Max sessions limit reached: " + sessions.size())); - } - } + + checkMaxSessionsLimit(); + + // Implicitly started session.. if (!getAttributes().isEmpty()) { this.state.compareAndSet(State.NEW, State.STARTED); } - InMemoryWebSessionStore.this.sessions.put(this.getId(), this); + + if (isStarted()) { + // Save + InMemoryWebSessionStore.this.sessions.put(this.getId(), this); + + // Unless it was invalidated + if (this.state.get().equals(State.EXPIRED)) { + InMemoryWebSessionStore.this.sessions.remove(this.getId()); + return Mono.error(new IllegalStateException("Session was invalidated")); + } + } + return Mono.empty(); } + private void checkMaxSessionsLimit() { + if (sessions.size() >= maxSessions) { + expiredSessionChecker.removeExpiredSessions(clock.instant()); + if (sessions.size() >= maxSessions) { + throw new IllegalStateException("Max sessions limit reached: " + sessions.size()); + } + } + } + @Override public boolean isExpired() { return isExpired(clock.instant()); diff --git a/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java index ee242cbbc4..080592aa5c 100644 --- a/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/session/InMemoryWebSessionStoreTests.java @@ -83,6 +83,7 @@ public class InMemoryWebSessionStoreTests { assertNotNull(session1); String id = session1.getId(); Instant time1 = session1.getLastAccessTime(); + session1.start(); session1.save().block(); // Fast-forward a few seconds @@ -95,6 +96,34 @@ public class InMemoryWebSessionStoreTests { assertTrue(time1.isBefore(time2)); } + @Test // SPR-17051 + public void sessionInvalidatedBeforeSave() { + // Request 1 creates session + WebSession session1 = this.store.createWebSession().block(); + assertNotNull(session1); + String id = session1.getId(); + session1.start(); + session1.save().block(); + + // Request 2 retrieves session + WebSession session2 = this.store.retrieveSession(id).block(); + assertNotNull(session2); + assertSame(session1, session2); + + // Request 3 retrieves and invalidates + WebSession session3 = this.store.retrieveSession(id).block(); + assertNotNull(session3); + assertSame(session1, session3); + session3.invalidate().block(); + + // Request 2 saves session after invalidated + session2.save().block(); + + // Session should not be present + WebSession session4 = this.store.retrieveSession(id).block(); + assertNull(session4); + } + @Test public void expirationCheckPeriod() {