spring-projects / spring-session

Spring Session
https://spring.io/projects/spring-session
Apache License 2.0
1.86k stars 1.12k forks source link

Session API doesn't clarify behavior for maxInactiveInterval of 0 #1192

Open vpavic opened 6 years ago

vpavic commented 6 years ago

See the following snippets from our Session API:

https://github.com/spring-projects/spring-session/blob/5d0775b802514da469e43fa00d9cdd58f238b30e/spring-session-core/src/main/java/org/springframework/session/Session.java#L135-L142

https://github.com/spring-projects/spring-session/blob/5d0775b802514da469e43fa00d9cdd58f238b30e/spring-session-core/src/main/java/org/springframework/session/Session.java#L144-L151

So the API defines negative value as never timeout and says nothing on 0 value. In practice, I believe we treat 0 as expire now but need to verify this for all SessionRepository implementations.

OTOH the Servlet API's HttpSession says the following:

/**
 * Specifies the time, in seconds, between client requests before the 
 * servlet container will invalidate this session. 
 *
 * <p>An <tt>interval</tt> value of zero or less indicates that the
 * session should never timeout.
 *
 * @param interval      An integer specifying the number
 *              of seconds 
 */    
public void setMaxInactiveInterval(int interval);
/**
 * Returns the maximum time interval, in seconds, that 
 * the servlet container will keep this session open between 
 * client accesses. After this interval, the servlet container
 * will invalidate the session.  The maximum time interval can be set
 * with the <code>setMaxInactiveInterval</code> method.
 *
 * <p>A return value of zero or less indicates that the
 * session will never timeout.
 *
 * @return      an integer specifying the number of
 *          seconds this session remains open
 *          between client requests
 *
 * @see     #setMaxInactiveInterval
 */
public int getMaxInactiveInterval();

With that in mind, the issue is that HttpSessionAdapter effectively doesn't respect the Servlet API by doing this:

https://github.com/spring-projects/spring-session/blob/5d0775b802514da469e43fa00d9cdd58f238b30e/spring-session-core/src/main/java/org/springframework/session/web/http/HttpSessionAdapter.java#L98-L106

Another API to consider here is WebSession, which also doesn't clarify behavior for 0 value.

/cc @rwinch

oliviarla commented 2 days ago

@vpavic I think there's mismatch in the comments like below.

In the annotation definition, negative is not allowed to the maxInactiveInterval. https://github.com/spring-projects/spring-session/blob/fb176f0b1791517a4c73a04256e2e514d732fefe/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/EnableJdbcHttpSession.java#L81-L86

But In the repository, negative maxInactiveInterval indicates that session will never be expired. https://github.com/spring-projects/spring-session/blob/fb176f0b1791517a4c73a04256e2e514d732fefe/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/JdbcIndexedSessionRepository.java#L386-L395

So if I set maxInactiveInterval using annotation, it will never be expired unlike comments in the annotation. https://github.com/spring-projects/spring-session/blob/fb176f0b1791517a4c73a04256e2e514d732fefe/spring-session-jdbc/src/main/java/org/springframework/session/jdbc/config/annotation/web/http/JdbcHttpSessionConfiguration.java#L288