From 57cff606fd10b4c12bf2ed54e748b78eecc1b50e Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 8 Mar 2013 13:33:48 +1100 Subject: [PATCH] 400144 When loading a session fails the JDBCSessionManger produces duplicate session IDs --- .../server/session/JDBCSessionIdManager.java | 2 +- .../server/session/JDBCSessionManager.java | 68 +++++--- .../jetty/server/session/JdbcTestServer.java | 6 +- .../ReloadedSessionMissingClassTest.java | 153 ++++++++++++++++++ .../src/test/resources/foobar.jar | Bin 0 -> 2531 bytes .../src/test/resources/foobarNOfoo.jar | Bin 0 -> 1971 bytes 6 files changed, 198 insertions(+), 31 deletions(-) create mode 100644 tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java create mode 100644 tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar create mode 100644 tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar 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 e15a47d6a2f..a72c0702b09 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 @@ -821,7 +821,7 @@ public class JDBCSessionIdManager extends AbstractSessionIdManager statement = connection.prepareStatement(_deleteOldExpiredSessions); statement.setLong(1, upperBound); int rows = statement.executeUpdate(); - if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows"); + if (LOG.isDebugEnabled()) LOG.debug("Deleted "+rows+" rows of old sessions expired before "+upperBound); } } } 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 4bb8815b02d..ffdfa41e966 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 @@ -279,6 +279,8 @@ public class JDBCSessionManager extends AbstractSessionManager return false; } } + + /** * Exit from session @@ -330,7 +332,7 @@ public class JDBCSessionManager extends AbstractSessionManager return "Session rowId="+_rowId+",id="+getId()+",lastNode="+_lastNode+ ",created="+getCreationTime()+",accessed="+getAccessed()+ ",lastAccessed="+getLastAccessedTime()+",cookieSet="+_cookieSet+ - "lastSaved="+_lastSaved; + ",lastSaved="+_lastSaved+",expiry="+_expiryTime; } } @@ -440,8 +442,6 @@ public class JDBCSessionManager extends AbstractSessionManager synchronized (this) { - try - { //check if we need to reload the session - //as an optimization, don't reload on every access //to reduce the load on the database. This introduces a window of @@ -469,22 +469,31 @@ public class JDBCSessionManager extends AbstractSessionManager " difference="+(now - memSession._lastSaved)); } - if (memSession==null) + try { - LOG.debug("getSession("+idInCluster+"): no session in session map. Reloading session data from db."); - session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + if (memSession==null) + { + LOG.debug("getSession("+idInCluster+"): no session in session map. Reloading session data from db."); + session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + } + else if ((now - memSession._lastSaved) >= (_saveIntervalSec * 1000L)) + { + LOG.debug("getSession("+idInCluster+"): stale session. Reloading session data from db."); + session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); + } + else + { + LOG.debug("getSession("+idInCluster+"): session in session map"); + session = memSession; + } } - else if ((now - memSession._lastSaved) >= (_saveIntervalSec * 1000L)) + catch (Exception e) { - LOG.debug("getSession("+idInCluster+"): stale session. Reloading session data from db."); - session = loadSession(idInCluster, canonicalize(_context.getContextPath()), getVirtualHost(_context)); - } - else - { - LOG.debug("getSession("+idInCluster+"): session in session map"); - session = memSession; + LOG.warn("Unable to load session "+idInCluster, e); + return null; } + //If we have a session if (session != null) { @@ -494,13 +503,23 @@ public class JDBCSessionManager extends AbstractSessionManager //if session doesn't expire, or has not already expired, update it and put it in this nodes' memory if (session._expiryTime <= 0 || session._expiryTime > now) { - if (LOG.isDebugEnabled()) LOG.debug("getSession("+idInCluster+"): lastNode="+session.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); + if (LOG.isDebugEnabled()) + LOG.debug("getSession("+idInCluster+"): lastNode="+session.getLastNode()+" thisNode="+getSessionIdManager().getWorkerName()); + session.setLastNode(getSessionIdManager().getWorkerName()); _sessions.put(idInCluster, session); - session.didActivate(); - //TODO is this the best way to do this? Or do this on the way out using - //the _dirty flag? - updateSessionNode(session); + + //update in db: if unable to update, session will be scavenged later + try + { + updateSessionNode(session); + session.didActivate(); + } + catch (Exception e) + { + LOG.warn("Unable to update freshly loaded session "+idInCluster, e); + return null; + } } else { @@ -519,12 +538,6 @@ public class JDBCSessionManager extends AbstractSessionManager } return session; - } - catch (Exception e) - { - LOG.warn("Unable to get session", e); - return null; - } } } @@ -840,7 +853,12 @@ public class JDBCSessionManager extends AbstractSessionManager _context.getContextHandler().handle(load); if (_exception.get()!=null) + { + //if the session could not be restored, take its id out of the pool of currently-in-use + //session ids + _jdbcSessionIdMgr.removeSession(id); throw _exception.get(); + } return _reference.get(); } 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 d65213c9092..dbdcd47e511 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 @@ -53,11 +53,7 @@ public class JdbcTestServer extends AbstractTestServer super(port, maxInactivePeriod, scavengePeriod, DEFAULT_CONNECTION_URL); } - public JdbcTestServer (int port, boolean optimize) - { - super(port); - } - + /** * @see org.eclipse.jetty.server.session.AbstractTestServer#newSessionHandler(org.eclipse.jetty.server.SessionManager) */ diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java new file mode 100644 index 00000000000..54094cd206f --- /dev/null +++ b/tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/ReloadedSessionMissingClassTest.java @@ -0,0 +1,153 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.FileWriter; +import java.net.URL; +import java.net.URLClassLoader; + +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.client.ContentExchange; +import org.eclipse.jetty.client.HttpClient; +import org.eclipse.jetty.http.HttpMethods; +import org.eclipse.jetty.util.IO; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.StdErrLog; +import org.eclipse.jetty.webapp.WebAppContext; +import org.junit.Test; + +/** + * ReloadedSessionMissingClassTest + * + * + * + */ +public class ReloadedSessionMissingClassTest +{ + + @Test + public void testSessionReloadWithMissingClass() throws Exception + { + ((StdErrLog)Log.getLogger(org.eclipse.jetty.server.session.JDBCSessionManager.class)).setHideStacks(true); + String contextPath = "/foo"; + File srcDir = new File(System.getProperty("basedir"), "src"); + File targetDir = new File(System.getProperty("basedir"), "target"); + File testDir = new File (srcDir, "test"); + File resourcesDir = new File (testDir, "resources"); + + File unpackedWarDir = new File (targetDir, "foo"); + if (unpackedWarDir.exists()) + IO.delete(unpackedWarDir); + unpackedWarDir.mkdir(); + + File webInfDir = new File (unpackedWarDir, "WEB-INF"); + webInfDir.mkdir(); + + File webXml = new File(webInfDir, "web.xml"); + String xml = + "\n" + + "\n" + + "\n" + + "\n"+ + " 1\n" + + "\n"+ + ""; + FileWriter w = new FileWriter(webXml); + w.write(xml); + w.close(); + + File foobarJar = new File (resourcesDir, "foobar.jar"); + File foobarNOfooJar = new File (resourcesDir, "foobarNOfoo.jar"); + + URL[] foobarUrls = new URL[]{foobarJar.toURI().toURL()}; + URL[] barUrls = new URL[]{foobarNOfooJar.toURI().toURL()}; + + URLClassLoader loaderWithFoo = new URLClassLoader(foobarUrls, Thread.currentThread().getContextClassLoader()); + URLClassLoader loaderWithoutFoo = new URLClassLoader(barUrls, Thread.currentThread().getContextClassLoader()); + + + AbstractTestServer server1 = new JdbcTestServer(0); + WebAppContext webApp = server1.addWebAppContext(unpackedWarDir.getCanonicalPath(), contextPath); + webApp.setClassLoader(loaderWithFoo); + webApp.addServlet("Bar", "/bar"); + server1.start(); + int port1 = server1.getPort(); + try + { + HttpClient client = new HttpClient(); + client.setConnectorType(HttpClient.CONNECTOR_SOCKET); + client.start(); + try + { + // Perform one request to server1 to create a session + ContentExchange exchange1 = new ContentExchange(true); + exchange1.setMethod(HttpMethods.GET); + exchange1.setURL("http://localhost:" + port1 + contextPath +"/bar?action=set"); + client.send(exchange1); + exchange1.waitForDone(); + assertEquals( HttpServletResponse.SC_OK, exchange1.getResponseStatus()); + String sessionCookie = exchange1.getResponseFields().getStringField("Set-Cookie"); + assertTrue(sessionCookie != null); + String sessionId = (String)webApp.getServletContext().getAttribute("foo"); + assertNotNull(sessionId); + // Mangle the cookie, replacing Path with $Path, etc. + sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path="); + + //Stop the webapp + webApp.stop(); + + webApp.setClassLoader(loaderWithoutFoo); + webApp.addServlet("Bar", "/bar"); + + //restart webapp + webApp.start(); + + ContentExchange exchange2 = new ContentExchange(true); + exchange2.setMethod(HttpMethods.GET); + exchange2.setURL("http://localhost:" + port1 + contextPath + "/bar?action=get"); + exchange2.getRequestFields().add("Cookie", sessionCookie); + client.send(exchange2); + exchange2.waitForDone(); + assertEquals(HttpServletResponse.SC_OK,exchange2.getResponseStatus()); + String afterStopSessionId = (String)webApp.getServletContext().getAttribute("foo.session"); + + assertNotNull(afterStopSessionId); + assertTrue(!afterStopSessionId.equals(sessionId)); + + } + finally + { + client.stop(); + } + } + finally + { + server1.stop(); + } + } +} diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobar.jar new file mode 100644 index 0000000000000000000000000000000000000000..29b46ddee9f7fd818a0973f7768f89899d2d5165 GIT binary patch literal 2531 zcmZ`*c{mi@9v-{FsLW&?YlCbXI&v~Bn$L~GM?>)cwyjTl1b_jr*n;Wp;f5s5N)PMkX z0LH`)se-aHRsBu^0N4On3odR(1nXa6*#8TL&?EN17^D@-)Wp^fjxk-qkh?7{RN#Fe z3l&8QxtD08M%5Ub4uzvJ$|y`%pJv}GeQTagk*C7+5TTc zmqS^8976Ay?GYpT7RL1P>k$3!f4>0$VCWGF_YCmB;jY;CIEvXrcALE9f^scENrphd z2Z8bINgEF^eux+SB?`X4lDp0X*dyt%T$GR8jcg_D??CwR4xH0~$w!(_<6H|OEjev;>%@e%V$`hg;K2Ytjw(2|MWz^CBs{50;m}2RUkbKW%>Cgs0 z#~mfUaW6jt@?;n(K{}`M2<`D8*e4H?!>dE3b?Rnt24uK0%Z`S>1 zUBV#F>GsP{;ChH?8FHmgy?l-OTzNuJj>NE*KeDE%R)-XmL&T^_C@zim=A>2fPp5cjYWMvdAS-lGXD79EVhR1ize@LS$ehWG(ydk)c zFXqx>SM$_S=#;y0T4{j6n6$4XdcGo_M+;yp$q(bqk4qw4z9_?lr( zOT!gzo6luf72WhVCvpy3HxpxZYiRGA->+qUbPDh9-o$H(9vmM%;)ZkCNa#FK_w6Au zH{kP=^mZ#{Ot+`-?CP29EKK0*e(YF8oluro`R)i6aASU=wtkj}XsP5<@e~gFY_3oE za=+}ZZ0|L6KCQ$%%AIngJIktscf=!0KNiHtCU~4(RCEnE&wUD_^scl-h4|{a_uvZV zDJ-g6|BHZT;X_h*VT<6LesS5B*W4S|#x5fy?&(Z4{KMeHS@vn$uE0$vp+OCCi-`4i z=VeC0NW%s>@$B2Ra)BvEs^PqNPvG^O_{CfYx2f6wOo<03LNXbTX$N)rH+kc`2KWeI z80Iyi^kVoYk@vPyMHz-ZFsy0u7^dKmP(qF50Z#e(AmTC>Jigyyg5=8fxwo@d5o2uF zdARqb*y{;45KTn0SZjZeu>g9=+&J&(!FEfwN7oE!!badkk1x_W*1HDV?uM}`V{_9HJMq1`ld{sA$4LbZA(oJY$h6gbuh;;9BrfLK@BN3`Hw_BuCfJrgkZf?Ysl3uH8si(fEQQtBJ;Tu|<~TSwUsl`Gq@bku`9pneWtYtR z)&8LTY{JteYGL%n1}#-NBT3&B34wRkkjn|Kc4Ze77f5w(bqOKzI)5k!PFhZn7$PVe zNRF2hT*dLi%M`;Bxo%y4+BP_=Lo#)wi+!t&<;^M6+1(7qSv9cjBb> zxl~)kv53F>9w(zw$LshNcC5#FYSf||@2QJ)fCzgSL#IV{>4JXg6! z4UkU9E$UcnhE%kaznvc-yiDiu5i)e`Jn)7{CrBBW)!e)CrT4O$UdGc!_0|#kL(N zt19%qPt| zZ5%DX_9WC8D_H2Lb+w*z8Wm=Cw{z?2+rR#7@YkNyK>Dk@{o~JO zVTAyG_Z>e?1;cmz-u8?H!-Zfieupw!0fy`N)sEArdr&41!n^{*GB67Y@E4puYoU)l z1HlYqs10+S{SIrUUtmvrm|=_}WM=AzncCZNgwEa1l4M3PSY%G#@2EF)8T`p9)`EkR Qu^cD8JJMf6#E;NF0i25EV*mgE literal 0 HcmV?d00001 diff --git a/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar b/tests/test-sessions/test-jdbc-sessions/src/test/resources/foobarNOfoo.jar new file mode 100644 index 0000000000000000000000000000000000000000..fcb3ddf78c97ee9ce0ae1c1a03d2bb275579c7c4 GIT binary patch literal 1971 zcmWIWW@Zs#;Nak3c#)mq#DD}i8CV#6T|*poJ^kGD|D9rBU}gyLX6FE@V1ggv$f(r%Ow?;Ofmi9_kZv9-{1TF?4Q5q z-`6u$$j|F5Jd^F={*mwA)erOhPVq1Ku&}TH!$upsiv^zUY>~3lKdx-k4vaZ!D$5x& z-Jd&9=aJO$bQ$xKBO8}ITz2LDr44Hy7uPIQJYG0ExmB@fXIA)MZJviIS#upDOHMT@ zPFsB|Y3aULk$1Bqw|Fd`Hh1=+^a$T)+s?=qo4@5OKQ2^Pb@_Yt%jgo3sIJhDJM?Vq ztCh~Iw%n$^$>{aUru=Dgcg?aLH!av=b6mD>x6EqBex~D6&2`D!RkG@)%2@7G+4&MZ^CtJm|eH}6q<&25Ir zyJpT)4xOt$^qFZY@AH7r;#T#XRh!cu7{;)fxtj`XO>V5VK4tLJYjWa@s;w6dc=oA8 ze7L(R_UC>jCaqYzpO2<#1n%2hYrW~kjeR%1_TP#qy>$9#pP7{C!~2G@#nC^SPWW&A zwr=x`oL4)#j``{Mo=%o{`6XlOB;VW%mjWJ?Z55v+z4HIPD-2WKKH9SFCD&#j?dVmz z^>|--*v);jX4z8J^Ao*hzgU!76?e|?^d!GU-1ibE+12v$FpJr;NJ)L=e9Ms_qJ3@g zBHhhLC#T;2?7Lf~>a^V(A=CNmPL<9-B=*{F;j-T;uP?{!I_d0Ky!&~z-p$*O@>re* zpUnLkA%5Ff*1PQcwMwP?!j4Wm)nup6+oG1+=B!`J-IvTcd0ON98DX(cUS8^#Tk9&W z)V=+Ms5ReD?#7eXdFBeM_+GGITwnT3@oPe4}9k+i#KhO7i3d^o4@&E zfbc_sut%q2m|vYgabC;U#p#5``Ge9I9x?O2kThLrCUDk8$m@=W%Pb4gV?NC5Cpx^4 za29S(e4{um@V~>KIH%gShx@;6xYsu|AWlKLV$QoS6CTczP;8Nsa2I~XU69J}^~3U< z44Z7u&!fkger7sYw%fFIbU)eNlxUSKaX_Iq?NN%XqtIfH$2_%jk8xN#z9_u2)a|Z% zKl_yfT;~oSa{bfb`KLwmPgCZfE)m!Hs@ef;_6q7#AKM&f?dz7AJg1;1xk~=x1x7u^ zOUX~Bm>!v^^wMG5-hRtFMw@G2{5CKU_EC2+JukF+dvoKtzuce#=*oZp14o$|7+Tno z^ZpjJybmg;KzTnau`F?G(Cxh220XRb!#C7TYWDftpuiu!(Yt|B_S(M&CF8B!;h$8j z&PF}C?|r&2+1fEoL?Y2)T7{*Jjm|u+=bUn1!`_ED?AM)mB}cKV_`PMIY2m6vE3dx2 zKKEc3SGu@U%=t8?v=6S;cE8?7g?!zm!!skO{Xzv}d5anMhvwDywsJfQWr%(%yHN2= z!s{*`qwlhwe23~D&zE9u%s+40Q?szhOySA$e9eG&cZ(a|?ai;|W9Hk~x^3dIcPGTZ zF>cAM*-FNC zVAS&|PqJkmn7I9W|H5t^Pl0gzxyALB4`+Sf={{F8xVnGZt=T@(9n$HYshtn)=boDp zxytKbr@qn8XE8>NY&&A@H$^oXrbN}wW!Tpg96zR)!IhFS_GB9Yef+C%fNrVBl(t_1Uph^oBz-u8;6^5=A zxgG&kSqNYYWWu!~Rba>_fa)9M)X0g@4rCnyVtDO?tPPY@k^KX%QsG+n0X+aqxB=d* PY#<31AbbU+H-mWq39A^~ literal 0 HcmV?d00001