Issue #411 MongoSessionManager does not save maxInactiveInterval and expiry correctly
Issue #415 Setting big session-timeout can overflow MAXINT
This commit is contained in:
parent
fa8b1c9220
commit
ead37b1b57
|
@ -73,17 +73,17 @@ import com.mongodb.WriteConcern;
|
|||
* { "_id" : ObjectId("52845534a40b66410f228f23"),
|
||||
* "accessed" : NumberLong("1384818548903"),
|
||||
* "maxIdle" : 1,
|
||||
* "created" : NumberLong("1384818548903"),
|
||||
* "expiry" : NumberLong("1384818549903"),
|
||||
* "id" : "w01ijx2vnalgv1sqrpjwuirprp7",
|
||||
* "valid" : true
|
||||
* "context" : { "::/contextA" : { "A" : "A",
|
||||
* "__metadata__" : { "version" : NumberLong(2) }
|
||||
* },
|
||||
* "::/contextB" : { "B" : "B",
|
||||
* "__metadata__" : { "version" : NumberLong(1) }
|
||||
* }
|
||||
* },
|
||||
* "created" : NumberLong("1384818548903"),
|
||||
* "expiry" : NumberLong("1384818549903"),
|
||||
* "id" : "w01ijx2vnalgv1sqrpjwuirprp7",
|
||||
* "valid" : true
|
||||
* }
|
||||
* }
|
||||
* </pre>
|
||||
* <p>
|
||||
|
@ -220,7 +220,8 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
{
|
||||
try
|
||||
{
|
||||
__log.debug("MongoSessionManager:save session {}", session.getClusterId());
|
||||
if (__log.isDebugEnabled())
|
||||
__log.debug("MongoSessionManager:save session {}", session.getClusterId());
|
||||
session.willPassivate();
|
||||
|
||||
// Form query for upsert
|
||||
|
@ -236,9 +237,8 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
// handle valid or invalid
|
||||
if (session.isValid())
|
||||
{
|
||||
long expiry = (session.getMaxInactiveInterval() > 0?(session.getAccessed()+(1000L*getMaxInactiveInterval())):0);
|
||||
__log.debug("MongoSessionManager: calculated expiry {} for session {}", expiry, session.getId());
|
||||
|
||||
long expiry = (session.getMaxInactiveInterval() > 0?(session.getAccessed()+(1000L*session.getMaxInactiveInterval())):0);
|
||||
|
||||
// handle new or existing
|
||||
if (version == null)
|
||||
{
|
||||
|
@ -249,11 +249,12 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
sets.put(__VALID,true);
|
||||
|
||||
sets.put(getContextAttributeKey(__VERSION),version);
|
||||
sets.put(__MAX_IDLE, getMaxInactiveInterval());
|
||||
sets.put(__EXPIRY, expiry);
|
||||
sets.put(__MAX_IDLE, session.getMaxInactiveInterval()); //seconds
|
||||
sets.put(__EXPIRY, expiry); //milliseconds
|
||||
}
|
||||
else
|
||||
{
|
||||
|
||||
version = new Long(((Number)version).longValue() + 1);
|
||||
update.put("$inc",_version_1);
|
||||
//if max idle time and/or expiry is smaller for this context, then choose that for the whole session doc
|
||||
|
@ -263,15 +264,22 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
DBObject o = _dbSessions.findOne(new BasicDBObject("id",session.getClusterId()), fields);
|
||||
if (o != null)
|
||||
{
|
||||
Integer currentMaxIdle = (Integer)o.get(__MAX_IDLE);
|
||||
Long currentExpiry = (Long)o.get(__EXPIRY);
|
||||
if (currentMaxIdle != null && getMaxInactiveInterval() > 0 && getMaxInactiveInterval() < currentMaxIdle)
|
||||
sets.put(__MAX_IDLE, getMaxInactiveInterval());
|
||||
if (currentExpiry != null && expiry > 0 && expiry != currentExpiry)
|
||||
Integer tmpInt = (Integer)o.get(__MAX_IDLE);
|
||||
int currentMaxIdle = (tmpInt == null? 0:tmpInt.intValue());
|
||||
Long tmpLong = (Long)o.get(__EXPIRY);
|
||||
long currentExpiry = (tmpLong == null? 0 : tmpLong.longValue());
|
||||
|
||||
|
||||
|
||||
if (currentMaxIdle != session.getMaxInactiveInterval())
|
||||
sets.put(__MAX_IDLE, session.getMaxInactiveInterval());
|
||||
|
||||
if (currentExpiry != expiry)
|
||||
sets.put(__EXPIRY, expiry);
|
||||
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
sets.put(__ACCESSED,session.getAccessed());
|
||||
Set<String> names = session.takeDirty();
|
||||
if (isSaveAllAttributes() || upsert)
|
||||
|
@ -371,6 +379,9 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
session.willPassivate();
|
||||
try
|
||||
{
|
||||
//replace the maxInactiveInterval with the saved value
|
||||
session.setMaxInactiveInterval((Integer)o.get(__MAX_IDLE));
|
||||
|
||||
DBObject attrs = (DBObject)getNestedValue(o,getContextKey());
|
||||
//if disk version now has no attributes, get rid of them
|
||||
if (attrs == null || attrs.keySet().size() == 0)
|
||||
|
@ -467,6 +478,7 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
Object version = o.get(getContextAttributeKey(__VERSION));
|
||||
Long created = (Long)o.get(__CREATED);
|
||||
Long accessed = (Long)o.get(__ACCESSED);
|
||||
Integer maxIdle = (Integer)o.get(__MAX_IDLE);
|
||||
|
||||
NoSqlSession session = null;
|
||||
|
||||
|
@ -480,6 +492,7 @@ public class MongoSessionManager extends NoSqlSessionManager
|
|||
__log.debug("MongoSessionManager: session {} present for context {}", clusterId, getContextKey());
|
||||
//only load a session if it exists for this context
|
||||
session = new NoSqlSession(this,created,accessed,clusterId,version);
|
||||
session.setMaxInactiveInterval(maxIdle); //setup the saved maxInactiveInterval for the session
|
||||
|
||||
for (String name : attrs.keySet())
|
||||
{
|
||||
|
|
|
@ -77,7 +77,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
|
|||
_requests=1;
|
||||
_maxIdleMs=_manager._dftMaxIdleSecs>0?_manager._dftMaxIdleSecs*1000L:-1;
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("new session & id "+_nodeId+" "+_clusterId);
|
||||
LOG.debug("New session & id "+_nodeId+" "+_clusterId);
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------- */
|
||||
|
@ -92,7 +92,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
|
|||
_requests=1;
|
||||
_maxIdleMs=_manager._dftMaxIdleSecs>0?_manager._dftMaxIdleSecs*1000L:-1;
|
||||
if (LOG.isDebugEnabled())
|
||||
LOG.debug("new session "+_nodeId+" "+_clusterId);
|
||||
LOG.debug("Restored session "+_nodeId+" "+_clusterId);
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------- */
|
||||
|
@ -557,6 +557,8 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
|
|||
@Override
|
||||
public void setMaxInactiveInterval(int secs)
|
||||
{
|
||||
if (secs < 0)
|
||||
LOG.warn("Session {} is now immortal (maxInactiveInterval={})", _clusterId, secs);
|
||||
_maxIdleMs=(long)secs*1000L;
|
||||
}
|
||||
|
||||
|
|
|
@ -75,6 +75,14 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen
|
|||
|
||||
/* ------------------------------------------------------------ */
|
||||
public final static int __distantFuture=60*60*24*7*52*20;
|
||||
|
||||
|
||||
/**
|
||||
* Web.xml session-timeout is set in minutes, but is stored as an int in seconds by HttpSession and
|
||||
* the sessionmanager. Thus MAX_INT is the max number of seconds that can be set, and MAX_INT/60 is the
|
||||
* max number of minutes that you can set.
|
||||
*/
|
||||
public final static java.math.BigDecimal MAX_INACTIVE_MINUTES = new java.math.BigDecimal(Integer.MAX_VALUE/60);
|
||||
|
||||
static final HttpSessionContext __nullSessionContext=new HttpSessionContext()
|
||||
{
|
||||
|
@ -619,6 +627,8 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen
|
|||
public void setMaxInactiveInterval(int seconds)
|
||||
{
|
||||
_dftMaxIdleSecs=seconds;
|
||||
if (_dftMaxIdleSecs < 0)
|
||||
__log.warn("Sessions created by this manager are immortal (default maxInactiveInterval={})"+_dftMaxIdleSecs);
|
||||
}
|
||||
|
||||
/* ------------------------------------------------------------ */
|
||||
|
|
|
@ -33,12 +33,12 @@ import java.util.Set;
|
|||
|
||||
import javax.servlet.DispatcherType;
|
||||
import javax.servlet.MultipartConfigElement;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.SessionTrackingMode;
|
||||
|
||||
import org.eclipse.jetty.security.ConstraintAware;
|
||||
import org.eclipse.jetty.security.ConstraintMapping;
|
||||
import org.eclipse.jetty.security.authentication.FormAuthenticator;
|
||||
import org.eclipse.jetty.server.session.AbstractSessionManager;
|
||||
import org.eclipse.jetty.servlet.BaseHolder.Source;
|
||||
import org.eclipse.jetty.servlet.ErrorPageErrorHandler;
|
||||
import org.eclipse.jetty.servlet.FilterHolder;
|
||||
|
@ -51,7 +51,6 @@ import org.eclipse.jetty.servlet.ServletHolder;
|
|||
import org.eclipse.jetty.servlet.ServletMapping;
|
||||
import org.eclipse.jetty.util.ArrayUtil;
|
||||
import org.eclipse.jetty.util.Loader;
|
||||
import org.eclipse.jetty.util.StringUtil;
|
||||
import org.eclipse.jetty.util.log.Log;
|
||||
import org.eclipse.jetty.util.log.Logger;
|
||||
import org.eclipse.jetty.util.security.Constraint;
|
||||
|
@ -650,8 +649,11 @@ public class StandardDescriptorProcessor extends IterativeDescriptorProcessor
|
|||
XmlParser.Node tNode = node.get("session-timeout");
|
||||
if (tNode != null)
|
||||
{
|
||||
int timeout = Integer.parseInt(tNode.toString(false, true));
|
||||
context.getSessionHandler().getSessionManager().setMaxInactiveInterval(timeout * 60);
|
||||
java.math.BigDecimal asDecimal = new java.math.BigDecimal(tNode.toString(false, true));
|
||||
if (asDecimal.compareTo(AbstractSessionManager.MAX_INACTIVE_MINUTES) > 0)
|
||||
throw new IllegalStateException ("Max session-timeout in minutes is "+AbstractSessionManager.MAX_INACTIVE_MINUTES);
|
||||
|
||||
context.getSessionHandler().getSessionManager().setMaxInactiveInterval(asDecimal.intValueExact() * 60);
|
||||
}
|
||||
|
||||
//Servlet Spec 3.0
|
||||
|
|
|
@ -113,7 +113,7 @@ public class MongoTestServer extends AbstractTestServer
|
|||
throw new RuntimeException(e);
|
||||
}
|
||||
|
||||
manager.setSavePeriod(1);
|
||||
manager.setSavePeriod(0);
|
||||
manager.setStalePeriod(0);
|
||||
manager.setSaveAllAttributes(_saveAllAttributes);
|
||||
//manager.setScavengePeriod((int)TimeUnit.SECONDS.toMillis(_scavengePeriod));
|
||||
|
|
|
@ -18,10 +18,33 @@
|
|||
|
||||
package org.eclipse.jetty.nosql.mongodb;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import javax.servlet.http.HttpSession;
|
||||
|
||||
import org.eclipse.jetty.client.HttpClient;
|
||||
import org.eclipse.jetty.client.api.ContentResponse;
|
||||
import org.eclipse.jetty.client.api.Request;
|
||||
import org.eclipse.jetty.server.session.AbstractSessionExpiryTest;
|
||||
import org.eclipse.jetty.server.session.AbstractTestServer;
|
||||
import org.eclipse.jetty.servlet.ServletContextHandler;
|
||||
import org.eclipse.jetty.servlet.ServletHolder;
|
||||
import org.eclipse.jetty.util.StringUtil;
|
||||
import org.junit.Test;
|
||||
|
||||
import com.mongodb.BasicDBObject;
|
||||
import com.mongodb.DBCollection;
|
||||
import com.mongodb.DBObject;
|
||||
|
||||
|
||||
public class SessionExpiryTest extends AbstractSessionExpiryTest
|
||||
{
|
||||
|
||||
|
@ -42,4 +65,208 @@ public class SessionExpiryTest extends AbstractSessionExpiryTest
|
|||
{
|
||||
super.testSessionExpiry();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testBigSessionExpiry() throws Exception
|
||||
{
|
||||
String contextPath = "";
|
||||
String servletMapping = "/server";
|
||||
int inactivePeriod = Integer.MAX_VALUE * 60; //integer overflow
|
||||
int scavengePeriod = 1;
|
||||
AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod);
|
||||
ChangeTimeoutServlet servlet = new ChangeTimeoutServlet();
|
||||
ServletHolder holder = new ServletHolder(servlet);
|
||||
ServletContextHandler context = server1.addContext(contextPath);
|
||||
context.addServlet(holder, servletMapping);
|
||||
TestHttpSessionListener listener = new TestHttpSessionListener();
|
||||
|
||||
context.getSessionHandler().addEventListener(listener);
|
||||
|
||||
server1.start();
|
||||
int port1 = server1.getPort();
|
||||
|
||||
try
|
||||
{
|
||||
HttpClient client = new HttpClient();
|
||||
client.start();
|
||||
String url = "http://localhost:" + port1 + contextPath + servletMapping;
|
||||
|
||||
//make a request to set up a session on the server
|
||||
ContentResponse response1 = client.GET(url + "?action=init");
|
||||
assertEquals(HttpServletResponse.SC_OK,response1.getStatus());
|
||||
String sessionCookie = response1.getHeaders().get("Set-Cookie");
|
||||
assertTrue(sessionCookie != null);
|
||||
// Mangle the cookie, replacing Path with $Path, etc.
|
||||
sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
|
||||
|
||||
String sessionId = AbstractTestServer.extractSessionId(sessionCookie);
|
||||
|
||||
DBCollection sessions = ((MongoSessionIdManager)((MongoTestServer)server1).getServer().getSessionIdManager()).getSessions();
|
||||
verifySessionCreated(listener,sessionId);
|
||||
//verify that the session timeout is set in mongo
|
||||
verifySessionTimeout(sessions, sessionId, inactivePeriod);
|
||||
|
||||
//get the session expiry time from mongo
|
||||
long expiry = getSessionExpiry(sessions, sessionId);
|
||||
assertEquals(0, expiry);
|
||||
|
||||
}
|
||||
finally
|
||||
{
|
||||
server1.stop();
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void changeSessionTimeout() throws Exception
|
||||
{
|
||||
String contextPath = "";
|
||||
String servletMapping = "/server";
|
||||
int inactivePeriod = 10;
|
||||
int scavengePeriod = 1;
|
||||
AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod);
|
||||
ChangeTimeoutServlet servlet = new ChangeTimeoutServlet();
|
||||
ServletHolder holder = new ServletHolder(servlet);
|
||||
ServletContextHandler context = server1.addContext(contextPath);
|
||||
context.addServlet(holder, servletMapping);
|
||||
TestHttpSessionListener listener = new TestHttpSessionListener();
|
||||
|
||||
context.getSessionHandler().addEventListener(listener);
|
||||
|
||||
server1.start();
|
||||
int port1 = server1.getPort();
|
||||
|
||||
try
|
||||
{
|
||||
HttpClient client = new HttpClient();
|
||||
client.start();
|
||||
String url = "http://localhost:" + port1 + contextPath + servletMapping;
|
||||
|
||||
//make a request to set up a session on the server
|
||||
ContentResponse response1 = client.GET(url + "?action=init");
|
||||
assertEquals(HttpServletResponse.SC_OK,response1.getStatus());
|
||||
String sessionCookie = response1.getHeaders().get("Set-Cookie");
|
||||
assertTrue(sessionCookie != null);
|
||||
// Mangle the cookie, replacing Path with $Path, etc.
|
||||
sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
|
||||
|
||||
String sessionId = AbstractTestServer.extractSessionId(sessionCookie);
|
||||
|
||||
DBCollection sessions = ((MongoSessionIdManager)((MongoTestServer)server1).getServer().getSessionIdManager()).getSessions();
|
||||
verifySessionCreated(listener,sessionId);
|
||||
//verify that the session timeout is set in mongo
|
||||
verifySessionTimeout(sessions, sessionId, inactivePeriod);
|
||||
|
||||
//get the session expiry time from mongo
|
||||
long expiry = getSessionExpiry(sessions, sessionId);
|
||||
|
||||
//make another request to change the session timeout to a smaller value
|
||||
inactivePeriod = 5;
|
||||
Request request = client.newRequest(url + "?action=change&val="+inactivePeriod);
|
||||
request.getHeaders().add("Cookie", sessionCookie);
|
||||
ContentResponse response2 = request.send();
|
||||
assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
|
||||
|
||||
|
||||
//check the timeout in mongo
|
||||
verifySessionTimeout(sessions, sessionId, inactivePeriod);
|
||||
//check the session expiry time has decreased from previous value
|
||||
assertTrue(getSessionExpiry(sessions, sessionId)<expiry);
|
||||
expiry = getSessionExpiry(sessions, sessionId);
|
||||
|
||||
//increase the session timeout
|
||||
inactivePeriod = 20;
|
||||
request = client.newRequest(url + "?action=change&val="+inactivePeriod);
|
||||
request.getHeaders().add("Cookie", sessionCookie);
|
||||
response2 = request.send();
|
||||
assertEquals(HttpServletResponse.SC_OK,response2.getStatus());
|
||||
//verify that the session timeout is set in mongo
|
||||
verifySessionTimeout(sessions, sessionId, inactivePeriod);
|
||||
assertTrue(getSessionAccessed(sessions, sessionId)+ (1000L*inactivePeriod) == getSessionExpiry(sessions, sessionId));
|
||||
}
|
||||
finally
|
||||
{
|
||||
server1.stop();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
public void verifySessionTimeout (DBCollection sessions, String id, int sec) throws Exception
|
||||
{
|
||||
assertNotNull(sessions);
|
||||
assertNotNull(id);
|
||||
|
||||
DBObject o = sessions.findOne(new BasicDBObject(MongoSessionManager.__ID,id));
|
||||
assertNotNull(o);
|
||||
Integer maxIdle = (Integer)o.get(MongoSessionManager.__MAX_IDLE);
|
||||
assertNotNull(maxIdle);
|
||||
assertEquals(sec, maxIdle.intValue());
|
||||
}
|
||||
|
||||
public long getSessionExpiry (DBCollection sessions, String id) throws Exception
|
||||
{
|
||||
assertNotNull(sessions);
|
||||
assertNotNull(id);
|
||||
|
||||
DBObject o = sessions.findOne(new BasicDBObject(MongoSessionManager.__ID,id));
|
||||
assertNotNull(o);
|
||||
Long expiry = (Long)o.get(MongoSessionManager.__EXPIRY);
|
||||
return (expiry == null? null : expiry.longValue());
|
||||
}
|
||||
|
||||
public int getSessionMaxInactiveInterval (DBCollection sessions, String id) throws Exception
|
||||
{
|
||||
assertNotNull(sessions);
|
||||
assertNotNull(id);
|
||||
|
||||
DBObject o = sessions.findOne(new BasicDBObject(MongoSessionManager.__ID,id));
|
||||
assertNotNull(o);
|
||||
Integer inactiveInterval = (Integer)o.get(MongoSessionManager.__MAX_IDLE);
|
||||
return (inactiveInterval == null? null : inactiveInterval.intValue());
|
||||
}
|
||||
|
||||
public long getSessionAccessed (DBCollection sessions, String id) throws Exception
|
||||
{
|
||||
assertNotNull(sessions);
|
||||
assertNotNull(id);
|
||||
|
||||
DBObject o = sessions.findOne(new BasicDBObject(MongoSessionManager.__ID,id));
|
||||
assertNotNull(o);
|
||||
Long accessed = (Long)o.get(MongoSessionManager.__ACCESSED);
|
||||
return (accessed == null? null : accessed.longValue());
|
||||
}
|
||||
|
||||
public void debugPrint (DBCollection sessions, String id) throws Exception
|
||||
{
|
||||
assertNotNull(sessions);
|
||||
assertNotNull(id);
|
||||
|
||||
DBObject o = sessions.findOne(new BasicDBObject(MongoSessionManager.__ID,id));
|
||||
assertNotNull(o);
|
||||
System.err.println(o);
|
||||
}
|
||||
|
||||
public static class ChangeTimeoutServlet extends HttpServlet
|
||||
{
|
||||
|
||||
@Override
|
||||
protected void doGet(HttpServletRequest request, HttpServletResponse httpServletResponse) throws ServletException, IOException
|
||||
{
|
||||
String action = request.getParameter("action");
|
||||
if ("init".equals(action))
|
||||
{
|
||||
HttpSession session = request.getSession(true);
|
||||
session.setAttribute("test", "test");
|
||||
}
|
||||
else if ("change".equals(action))
|
||||
{
|
||||
String tmp = request.getParameter("val");
|
||||
int val = (StringUtil.isBlank(tmp)?0:Integer.valueOf(tmp.trim()));
|
||||
HttpSession session = request.getSession(false);
|
||||
assertNotNull(session);
|
||||
session.setMaxInactiveInterval(val);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue