[patch] patch for 7.0.x (PR 156)

156-7.0.x.patch (text/plain), 10.73 KB, created by Martin L on 2019-04-12 08:23:16 UTC

(hide)

Description: patch for 7.0.x (PR 156)

Filename:

MIME Type:

Creator: Martin L

Created: 2019-04-12 08:23:16 UTC

Size: 10.73 KB

patch

obsolete

>From 03c30ee571c2d82aba33c2219961ec28542d5c6d Mon Sep 17 00:00:00 2001 >From: Martin Lemanski <martin.lemanski@willhaben.at> >Date: Mon, 8 Apr 2019 15:42:08 +0200 >Subject: [PATCH 1/2] Don't serialize the whole valve into session, but use a > smaller object. It helps to serialize this sessions with libraries like kryo. > >Background: >we've found out, that our kryo library serializes the whole valve. >Our stack indicates, that jdk internals are getting serialized into the >sessions. It's unnecessary and can be solves more easily with this fix. >--- > .../valves/CrawlerSessionManagerValve.java | 30 +++++++----- > .../TestCrawlerSessionManagerValve.java | 46 ++++++++++++++++++- > 2 files changed, 64 insertions(+), 12 deletions(-) > >diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >index ac35a3b262..659a3ab724 100644 >--- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >+++ b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >@@ -42,7 +42,7 @@ > * 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 +241,7 @@ public void invoke(Request request, Response response) throws IOException, Servl > 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, sessionIdClientId)); > s.setMaxInactiveInterval(sessionInactiveInterval); > > if (log.isDebugEnabled()) { >@@ -269,18 +269,26 @@ private String getClientIdentifier(Host host, Context context, String clientIp) > return result.toString(); > } > >+ private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener { >+ private final Map<String, String> clientIdSessionId; >+ private final Map<String, String> sessionIdClientId; > >- @Override >- public void valueBound(HttpSessionBindingEvent event) { >- // NOOP >- } >+ public CrawlerHttpSessionBindingListener(Map<String, String> clientIdSessionId, Map<String, String> sessionIdClientId) { >+ this.clientIdSessionId = clientIdSessionId; >+ this.sessionIdClientId = sessionIdClientId; >+ } > >+ @Override >+ public void valueBound(HttpSessionBindingEvent event) { >+ // NOOP >+ } > >- @Override >- public void valueUnbound(HttpSessionBindingEvent event) { >- String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); >- if (clientIdentifier != null) { >- clientIdSessionId.remove(clientIdentifier); >+ @Override >+ public void valueUnbound(HttpSessionBindingEvent event) { >+ String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); >+ if (clientIdentifier != null) { >+ clientIdSessionId.remove(clientIdentifier); >+ } > } > } > } >diff --git a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java >index 59a192ce42..a4203f1561 100644 >--- a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java >+++ b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java >@@ -22,7 +22,13 @@ > > import javax.servlet.ServletException; > import javax.servlet.http.HttpSession; >+import javax.servlet.http.HttpSessionBindingListener; > >+import org.apache.catalina.Manager; >+import org.apache.catalina.core.StandardContext; >+import org.apache.catalina.session.StandardManager; >+import org.apache.catalina.session.StandardSession; >+import org.hamcrest.CoreMatchers; > import org.junit.Test; > > import org.apache.catalina.Context; >@@ -33,8 +39,20 @@ > import org.easymock.EasyMock; > import org.easymock.IExpectationSetters; > >+import static org.hamcrest.CoreMatchers.*; >+import static org.hamcrest.MatcherAssert.assertThat; >+ > 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 void testCrawlerMultipleHostsHostAware() throws Exception { > 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); >+ >+ assertThat(valve.getClientIpSessionId().values(), hasItem("id")); >+ >+ session.expire(); >+ >+ assertThat(valve.getClientIpSessionId().values().size(), is(0)); >+ } >+ > > private void verifyCrawlingLocalhost(CrawlerSessionManagerValve valve, String hostname) > throws IOException, ServletException { >@@ -97,7 +141,7 @@ private HttpSession createSessionExpectations(CrawlerSessionManagerValve valve, > 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(); > >From e5128bf506202398415ab41b74b4362026b2419a Mon Sep 17 00:00:00 2001 >From: Martin Lemanski <martin.lemanski@willhaben.at> >Date: Tue, 9 Apr 2019 14:43:35 +0200 >Subject: [PATCH 2/2] Don't serialize the whole valve into session, but use a > smaller object. It helps to serialize this sessions with libraries like kryo. > >Apply feedback from PR review >--- > .../valves/CrawlerSessionManagerValve.java | 18 +++++++++--------- > .../valves/TestCrawlerSessionManagerValve.java | 3 +-- > 2 files changed, 10 insertions(+), 11 deletions(-) > >diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >index 659a3ab724..49eb49cbe0 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; >@@ -241,7 +242,7 @@ public void invoke(Request request, Response response) throws IOException, Servl > clientIdSessionId.put(clientIdentifier, s.getId()); > sessionIdClientId.put(s.getId(), clientIdentifier); > // #valueUnbound() will be called on session expiration >- s.setAttribute(this.getClass().getName(), new CrawlerHttpSessionBindingListener(clientIdSessionId, sessionIdClientId)); >+ s.setAttribute(this.getClass().getName(), new CrawlerHttpSessionBindingListener(clientIdSessionId, clientIdentifier)); > s.setMaxInactiveInterval(sessionInactiveInterval); > > if (log.isDebugEnabled()) { >@@ -269,13 +270,13 @@ private String getClientIdentifier(Host host, Context context, String clientIp) > return result.toString(); > } > >- private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener { >- private final Map<String, String> clientIdSessionId; >- private final Map<String, String> sessionIdClientId; >+ private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener, Serializable { >+ private final transient Map<String, String> clientIdSessionId; >+ private final transient String clientIdentifier; > >- public CrawlerHttpSessionBindingListener(Map<String, String> clientIdSessionId, Map<String, String> sessionIdClientId) { >+ private CrawlerHttpSessionBindingListener(Map<String, String> clientIdSessionId, String clientIdentifier) { > this.clientIdSessionId = clientIdSessionId; >- this.sessionIdClientId = sessionIdClientId; >+ this.clientIdentifier = clientIdentifier; > } > > @Override >@@ -285,9 +286,8 @@ public void valueBound(HttpSessionBindingEvent event) { > > @Override > public void valueUnbound(HttpSessionBindingEvent event) { >- String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); >- if (clientIdentifier != null) { >- clientIdSessionId.remove(clientIdentifier); >+ 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 a4203f1561..b1360a5eaf 100644 >--- a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java >+++ b/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java >@@ -28,7 +28,6 @@ > import org.apache.catalina.core.StandardContext; > import org.apache.catalina.session.StandardManager; > import org.apache.catalina.session.StandardSession; >-import org.hamcrest.CoreMatchers; > import org.junit.Test; > > import org.apache.catalina.Context; >@@ -48,7 +47,7 @@ > > static { > TEST_MANAGER = new StandardManager(); >- TEST_MANAGER.setContext(new StandardContext()); >+ TEST_MANAGER.setContainer(new StandardContext()); > } > > View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]