You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/05/01 10:43:46 UTC
[tomcat] branch master updated: Place data holder rather than
CrawlerSessionManagerValve in session
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org