From 71f4d444a558466e3edd40e5350fb88122e1ca14 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 7 Sep 2010 23:12:21 +0000 Subject: [PATCH] 324601 Check session expiry on access git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@2254 7e9141cc-0065-0410-87d8-b60c137991c4 --- .../session/AbstractSessionManager.java | 22 +++++++----- .../server/session/HashSessionManager.java | 10 +++--- .../server/session/JDBCSessionManager.java | 34 ++++++++++++++----- .../eclipse/jetty/server/ResponseTest.java | 2 +- .../jetty/server/session/JdbcTestServer.java | 17 ++++++---- .../server/session/LastAccessTimeTest.java | 2 ++ .../AbstractInvalidationSessionTest.java | 14 ++++---- .../session/AbstractLastAccessTimeTest.java | 26 +++++++++----- .../AbstractWebAppObjectInSessionTest.java | 9 +++-- .../session/WebAppObjectInSessionServlet.java | 8 +++-- 10 files changed, 95 insertions(+), 49 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java index 60a9194b59a..a877c4e5a9a 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java @@ -42,6 +42,7 @@ import org.eclipse.jetty.server.SessionManager; import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.util.LazyList; import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.statistic.CounterStatistic; import org.eclipse.jetty.util.statistic.SampleStatistic; @@ -839,15 +840,19 @@ public abstract class AbstractSessionManager extends AbstractLifeCycle implement _accessed=_created; _lastAccessed=_created; _requests=1; + Log.debug("new session & id "+_nodeId+" "+_clusterId); } /* ------------------------------------------------------------- */ - protected Session(long created, String clusterId) + protected Session(long created, long accessed, String clusterId) { _created=created; _clusterId=clusterId; _nodeId=_sessionIdManager.getNodeId(_clusterId,null); - _accessed=_created; + _accessed=accessed; + _lastAccessed=accessed; + _requests=1; + Log.debug("new session "+_nodeId+" "+_clusterId); } /* ------------------------------------------------------------- */ @@ -995,19 +1000,19 @@ public abstract class AbstractSessionManager extends AbstractLifeCycle implement { if (!_invalid) { - if (_maxIdleMs>0 && _lastAccessed + _maxIdleMs < time) + _newSession=false; + _lastAccessed=_accessed; + _accessed=time; + + if (_maxIdleMs>0 && _lastAccessed>0 && _lastAccessed + _maxIdleMs < time) { invalidate(); } - else + else { - _newSession=false; - _lastAccessed=_accessed; - _accessed=time; _requests++; } } - } } @@ -1055,6 +1060,7 @@ public abstract class AbstractSessionManager extends AbstractLifeCycle implement { try { + Log.debug("invalidate ",_clusterId); // Notify listeners and unbind values if (_invalid) throw new IllegalStateException(); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java index c3e269a146c..2c7d2feb33e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/HashSessionManager.java @@ -365,9 +365,9 @@ public class HashSessionManager extends AbstractSessionManager } /* ------------------------------------------------------------ */ - protected AbstractSessionManager.Session newSession(long created, String clusterId) + protected AbstractSessionManager.Session newSession(long created, long accessed, String clusterId) { - return new Session(created,clusterId); + return new Session(created,accessed, clusterId); } /* ------------------------------------------------------------ */ @@ -500,7 +500,7 @@ public class HashSessionManager extends AbstractSessionManager //boolean isNew = in.readBoolean(); int requests = in.readInt(); - Session session = (Session)newSession(created, clusterId); + Session session = (Session)newSession(created, System.currentTimeMillis(), clusterId); session._cookieSet = cookieSet; session._lastAccessed = lastAccessed; @@ -543,9 +543,9 @@ public class HashSessionManager extends AbstractSessionManager } /* ------------------------------------------------------------- */ - protected Session(long created, String clusterId) + protected Session(long created, long accessed, String clusterId) { - super(created, clusterId); + super(created, accessed, clusterId); } /* ------------------------------------------------------------- */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java index 6be7279f772..a8ddf526e9d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java @@ -274,9 +274,9 @@ public class JDBCSessionManager extends AbstractSessionManager * Session restored in database. * @param data */ - protected Session (SessionData data) + protected Session (long accessed, SessionData data) { - super(data.getCreated(), data.getId()); + super(data.getCreated(), accessed, data.getId()); _data=data; _data.setMaxIdleMs(_dftMaxIdleSecs*1000); _values=data.getAttributeMap(); @@ -483,13 +483,29 @@ public class JDBCSessionManager extends AbstractSessionManager //sessions to the same node, changing only iff that node fails. SessionData data = null; long now = System.currentTimeMillis(); - if (Log.isDebugEnabled()) Log.debug("now="+now+ - " lastSaved="+(session==null?0:session._data._lastSaved)+ - " interval="+(_saveIntervalSec * 1000)+ - " difference="+(now - (session==null?0:session._data._lastSaved))); + if (Log.isDebugEnabled()) + { + if (session==null) + Log.debug("now="+now+ + " lastSaved="+(session==null?0:session._data._lastSaved)+ + " interval="+(_saveIntervalSec * 1000)); + else + Log.debug("now="+now+ + " lastSaved="+(session==null?0:session._data._lastSaved)+ + " interval="+(_saveIntervalSec * 1000)+ + " lastNode="+session._data.getLastNode()+ + " thisNode="+getIdManager().getWorkerName()+ + " difference="+(now - session._data._lastSaved)); + } if (session==null || ((now - session._data._lastSaved) >= (_saveIntervalSec * 1000))) { + Log.debug("no session ",idInCluster); + data = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + } + else if ((now - session._data._lastSaved) >= (_saveIntervalSec * 1000)) + { + Log.debug("old session",idInCluster); data = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); } else @@ -501,12 +517,12 @@ public class JDBCSessionManager extends AbstractSessionManager { if (!data.getLastNode().equals(getIdManager().getWorkerName()) || session==null) { - //if the session in the database has not already expired - if (data._expiryTime > System.currentTimeMillis()) + if (data._expiryTime > now) { + Log.debug("expired session",idInCluster); //session last used on a different node, or we don't have it in memory - session = new Session(data); + session = new Session(now,data); _sessions.put(idInCluster, session); session.didActivate(); //TODO is this the best way to do this? Or do this on the way out using diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index 97e5548c701..af169518541 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -424,7 +424,7 @@ public class ResponseTest { public TestSession(AbstractSessionManager abstractSessionManager, String id) { - abstractSessionManager.super(System.currentTimeMillis(), id); + abstractSessionManager.super(System.currentTimeMillis(),System.currentTimeMillis(), id); } public Object getAttribute(String name) diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java index 1e900108b2d..ffc61084174 100644 --- a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java @@ -55,17 +55,22 @@ public class JdbcTestServer extends AbstractTestServer return new SessionHandler(sessionManager); } + static int __workers=0; + /** * @see org.eclipse.jetty.server.session.AbstractTestServer#newSessionIdManager() */ @Override - public SessionIdManager newSessionIdManager() + public SessionIdManager newSessionIdManager() { - JDBCSessionIdManager idManager = new JDBCSessionIdManager(_server); - idManager.setScavengeInterval(_scavengePeriod); - idManager.setWorkerName(String.valueOf(System.currentTimeMillis())); - idManager.setDriverInfo(DRIVER_CLASS, CONNECTION_URL); - return idManager; + synchronized(JdbcTestServer.class) + { + JDBCSessionIdManager idManager = new JDBCSessionIdManager(_server); + idManager.setScavengeInterval(_scavengePeriod); + idManager.setWorkerName("w"+(__workers++)); + idManager.setDriverInfo(DRIVER_CLASS, CONNECTION_URL); + return idManager; + } } /** diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/LastAccessTimeTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/LastAccessTimeTest.java index 77c96a7afb2..4d700795935 100644 --- a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/LastAccessTimeTest.java +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/LastAccessTimeTest.java @@ -12,6 +12,7 @@ // ======================================================================== package org.eclipse.jetty.server.session; +import org.eclipse.jetty.util.log.Log; import org.junit.Test; /** @@ -27,6 +28,7 @@ public class LastAccessTimeTest extends AbstractLastAccessTimeTest @Test public void testLastAccessTime() throws Exception { + // Log.getLog().setDebugEnabled(true); super.testLastAccessTime(); } } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractInvalidationSessionTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractInvalidationSessionTest.java index ecf10089851..d7f79294479 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractInvalidationSessionTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractInvalidationSessionTest.java @@ -26,6 +26,8 @@ import org.eclipse.jetty.client.ContentExchange; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.http.HttpMethods; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * AbstractInvalidationSessionTest @@ -71,9 +73,9 @@ public abstract class AbstractInvalidationSessionTest exchange1.setURL(urls[0] + "?action=init"); client.send(exchange1); exchange1.waitForDone(); - assert exchange1.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus()); String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); - assert sessionCookie != null; + assertTrue(sessionCookie != null); // Mangle the cookie, replacing Path with $Path, etc. sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); @@ -84,7 +86,7 @@ public abstract class AbstractInvalidationSessionTest exchange2.getRequestFields().add("Cookie", sessionCookie); client.send(exchange2); exchange2.waitForDone(); - assert exchange2.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); // Invalidate on node1 exchange1 = new ContentExchange(true); @@ -93,7 +95,7 @@ public abstract class AbstractInvalidationSessionTest exchange1.getRequestFields().add("Cookie", sessionCookie); client.send(exchange1); exchange1.waitForDone(); - assert exchange1.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK, exchange1.getResponseStatus()); pause(); @@ -104,7 +106,7 @@ public abstract class AbstractInvalidationSessionTest exchange2.getRequestFields().add("Cookie", sessionCookie); client.send(exchange2); exchange2.waitForDone(); - assert exchange2.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); } finally { @@ -147,7 +149,7 @@ public abstract class AbstractInvalidationSessionTest else if ("test".equals(action)) { HttpSession session = request.getSession(false); - assert session == null; + assertEquals(null,session); } } } diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractLastAccessTimeTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractLastAccessTimeTest.java index 379f6b3b455..c61507df0e7 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractLastAccessTimeTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractLastAccessTimeTest.java @@ -25,6 +25,9 @@ import org.eclipse.jetty.client.ContentExchange; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.http.HttpMethods; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + /** * AbstractLastAccessTimeTest @@ -40,18 +43,18 @@ public abstract class AbstractLastAccessTimeTest String contextPath = ""; String servletMapping = "/server"; - int port1 = random.nextInt(50000) + 10000; int maxInactivePeriod = 8; int scavengePeriod = 2; - AbstractTestServer server1 = createServer(port1, maxInactivePeriod, scavengePeriod); + AbstractTestServer server1 = createServer(0, maxInactivePeriod, scavengePeriod); server1.addContext(contextPath).addServlet(TestServlet.class, servletMapping); server1.start(); + int port1=server1.getPort(); try { - int port2 = random.nextInt(50000) + 10000; - AbstractTestServer server2 = createServer(port2, maxInactivePeriod, scavengePeriod); + AbstractTestServer server2 = createServer(0, maxInactivePeriod, scavengePeriod); server2.addContext(contextPath).addServlet(TestServlet.class, servletMapping); server2.start(); + int port2=server2.getPort(); try { HttpClient client = new HttpClient(); @@ -65,9 +68,9 @@ public abstract class AbstractLastAccessTimeTest exchange1.setURL("http://localhost:" + port1 + contextPath + servletMapping + "?action=init"); client.send(exchange1); exchange1.waitForDone(); - assert exchange1.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK, exchange1.getResponseStatus()); String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); - assert sessionCookie != null; + assertTrue( sessionCookie != null ); // Mangle the cookie, replacing Path with $Path, etc. sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); @@ -85,8 +88,11 @@ public abstract class AbstractLastAccessTimeTest exchange2.getRequestFields().add("Cookie", sessionCookie); client.send(exchange2); exchange2.waitForDone(); - assert exchange2.getResponseStatus() == HttpServletResponse.SC_OK; - + assertEquals(HttpServletResponse.SC_OK , exchange2.getResponseStatus()); + String setCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + if (setCookie!=null) + sessionCookie = setCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + Thread.sleep(requestInterval); } @@ -103,7 +109,9 @@ public abstract class AbstractLastAccessTimeTest exchange1.getRequestFields().add("Cookie", sessionCookie); client.send(exchange1); exchange1.waitForDone(); - assert exchange1.getResponseStatus() == HttpServletResponse.SC_OK; + assertEquals(HttpServletResponse.SC_OK, exchange1.getResponseStatus()); + + // TODO shouldn't the session be expired???? } finally { diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractWebAppObjectInSessionTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractWebAppObjectInSessionTest.java index 3679dc9d420..d19b97e5a5b 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractWebAppObjectInSessionTest.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractWebAppObjectInSessionTest.java @@ -27,6 +27,8 @@ import org.eclipse.jetty.http.HttpMethods; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.resource.Resource; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * AbstractWebAppObjectInSessionTest @@ -126,9 +128,9 @@ public abstract class AbstractWebAppObjectInSessionTest exchange1.setURL("http://localhost:" + port1 + contextPath + servletMapping + "?action=set"); client.send(exchange1); exchange1.waitForDone(); - assert exchange1.getResponseStatus() == HttpServletResponse.SC_OK : exchange1.getResponseStatus(); + assertEquals( HttpServletResponse.SC_OK, exchange1.getResponseStatus()); String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); - assert sessionCookie != null; + assertTrue(sessionCookie != null); // Mangle the cookie, replacing Path with $Path, etc. sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); @@ -139,7 +141,8 @@ public abstract class AbstractWebAppObjectInSessionTest exchange2.getRequestFields().add("Cookie", sessionCookie); client.send(exchange2); exchange2.waitForDone(); - assert exchange2.getResponseStatus() == HttpServletResponse.SC_OK; + + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); } finally { diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/WebAppObjectInSessionServlet.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/WebAppObjectInSessionServlet.java index aa7575485a3..1174b624343 100644 --- a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/WebAppObjectInSessionServlet.java +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/WebAppObjectInSessionServlet.java @@ -22,6 +22,8 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import org.junit.Assert; + /** * WebAppObjectInSessionServlet @@ -50,7 +52,7 @@ public class WebAppObjectInSessionServlet extends HttpServlet { HttpSession session = request.getSession(false); Object staticAttribute = session.getAttribute("staticAttribute"); - assert staticAttribute instanceof TestSharedStatic; + Assert.assertTrue(staticAttribute instanceof TestSharedStatic); // Object objectAttribute = session.getAttribute("objectAttribute"); // assert objectAttribute instanceof TestSharedNonStatic; // Object sessionAttribute = session.getAttribute("sessionAttribute"); @@ -59,7 +61,9 @@ public class WebAppObjectInSessionServlet extends HttpServlet } catch (Exception e) { - e.printStackTrace(); + // e.printStackTrace(); + httpServletResponse.sendError(500,e.toString()); + throw new ServletException(e); } }