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

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
                     {
-                        _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();

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);
         }
         
         /* ------------------------------------------------------------- */

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+
+                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)+
-                        " difference="+(now - (session==null?0:session._data._lastSaved)));
+                                " 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

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)

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JdbcTestServer.java

@@ -55,18 +55,23 @@ 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()
+    {
+        synchronized(JdbcTestServer.class)
         {
             JDBCSessionIdManager idManager = new JDBCSessionIdManager(_server);
             idManager.setScavengeInterval(_scavengePeriod);
-        idManager.setWorkerName(String.valueOf(System.currentTimeMillis()));
+            idManager.setWorkerName("w"+(__workers++));
             idManager.setDriverInfo(DRIVER_CLASS, CONNECTION_URL);
             return idManager;
         }
+    }
 
     /** 
      * @see org.eclipse.jetty.server.session.AbstractTestServer#newSessionManager()

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();
     }
 }

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);
             }
         }
     }

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(0, maxInactivePeriod, scavengePeriod);
-            AbstractTestServer server2 = createServer(port2, 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,7 +88,10 @@ 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
                 {

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
                 {

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);
         }
     }