DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT <http://nagoya.apache.org/bugzilla/show_bug.cgi?id=15894>. ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND INSERTED IN THE BUG DATABASE.
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=15894 Access to other sessions possible (session is immediately recycled after invalidation/expiration) Summary: Access to other sessions possible (session is immediately recycled after invalidation/expiration) Product: Tomcat 3 Version: 3.3.1 Final Platform: All OS/Version: All Status: NEW Severity: Critical Priority: Other Component: Servlet AssignedTo: [EMAIL PROTECTED] ReportedBy: [EMAIL PROTECTED] Hello it is possible to access a session of somebody else through calling session.invalidate. Here is an example JSP: <% HttpSession s = request.getSession(true); String name = request.getParameter("name"); s.setAttribute("name", name); Thread.sleep(5000); s.invalidate(); Thread.sleep(5000); s = request.getSession(true); String nameFromSession = (String) s.getAttribute("name"); %> <html> <head><title>Test Invalidate</title></head> <body bgcolor="white"> <font size=4> <% if (nameFromSession == null) { %> second session was clean. <% } else { %> Session not Empty after invalidate!!!<p> I got the session from <%=nameFromSession%> <% } %> </font> </body> </html> Login on one browser with http://localhost:8080/examples/testInvalidate.jsp?name=Martin and then after 8 seconds on another browser (if cookies are disabled you can use two windows of the same browser) with http://localhost:8080/examples/testInvalidate.jsp?name=Waldemar You'll get the session of Martin. The problem is that SimpleSessionStore recycles the sessions right away. I attached a path to delay the recycling. The patch also touches three files to reduce racing problems. To fix the problem only SimpleSessionStore needs to be updated. Christian Wicke Here is my patch: diff -Naur ./facade22/org/apache/tomcat/facade/HttpSessionFacade.java ../../../jakarta-tomcat-3.3.1-src/src/facade22/org/apache/tomcat/facade/HttpSessionFacade.java --- ./facade22/org/apache/tomcat/facade/HttpSessionFacade.java Wed Jan 8 15:17:18 2003 +++ ../../../jakarta-tomcat-3.3.1-src/src/facade22/org/apache/tomcat/facade/HttpSessionFacade.java Tue Mar 26 16:36:48 2002 @@ -148,8 +148,7 @@ * @exception IllegalStateException if this method is called on * an invalidated session */ - // avoid parallel call to invalidate through synchronized - public synchronized void invalidate() { + public void invalidate() { checkValid(); realSession.getTimeStamp().setValid( false ); // remove all attributes diff -Naur ./share/org/apache/tomcat/modules/session/SessionExpirer.java ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionExpirer.java --- ./share/org/apache/tomcat/modules/session/SessionExpirer.java Wed Jan 8 13:32:21 2003 +++ ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionExpirer.java Tue Mar 26 16:36:49 2002 @@ -159,14 +159,7 @@ } public void expired(TimeStamp o ) { - ServerSession sses=(ServerSession)o.getParent(); - // double check if this session is not touched in - // meantime by SessionId.processSession - synchronized (sses) { - if (System.currentTimeMillis() - o.getLastAccessedTime() < o.getMaxInactiveInterval()) - // session touched in meantime, don't expire - return; - + ServerSession sses=(ServerSession)o.getParent(); if( debug > 0 ) { se.log( "Session expired " + sses); } @@ -174,7 +167,6 @@ // After expiring it, we clean up. if( debug > 0 ) se.log( "Recycling " + sses); sses.recycle(); - } } } } diff -Naur ./share/org/apache/tomcat/modules/session/SessionId.java ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionId.java --- ./share/org/apache/tomcat/modules/session/SessionId.java Wed Jan 8 15:13:14 2003 +++ ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SessionId.java Tue Mar 26 16:37:15 2002 @@ -239,13 +239,6 @@ sessionId, false ); if( sess!=null ) break; } - if (sess == null) - return null; - // avoid parallel expiration of the SessionExpirer - synchronized (sess) { - //double check whether the session has been expired meantime - if (sess.getState() == ServerSession.STATE_EXPIRED) - return null; /* The following block of code verifies if Tomcat session matches SSL session (if one was ever passed to Tomcat). Just in case @@ -253,7 +246,7 @@ We can't verify that if SSL is not used. */ // Do this only if request is over SSL - if(checkSSLSessionId && request.isSecure() ){ + if(checkSSLSessionId && sess != null && request.isSecure() ){ // SSL session ID from session and request - they have to be equal! String ids=(String)sess.getAttribute("javax.servlet.session.ssl_session"), idr=(String)request.getAttribute("javax.servlet.request.ssl_session"); @@ -283,7 +276,7 @@ // it and adjust the session request.setSession( sess ); request.setSessionId( sessionId ); - + sess.touch( System.currentTimeMillis() ); // if the session was NEW ( never accessed - change it's state ) @@ -291,10 +284,9 @@ sess.setState( ServerSession.STATE_ACCESSED, request); } } - } return sess; } - + // /** Fix the session id. If the session is not valid return null. // * It will also clean up the session from load-balancing strings. // * @return sessionId, or null if not valid diff -Naur ./share/org/apache/tomcat/modules/session/SimpleSessionStore.java ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SimpleSessionStore.java --- ./share/org/apache/tomcat/modules/session/SimpleSessionStore.java Tue Jan 7 14:21:48 2003 +++ ../../../jakarta-tomcat-3.3.1-src/src/share/org/apache/tomcat/modules/session/SimpleSessionStore.java Wed Jan 8 16:15:19 2003 @@ -207,7 +207,9 @@ SimpleSessionManager sm= getManager( ctx ); if( sm == null ) { - sm=new SimpleSessionManager(); + SessionRecycler r = new SessionRecycler(); + r.startRecycler() + sm=new SimpleSessionManager(r); sm.setDebug( debug ); sm.setModule( this ); ctx.getContainer().setNote( manager_note, sm ); @@ -236,16 +238,16 @@ session.setState( ServerSession.STATE_SUSPEND ); session.setState( ServerSession.STATE_EXPIRED ); } + sm.getSessionRecycler.stopRecycler(); } public int sessionState( Request req, ServerSession session, int state ) { TimeStamp ts=session.getTimeStamp(); if( state==ServerSession.STATE_EXPIRED ) { - // session moved to expire state - remove all attributes from - // storage - SimpleSessionManager ssm=(SimpleSessionManager)session.getManager(); - ssm.removeSession( session ); + // session moved to expire state - register to be recycled + ((SimpleSessionManager)session.getManager()).getSessionRecycler() + .registerToRecycle(session); } return state; } @@ -288,8 +290,9 @@ * The actual "simple" manager * */ - public static class SimpleSessionManager + public static class SimpleSessionManager { + private SessionRecycler sessionRecycler; private int debug=0; private BaseInterceptor mod; /** The set of previously recycled Sessions for this Manager. @@ -302,8 +305,13 @@ */ protected Hashtable sessions = new Hashtable(); - public SimpleSessionManager() { + public SimpleSessionManager(SessionRecycler r) { + sessionRecycler = r } + + public SessionRecycler getSessionRecycler() { + return sessionRecycler; + } public void setDebug( int l ) { debug=l; @@ -385,4 +393,62 @@ } } +/** + * Recycles Sessions after waiting for some time. + * Fast Recycling is dangerous this it allows Threads to + * access sessions after it's been recycled + * + * @author Christian Wicke [EMAIL PROTECTED] + */ + // Some of the code was borrowed from Expirer.java and Reaper.java +public final class SessionRecycler extends Thread { + private long interval = 1000 * 60; //ms + private Stack toBeRecycledNow; + private Stack toBeRecycledLater; + private boolean running; + + private SessionRecycler() { + toBeRecycledNow = new Stack(); + toBeRecycledLater = new Stack(); + } + + public synchronized void startRecycler() { + this.running = true; + this.start(); + } + + public synchronized void stopRecycler() { + this.running = false; + System.out.println("Stop recycler"); + this.interrupt(); + } + + public synchronized void registerToRecycle(ServerSession s) { + toBeRecycledLater.push(s); + } + + public void run() { + while (running) { + if( !running) break; + try { + this.sleep(interval); + } catch (InterruptedException ie) { + // wake up when stopRecycler has been called + } + + if( !running) break; + while (!toBeRecycledNow.empty()) { + ServerSession s = (ServerSession) toBeRecycledNow.pop(); + SimpleSessionManager ssm=(SimpleSessionManager)s.getManager(); + ssm.removeSession( s ); + } + synchronized (this) { + Stack temp = toBeRecycledNow; + toBeRecycledNow = toBeRecycledLater; + toBeRecycledLater = temp; + } + } + } +} + } -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>