Skip to content

Commit

Permalink
Avoid to retain SessionIdGenerator in session object
Browse files Browse the repository at this point in the history
session id generation shouldn't be the responsibility of session object, since SessionIdGenerator held by session object is only used by changeSessionId(), we could introduce overload method passing it as parameter to avoid that.
  • Loading branch information
quaff committed Aug 18, 2023
1 parent 0ed5d34 commit f49b763
Show file tree
Hide file tree
Showing 28 changed files with 265 additions and 166 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
*
* @author Rob Winch
* @author Vedran Pavic
* @author Yanming Zhou
* @since 1.0
*/
public final class MapSession implements Session, Serializable {
Expand Down Expand Up @@ -73,8 +74,6 @@ public final class MapSession implements Session, Serializable {
*/
private Duration maxInactiveInterval = DEFAULT_MAX_INACTIVE_INTERVAL;

private transient SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT;

/**
* Creates a new instance with identifier generated by
* {@link SessionIdGenerator#DEFAULT}.
Expand All @@ -83,17 +82,6 @@ public MapSession() {
this(SessionIdGenerator.DEFAULT.generate());
}

/**
* Creates a new instance using the specified {@link SessionIdGenerator} to generate
* the session id.
* @param sessionIdGenerator the {@link SessionIdGenerator} to use.
* @since 3.2
*/
public MapSession(SessionIdGenerator sessionIdGenerator) {
this(sessionIdGenerator.generate());
this.sessionIdGenerator = sessionIdGenerator;
}

/**
* Creates a new instance with the specified id. This is preferred to the default
* constructor when the id is known to prevent unnecessary consumption on entropy
Expand Down Expand Up @@ -146,17 +134,17 @@ public String getId() {
/**
* Get the original session id.
* @return the original session id
* @see #changeSessionId()
* @see #changeSessionId(SessionIdGenerator)
*/
public String getOriginalId() {
return this.originalId;
}

@Override
public String changeSessionId() {
String changedId = this.sessionIdGenerator.generate();
setId(changedId);
return changedId;
public String changeSessionId(SessionIdGenerator sessionIdGenerator) {
String newSessionId = sessionIdGenerator.regenerate(this);
setId(newSessionId);
return newSessionId;
}

@Override
Expand Down Expand Up @@ -241,15 +229,6 @@ public int hashCode() {
return this.id.hashCode();
}

/**
* Sets the {@link SessionIdGenerator} to use when generating a new session id.
* @param sessionIdGenerator the {@link SessionIdGenerator} to use.
* @since 3.2
*/
public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
this.sessionIdGenerator = sessionIdGenerator;
}

private static final long serialVersionUID = 7160779239673823561L;

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
* </p>
*
* @author Rob Winch
* @author Yanming Zhou
* @since 1.0
*/
public class MapSessionRepository implements SessionRepository<MapSession> {
Expand Down Expand Up @@ -73,9 +74,7 @@ public void save(MapSession session) {
if (!session.getId().equals(session.getOriginalId())) {
this.sessions.remove(session.getOriginalId());
}
MapSession saved = new MapSession(session);
saved.setSessionIdGenerator(this.sessionIdGenerator);
this.sessions.put(session.getId(), saved);
this.sessions.put(session.getId(), new MapSession(session));
}

@Override
Expand All @@ -88,9 +87,7 @@ public MapSession findById(String id) {
deleteById(saved.getId());
return null;
}
MapSession result = new MapSession(saved);
result.setSessionIdGenerator(this.sessionIdGenerator);
return result;
return new MapSession(saved);
}

@Override
Expand All @@ -100,7 +97,7 @@ public void deleteById(String id) {

@Override
public MapSession createSession() {
MapSession result = new MapSession(this.sessionIdGenerator);
MapSession result = new MapSession(this.sessionIdGenerator.generate());
result.setMaxInactiveInterval(this.defaultMaxInactiveInterval);
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public Mono<MapSession> findById(String id) {
return Mono.defer(() -> Mono.justOrEmpty(this.sessions.get(id))
.filter((session) -> !session.isExpired())
.map(MapSession::new)
.doOnNext((session) -> session.setSessionIdGenerator(this.sessionIdGenerator))
.switchIfEmpty(deleteById(id).then(Mono.empty())));
// @formatter:on
}
Expand All @@ -107,7 +106,6 @@ public Mono<MapSession> createSession() {
.map((sessionId) -> {
MapSession result = new MapSession(sessionId);
result.setMaxInactiveInterval(this.defaultMaxInactiveInterval);
result.setSessionIdGenerator(this.sessionIdGenerator);
return result;
});
// @formatter:on
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -26,6 +26,7 @@
*
* @author Rob Winch
* @author Vedran Pavic
* @author Yanming Zhou
* @since 1.0
*/
public interface Session {
Expand All @@ -37,11 +38,23 @@ public interface Session {
String getId();

/**
* Changes the session id. After invoking the {@link #getId()} will return a new
* identifier.
* Changes the session id with the {@link SessionIdGenerator#DEFAULT}. After invoking
* the {@link #getId()} will return a new identifier.
* @return the new session id which {@link #getId()} will now return
* @deprecated use {@link #changeSessionId(SessionIdGenerator)} instead
*/
String changeSessionId();
@Deprecated
default String changeSessionId() {
return changeSessionId(SessionIdGenerator.DEFAULT);
}

/**
* Changes the session id with the supplied {@link SessionIdGenerator}. After invoking
* the {@link #getId()} will return a new identifier.
* @param sessionIdGenerator the {@link SessionIdGenerator} to use
* @return the new session id which {@link #getId()} will now return
*/
String changeSessionId(SessionIdGenerator sessionIdGenerator);

/**
* Gets the Object associated with the specified name or null if no Object is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* An interface for specifying a strategy for generating session identifiers.
*
* @author Marcus da Coregio
* @author Yanming Zhou
* @since 3.2
*/
public interface SessionIdGenerator {
Expand All @@ -31,7 +32,22 @@ public interface SessionIdGenerator {
*/
SessionIdGenerator DEFAULT = UuidSessionIdGenerator.getInstance();

/**
* Generate identifier for creating new session.
* @return the generated session identifier
*/
@NonNull
String generate();

/**
* Generate identifier for changing id of existing session.
* @param session the existing {@link Session} object
* @return the generated session identifier
* @see Session#changeSessionId(SessionIdGenerator)
*/
@NonNull
default String regenerate(Session session) {
return generate();
}

}
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -34,6 +34,7 @@
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.session.Session;
import org.springframework.session.SessionIdGenerator;
import org.springframework.session.SessionRepository;
import org.springframework.session.events.SessionCreatedEvent;
import org.springframework.session.events.SessionDestroyedEvent;
Expand Down Expand Up @@ -87,6 +88,7 @@
*
* @author Rob Winch
* @author Vedran Pavic
* @author Yanming Zhou
* @since 1.1
* @see EnableSpringHttpSession
*/
Expand All @@ -107,6 +109,8 @@ public class SpringHttpSessionConfiguration implements InitializingBean, Applica

private List<HttpSessionListener> httpSessionListeners = new ArrayList<>();

private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT;

@Override
public void afterPropertiesSet() {
CookieSerializer cookieSerializer = (this.cookieSerializer != null) ? this.cookieSerializer
Expand All @@ -124,6 +128,7 @@ public <S extends Session> SessionRepositoryFilter<? extends Session> springSess
SessionRepository<S> sessionRepository) {
SessionRepositoryFilter<S> sessionRepositoryFilter = new SessionRepositoryFilter<>(sessionRepository);
sessionRepositoryFilter.setHttpSessionIdResolver(this.httpSessionIdResolver);
sessionRepositoryFilter.setSessionIdGenerator(this.sessionIdGenerator);
return sessionRepositoryFilter;
}

Expand Down Expand Up @@ -155,6 +160,11 @@ public void setHttpSessionListeners(List<HttpSessionListener> listeners) {
this.httpSessionListeners = listeners;
}

@Autowired(required = false)
public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
this.sessionIdGenerator = sessionIdGenerator;
}

private CookieSerializer createDefaultCookieSerializer() {
DefaultCookieSerializer cookieSerializer = new DefaultCookieSerializer();
if (this.servletContext != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -21,6 +21,7 @@
import org.springframework.context.annotation.Configuration;
import org.springframework.session.ReactiveSessionRepository;
import org.springframework.session.Session;
import org.springframework.session.SessionIdGenerator;
import org.springframework.session.web.server.session.SpringSessionWebSessionStore;
import org.springframework.web.server.adapter.WebHttpHandlerBuilder;
import org.springframework.web.server.session.DefaultWebSessionManager;
Expand All @@ -33,6 +34,7 @@
*
* @author Greg Turnquist
* @author Rob Winch
* @author Yanming Zhou
* @since 2.0
* @see EnableSpringWebSession
*/
Expand All @@ -41,11 +43,18 @@ public class SpringWebSessionConfiguration {

private WebSessionIdResolver webSessionIdResolver;

private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT;

@Autowired(required = false)
public void setWebSessionIdResolver(WebSessionIdResolver webSessionIdResolver) {
this.webSessionIdResolver = webSessionIdResolver;
}

@Autowired(required = false)
public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
this.sessionIdGenerator = sessionIdGenerator;
}

/**
* Configure a {@link WebSessionManager} using a provided
* {@link ReactiveSessionRepository}.
Expand All @@ -55,7 +64,8 @@ public void setWebSessionIdResolver(WebSessionIdResolver webSessionIdResolver) {
*/
@Bean(WebHttpHandlerBuilder.WEB_SESSION_MANAGER_BEAN_NAME)
public WebSessionManager webSessionManager(ReactiveSessionRepository<? extends Session> repository) {
SpringSessionWebSessionStore<? extends Session> sessionStore = new SpringSessionWebSessionStore<>(repository);
SpringSessionWebSessionStore<? extends Session> sessionStore = new SpringSessionWebSessionStore<>(repository,
SpringWebSessionConfiguration.this.sessionIdGenerator);
DefaultWebSessionManager manager = new DefaultWebSessionManager();
manager.setSessionStore(sessionStore);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import org.springframework.core.annotation.Order;
import org.springframework.session.Session;
import org.springframework.session.SessionIdGenerator;
import org.springframework.session.SessionRepository;

/**
Expand Down Expand Up @@ -106,6 +107,8 @@ public class SessionRepositoryFilter<S extends Session> extends OncePerRequestFi

private HttpSessionIdResolver httpSessionIdResolver = new CookieHttpSessionIdResolver();

private SessionIdGenerator sessionIdGenerator = SessionIdGenerator.DEFAULT;

/**
* Creates a new instance.
* @param sessionRepository the <code>SessionRepository</code> to use. Cannot be null.
Expand All @@ -130,6 +133,18 @@ public void setHttpSessionIdResolver(HttpSessionIdResolver httpSessionIdResolver
this.httpSessionIdResolver = httpSessionIdResolver;
}

/**
* Sets the {@link SessionIdGenerator} to be used. The default is a
* {@link SessionIdGenerator#DEFAULT}.
* @param sessionIdGenerator the {@link SessionIdGenerator} to use. Cannot be null.
*/
public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
if (sessionIdGenerator == null) {
throw new IllegalArgumentException("sessionIdGenerator cannot be null");
}
this.sessionIdGenerator = sessionIdGenerator;
}

@Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain)
throws ServletException, IOException {
Expand Down Expand Up @@ -259,7 +274,7 @@ public String changeSessionId() {
"Cannot change session ID. There is no session associated with this request.");
}

return getCurrentSession().getSession().changeSessionId();
return getCurrentSession().getSession().changeSessionId(SessionRepositoryFilter.this.sessionIdGenerator);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -35,6 +35,7 @@
import org.springframework.lang.Nullable;
import org.springframework.session.ReactiveSessionRepository;
import org.springframework.session.Session;
import org.springframework.session.SessionIdGenerator;
import org.springframework.util.Assert;
import org.springframework.web.server.WebSession;
import org.springframework.web.server.session.WebSessionStore;
Expand All @@ -47,17 +48,23 @@
* @param <S> the {@link Session} type
* @author Rob Winch
* @author Vedran Pavic
* @author Yanming Zhou
* @since 2.0
*/
public class SpringSessionWebSessionStore<S extends Session> implements WebSessionStore {

private final ReactiveSessionRepository<S> sessions;

private final SessionIdGenerator sessionIdGenerator;

private Clock clock = Clock.system(ZoneOffset.UTC);

public SpringSessionWebSessionStore(ReactiveSessionRepository<S> reactiveSessionRepository) {
public SpringSessionWebSessionStore(ReactiveSessionRepository<S> reactiveSessionRepository,
SessionIdGenerator sessionIdGenerator) {
Assert.notNull(reactiveSessionRepository, "reactiveSessionRepository cannot be null");
Assert.notNull(sessionIdGenerator, "sessionIdGenerator cannot be null");
this.sessions = reactiveSessionRepository;
this.sessionIdGenerator = sessionIdGenerator;
}

/**
Expand Down Expand Up @@ -134,7 +141,7 @@ public String getId() {
@Override
public Mono<Void> changeSessionId() {
return Mono.defer(() -> {
this.session.changeSessionId();
this.session.changeSessionId(SpringSessionWebSessionStore.this.sessionIdGenerator);
return save();
});
}
Expand Down
Loading

0 comments on commit f49b763

Please sign in to comment.