[patch] patch for master

154-master.patch (text/plain), 10.22 KB, created by Martin L on 2019-04-12 08:21:16 UTC

(hide)

Description: patch for master

Filename:

MIME Type:

Creator: Martin L

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

Size: 10.22 KB

patch

obsolete

>From 7cddc41b007527d1b4247e05619d8adf81374420 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 | 23 +++++++--- > .../TestCrawlerSessionManagerValve.java | 46 ++++++++++++++++++- > 2 files changed, 61 insertions(+), 8 deletions(-) > >diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >index a268d4b426..fdf39577ab 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,12 +269,21 @@ 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 valueUnbound(HttpSessionBindingEvent event) { >- String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); >- if (clientIdentifier != null) { >- clientIdSessionId.remove(clientIdentifier); >+ public CrawlerHttpSessionBindingListener(Map<String, String> clientIdSessionId, Map<String, String> sessionIdClientId) { >+ this.clientIdSessionId = clientIdSessionId; >+ this.sessionIdClientId = sessionIdClientId; >+ } >+ >+ @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 f7a7e2652d..9f0df1361e 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 579af295b5c3047699060dd969ad9b966551354e Mon Sep 17 00:00:00 2001 >From: Martin Lemanski <martin.lemanski@willhaben.at> >Date: Tue, 9 Apr 2019 14:53:45 +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 > >fix 63324 >--- > .../valves/CrawlerSessionManagerValve.java | 18 +++++++++--------- > .../valves/TestCrawlerSessionManagerValve.java | 1 - > 2 files changed, 9 insertions(+), 10 deletions(-) > >diff --git a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java b/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java >index fdf39577ab..d85f1537dd 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,20 +270,19 @@ 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 > 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 9f0df1361e..3f4b978ed4 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; 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]