This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new 1c35c6d Place data holder rather than CrawlerSessionManagerValve in session 1c35c6d is described below commit 1c35c6dbd5f158f62a63c428f09537c876bd3735 Author: Mark Thomas <ma...@apache.org> AuthorDate: Wed May 1 11:43:03 2019 +0100 Place data holder rather than CrawlerSessionManagerValve in session Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63324 Refactor the CrawlerSessionManagerValve so that the object placed in the session is compatible with session serialization with mem-cached. Patch provided by Martin Lemanski. --- .../valves/CrawlerSessionManagerValve.java | 26 ++++++++---- .../valves/TestCrawlerSessionManagerValve.java | 46 +++++++++++++++++++++- webapps/docs/changelog.xml | 6 +++ 3 files changed, 70 insertions(+), 8 deletions(-) diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java index a268d4b..0a7968d 100644 --- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java +++ b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java @@ -17,6 +17,7 @@ package org.apache.catalina.valves; import java.io.IOException; +import java.io.Serializable; import java.util.Enumeration; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -42,7 +43,7 @@ import org.apache.juli.logging.LogFactory; * users - regardless of whether or not they provide a session token with their * requests. */ -public class CrawlerSessionManagerValve extends ValveBase implements HttpSessionBindingListener { +public class CrawlerSessionManagerValve extends ValveBase { private static final Log log = LogFactory.getLog(CrawlerSessionManagerValve.class); @@ -241,7 +242,8 @@ public class CrawlerSessionManagerValve extends ValveBase implements HttpSession clientIdSessionId.put(clientIdentifier, s.getId()); sessionIdClientId.put(s.getId(), clientIdentifier); // #valueUnbound() will be called on session expiration - s.setAttribute(this.getClass().getName(), this); + s.setAttribute(this.getClass().getName(), + new CrawlerHttpSessionBindingListener(clientIdSessionId, clientIdentifier)); s.setMaxInactiveInterval(sessionInactiveInterval); if (log.isDebugEnabled()) { @@ -269,12 +271,22 @@ public class CrawlerSessionManagerValve extends ValveBase implements HttpSession return result.toString(); } + private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener, Serializable { + private static final long serialVersionUID = 1L; - @Override - public void valueUnbound(HttpSessionBindingEvent event) { - String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); - if (clientIdentifier != null) { - clientIdSessionId.remove(clientIdentifier); + private final transient Map<String, String> clientIdSessionId; + private final transient String clientIdentifier; + + private CrawlerHttpSessionBindingListener(Map<String, String> clientIdSessionId, String clientIdentifier) { + this.clientIdSessionId = clientIdSessionId; + this.clientIdentifier = clientIdentifier; + } + + @Override + public void valueUnbound(HttpSessionBindingEvent event) { + if (clientIdentifier != null && clientIdSessionId != null) { + clientIdSessionId.remove(clientIdentifier, event.getSession().getId()); + } } } } diff --git a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java index f7a7e26..2055402 100644 --- a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java +++ b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java @@ -22,19 +22,37 @@ import java.util.Collections; import javax.servlet.ServletException; import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpSessionBindingListener; +import org.hamcrest.CoreMatchers; +import org.hamcrest.MatcherAssert; + +import org.junit.Assert; import org.junit.Test; import org.apache.catalina.Context; import org.apache.catalina.Host; +import org.apache.catalina.Manager; import org.apache.catalina.Valve; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; +import org.apache.catalina.core.StandardContext; +import org.apache.catalina.session.StandardManager; +import org.apache.catalina.session.StandardSession; import org.easymock.EasyMock; import org.easymock.IExpectationSetters; public class TestCrawlerSessionManagerValve { + private static final Manager TEST_MANAGER; + + static { + TEST_MANAGER = new StandardManager(); + TEST_MANAGER.setContext(new StandardContext()); + } + + + @Test public void testCrawlerIpsPositive() throws Exception { CrawlerSessionManagerValve valve = new CrawlerSessionManagerValve(); @@ -79,6 +97,32 @@ public class TestCrawlerSessionManagerValve { verifyCrawlingLocalhost(valve, "example.invalid"); } + @Test + public void testCrawlersSessionIdIsRemovedAfterSessionExpiry() throws IOException, ServletException { + CrawlerSessionManagerValve valve = new CrawlerSessionManagerValve(); + valve.setCrawlerIps("216\\.58\\.206\\.174"); + valve.setCrawlerUserAgents(valve.getCrawlerUserAgents()); + valve.setNext(EasyMock.createMock(Valve.class)); + valve.setSessionInactiveInterval(0); + StandardSession session = new StandardSession(TEST_MANAGER); + session.setId("id"); + session.setValid(true); + + Request request = createRequestExpectations("216.58.206.174", session, true); + + EasyMock.replay(request); + + valve.invoke(request, EasyMock.createMock(Response.class)); + + EasyMock.verify(request); + + MatcherAssert.assertThat(valve.getClientIpSessionId().values(), CoreMatchers.hasItem("id")); + + session.expire(); + + Assert.assertEquals(0, valve.getClientIpSessionId().values().size()); + } + private void verifyCrawlingLocalhost(CrawlerSessionManagerValve valve, String hostname) throws IOException, ServletException { @@ -97,7 +141,7 @@ public class TestCrawlerSessionManagerValve { HttpSession session = EasyMock.createMock(HttpSession.class); if (isBot) { EasyMock.expect(session.getId()).andReturn("id").times(2); - session.setAttribute(valve.getClass().getName(), valve); + session.setAttribute(EasyMock.eq(valve.getClass().getName()), EasyMock.anyObject(HttpSessionBindingListener.class)); EasyMock.expectLastCall(); session.setMaxInactiveInterval(60); EasyMock.expectLastCall(); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 928d865..c65b93b 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -84,6 +84,12 @@ Refactor <code>ManagerServlet</code> to avoid loading classes when filtering JNDI resources for resources of a specified type. (markt) </scode> + <fix> + <bug>63324</bug>: Refactor the <code>CrawlerSessionManagerValve</code> + so that the object placed in the session is compatible with session + serialization with mem-cached. Patch provided by Martin Lemanski. + (markt) + </fix> <add> <bug>63358</bug>: Expand the <code>throwOnFailure</code> support in the <code>Connector</code> to include the adding of a <code>Connector</code> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org