diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java index c8578ba8b8c..5ffb6dd9dad 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionIdManager.java @@ -30,7 +30,9 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Random; import java.util.Timer; @@ -81,7 +83,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager protected String _createSessionIdTable; protected String _createSessionTable; - protected String _selectExpiredSessions; + protected String _selectBoundedExpiredSessions; protected String _deleteOldExpiredSessions; protected String _insertId; @@ -96,6 +98,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager protected DatabaseAdaptor _dbAdaptor; + private String _selectExpiredSessions; + /** * DatabaseAdaptor @@ -114,7 +118,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager String _dbName; boolean _isLower; boolean _isUpper; - + public DatabaseAdaptor (DatabaseMetaData dbMeta) @@ -123,7 +127,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager _dbName = dbMeta.getDatabaseProductName().toLowerCase(); LOG.debug ("Using database {}",_dbName); _isLower = dbMeta.storesLowerCaseIdentifiers(); - _isUpper = dbMeta.storesUpperCaseIdentifiers(); + _isUpper = dbMeta.storesUpperCaseIdentifiers(); } /** @@ -455,6 +459,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager inUse = _sessionIds.contains(clusterId); } + if (inUse) return true; //optimisation - if this session is one we've been managing, we can check locally @@ -514,7 +519,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager try { initializeDatabase(); - prepareTables(); + prepareTables(); + cleanExpiredSessions(); super.doStart(); if (LOG.isDebugEnabled()) LOG.debug("Scavenging interval = "+getScavengeInterval()+" sec"); @@ -542,6 +548,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager _timer.cancel(); _timer=null; } + _sessionIds.clear(); super.doStop(); } @@ -559,31 +566,9 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager else return DriverManager.getConnection(_connectionUrl); } - - private void initializeDatabase () - throws Exception - { - if (_datasource != null) - return; //already set up - - if (_jndiName!=null) - { - InitialContext ic = new InitialContext(); - _datasource = (DataSource)ic.lookup(_jndiName); - } - else if ( _driver != null && _connectionUrl != null ) - { - DriverManager.registerDriver(_driver); - } - else if (_driverClassName != null && _connectionUrl != null) - { - Class.forName(_driverClassName); - } - else - throw new IllegalStateException("No database configured for sessions"); - } + /** @@ -594,7 +579,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager throws SQLException { _createSessionIdTable = "create table "+_sessionIdTable+" (id varchar(120), primary key(id))"; - _selectExpiredSessions = "select * from "+_sessionTable+" where expiryTime >= ? and expiryTime <= ?"; + _selectBoundedExpiredSessions = "select * from "+_sessionTable+" where expiryTime >= ? and expiryTime <= ?"; + _selectExpiredSessions = "select * from "+_sessionTable+" where expiryTime >0 and expiryTime <= ?"; _deleteOldExpiredSessions = "delete from "+_sessionTable+" where expiryTime >0 and expiryTime <= ?"; _insertId = "insert into "+_sessionIdTable+" (id) values (?)"; @@ -794,7 +780,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager connection = getConnection(); connection.setAutoCommit(true); //"select sessionId from JettySessions where expiryTime > (lastScavengeTime - scanInterval) and expiryTime < lastScavengeTime"; - PreparedStatement statement = connection.prepareStatement(_selectExpiredSessions); + PreparedStatement statement = connection.prepareStatement(_selectBoundedExpiredSessions); long lowerBound = (_lastScavengeTime - _scavengeIntervalMs); long upperBound = _lastScavengeTime; if (LOG.isDebugEnabled()) @@ -833,7 +819,8 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager if (LOG.isDebugEnabled()) LOG.debug("Deleting old expired sessions expired before "+upperBound); statement = connection.prepareStatement(_deleteOldExpiredSessions); statement.setLong(1, upperBound); - statement.executeUpdate(); + int rows = statement.executeUpdate(); + if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows"); } } } @@ -861,4 +848,121 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager } } } + + /** + * Get rid of sessions and sessionids from sessions that have already expired + * @throws Exception + */ + private void cleanExpiredSessions () + throws Exception + { + Connection connection = null; + List expiredSessionIds = new ArrayList(); + try + { + connection = getConnection(); + connection.setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE); + connection.setAutoCommit(false); + + PreparedStatement statement = connection.prepareStatement(_selectExpiredSessions); + long now = System.currentTimeMillis(); + if (LOG.isDebugEnabled()) LOG.debug ("Searching for sessions expired before {}", now); + + statement.setLong(1, now); + ResultSet result = statement.executeQuery(); + while (result.next()) + { + String sessionId = result.getString("sessionId"); + expiredSessionIds.add(sessionId); + if (LOG.isDebugEnabled()) LOG.debug ("Found expired sessionId={}", sessionId); + } + + Statement sessionsTableStatement = null; + Statement sessionIdsTableStatement = null; + + if (!expiredSessionIds.isEmpty()) + { + sessionsTableStatement = connection.createStatement(); + sessionsTableStatement.executeUpdate(createCleanExpiredSessionsSql("delete from "+_sessionTable+" where sessionId in ", expiredSessionIds)); + sessionIdsTableStatement = connection.createStatement(); + sessionIdsTableStatement.executeUpdate(createCleanExpiredSessionsSql("delete from "+_sessionIdTable+" where id in ", expiredSessionIds)); + } + connection.commit(); + + synchronized (_sessionIds) + { + _sessionIds.removeAll(expiredSessionIds); //in case they were in our local cache of session ids + } + } + catch (Exception e) + { + if (connection != null) + connection.rollback(); + throw e; + } + finally + { + try + { + if (connection != null) + connection.close(); + } + catch (SQLException e) + { + LOG.warn(e); + } + } + } + + + /** + * + * @param sql + * @param connection + * @param expiredSessionIds + * @throws Exception + */ + private String createCleanExpiredSessionsSql (String sql,Collection expiredSessionIds) + throws Exception + { + StringBuffer buff = new StringBuffer(); + buff.append(sql); + buff.append("("); + Iterator itor = expiredSessionIds.iterator(); + while (itor.hasNext()) + { + buff.append("'"+(itor.next())+"'"); + if (itor.hasNext()) + buff.append(","); + } + buff.append(")"); + + if (LOG.isDebugEnabled()) LOG.debug("Cleaning expired sessions with: {}", buff); + return buff.toString(); + } + + private void initializeDatabase () + throws Exception + { + if (_datasource != null) + return; //already set up + + if (_jndiName!=null) + { + InitialContext ic = new InitialContext(); + _datasource = (DataSource)ic.lookup(_jndiName); + } + else if ( _driver != null && _connectionUrl != null ) + { + DriverManager.registerDriver(_driver); + } + else if (_driverClassName != null && _connectionUrl != null) + { + Class.forName(_driverClassName); + } + else + throw new IllegalStateException("No database configured for sessions"); + } + + } 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 e79f8267cc3..d37b4eba0c2 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 @@ -542,7 +542,7 @@ public class JDBCSessionManager extends AbstractSessionManager //if the session has no expiry, or it is not already expired if (data._expiryTime <= 0 || data._expiryTime > now) { - LOG.debug("getSession("+idInCluster+"): lastNode="+data.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); + if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): lastNode="+data.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); data.setLastNode(getSessionIdManager().getWorkerName()); //session last used on a different node, or we don't have it in memory session = new Session(now,data); @@ -553,18 +553,21 @@ public class JDBCSessionManager extends AbstractSessionManager updateSessionNode(data); } else - if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): Session has expired"); + { + LOG.debug("getSession ({}): Session has expired", idInCluster); + + } } else - if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): Session not stale "+session._data); + LOG.debug("getSession({}): Session not stale {}", idInCluster,session._data); //session in db shares same id, but is not for this context } else { //No session in db with matching id and context path. session=null; - if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): No session in database matching id="+idInCluster); + LOG.debug("getSession({}): No session in database matching id={}",idInCluster,idInCluster); } return session; @@ -608,6 +611,7 @@ public class JDBCSessionManager extends AbstractSessionManager _jdbcSessionIdMgr = (JDBCSessionIdManager)_sessionIdManager; _sessions = new ConcurrentHashMap(); + super.doStart(); } diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/SessionExpiryTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/SessionExpiryTest.java new file mode 100644 index 00000000000..f9c09224e2a --- /dev/null +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/SessionExpiryTest.java @@ -0,0 +1,53 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// +package org.eclipse.jetty.server.session; + +import org.junit.Test; + +/** + * SessionExpiryTest + * + * + * + */ +public class SessionExpiryTest extends AbstractSessionExpiryTest +{ + + /** + * @see org.eclipse.jetty.server.session.AbstractSessionExpiryTest#createServer(int, int, int) + */ + @Override + public AbstractTestServer createServer(int port, int max, int scavenge) + { + return new JdbcTestServer(port,max,scavenge); + } + + @Test + public void testSessionExpiry() throws Exception + { + super.testSessionExpiry(); + } + + @Test + public void testSessionNotExpired() throws Exception + { + super.testSessionNotExpired(); + } + + +} diff --git a/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionExpiryTest.java b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionExpiryTest.java new file mode 100644 index 00000000000..ad870dfc863 --- /dev/null +++ b/tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionExpiryTest.java @@ -0,0 +1,210 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// +package org.eclipse.jetty.server.session; + +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.ContentExchange; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.http.HttpMethods; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.SessionManager; +import org.eclipse.jetty.server.session.AbstractTestServer; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + + + +/** + * AbstractSessionExpiryTest + * + * + * + */ +public abstract class AbstractSessionExpiryTest +{ + public abstract AbstractTestServer createServer(int port, int max, int scavenge); + + public void pause(int scavengePeriod) + { + try + { + Thread.sleep(scavengePeriod * 2500L); + } + catch (InterruptedException e) + { + e.printStackTrace(); + } + } + + @Test + public void testSessionNotExpired() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + int inactivePeriod = 10; + int scavengePeriod = 10; + AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + server1.addContext(contextPath).addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try + { + HttpClient client = new HttpClient(); + client.setConnectorType(HttpClient.CONNECTOR_SOCKET); + client.start(); + String url = "http://localhost:" + port1 + contextPath + servletMapping; + + //make a request to set up a session on the server + ContentExchange exchange1 = new ContentExchange(true); + exchange1.setMethod(HttpMethods.GET); + exchange1.setURL(url + "?action=init"); + client.send(exchange1); + exchange1.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus()); + String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //now stop the server + server1.stop(); + + //start the server again, before the session times out + server1.start(); + port1 = server1.getPort(); + url = "http://localhost:" + port1 + contextPath + servletMapping; + + //make another request, the session should not have expired + ContentExchange exchange2 = new ContentExchange(true); + exchange2.setMethod(HttpMethods.GET); + exchange2.setURL(url + "?action=notexpired"); + exchange2.getRequestFields().add("Cookie", sessionCookie); + client.send(exchange2); + exchange2.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); + + } + finally + { + server1.stop(); + } + } + + @Test + public void testSessionExpiry() throws Exception + { + String contextPath = ""; + String servletMapping = "/server"; + int inactivePeriod = 2; + int scavengePeriod = 10; + AbstractTestServer server1 = createServer(0, inactivePeriod, scavengePeriod); + TestServlet servlet = new TestServlet(); + ServletHolder holder = new ServletHolder(servlet); + server1.addContext(contextPath).addServlet(holder, servletMapping); + server1.start(); + int port1 = server1.getPort(); + + try + { + HttpClient client = new HttpClient(); + client.setConnectorType(HttpClient.CONNECTOR_SOCKET); + client.start(); + String url = "http://localhost:" + port1 + contextPath + servletMapping; + + //make a request to set up a session on the server + ContentExchange exchange1 = new ContentExchange(true); + exchange1.setMethod(HttpMethods.GET); + exchange1.setURL(url + "?action=init"); + client.send(exchange1); + exchange1.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange1.getResponseStatus()); + String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //now stop the server + server1.stop(); + + //and wait until the expiry time has passed + pause(inactivePeriod); + + //restart the server + server1.start(); + port1 = server1.getPort(); + url = "http://localhost:" + port1 + contextPath + servletMapping; + + //make another request, the session should have expired + ContentExchange exchange2 = new ContentExchange(true); + exchange2.setMethod(HttpMethods.GET); + exchange2.setURL(url + "?action=test"); + exchange2.getRequestFields().add("Cookie", sessionCookie); + client.send(exchange2); + exchange2.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); + } + finally + { + server1.stop(); + } + } + + public static class TestServlet extends HttpServlet + { + public String originalId = null; + public String testId = null; + public String checkId = null; + + @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"); + originalId = session.getId(); + } + else if ("test".equals(action)) + { + HttpSession session = request.getSession(true); + assertTrue(session != null); + assertTrue(!originalId.equals(session.getId())); + } + else if ("notexpired".equals(action)) + { + HttpSession session = request.getSession(false); + assertTrue(session != null); + assertTrue(originalId.equals(session.getId())); + } + + } + } +}