This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/7.0.x by this push:
     new 886a779  Place data holder rather than CrawlerSessionManagerValve in 
session
886a779 is described below

commit 886a779582508450a6df2f0dfd88b941dd375be0
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         | 33 ++++++++++------
 .../valves/TestCrawlerSessionManagerValve.java     | 46 +++++++++++++++++++++-
 webapps/docs/changelog.xml                         |  6 +++
 3 files changed, 73 insertions(+), 12 deletions(-)

diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java 
b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java
index ac35a3b..773ef09 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,18 +271,27 @@ 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 valueBound(HttpSessionBindingEvent event) {
-        // NOOP
-    }
+        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) {
-        String clientIdentifier = 
sessionIdClientId.remove(event.getSession().getId());
-        if (clientIdentifier != null) {
-            clientIdSessionId.remove(clientIdentifier);
+        @Override
+        public void valueBound(HttpSessionBindingEvent event) {
+            // NO-OP
+        }
+
+        @Override
+        public void valueUnbound(HttpSessionBindingEvent event) {
+            if (clientIdentifier != null && clientIdSessionId != null) {
+                clientIdSessionId.remove(clientIdentifier);
+            }
         }
     }
 }
diff --git 
a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java 
b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java
index 59a192c..0d6a46a 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 433d2fd..f6d5b6b 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -94,6 +94,12 @@
         defined in <code>server.xml</code> with a <code>docBase</code> but not
         the optional <code>path</code>. (markt)
       </fix>
+      <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>
     </changelog>
   </subsection>
   <subsection name="Coyote">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to