From 382aed8374b8f5746c9bf9581d255670acc57958 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 15 Apr 2021 19:03:50 +1000 Subject: [PATCH 1/3] Issue #5817 - allow CustomRequestLog to be filtered with BiPredicate Signed-off-by: Lachlan Roberts --- .../jetty/server/CustomRequestLog.java | 20 ++++++++++++++++--- .../jetty/test/CustomRequestLogTest.java | 18 ++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java index 33db3e00871..a0da252bff3 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Locale; import java.util.TimeZone; import java.util.concurrent.TimeUnit; +import java.util.function.BiPredicate; import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -279,6 +280,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog private final String _formatString; private transient PathMappings _ignorePathMap; private String[] _ignorePaths; + private BiPredicate _filter; public CustomRequestLog() { @@ -311,6 +313,15 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog } } + /** + * This allows you to set a custom filter to decide whether to log a request or omit it from the request log. + * @param filter - a BiPredicate which returns true if this request should be logged. + */ + public void setFilter(BiPredicate filter) + { + _filter = filter; + } + @ManagedAttribute("The RequestLogWriter") public RequestLog.Writer getWriter() { @@ -325,11 +336,14 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog @Override public void log(Request request, Response response) { + if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null) + return; + + if (_filter != null && !_filter.test(request, response)) + return; + try { - if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null) - return; - StringBuilder sb = _buffers.get(); sb.setLength(0); diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java index 41a04b19882..fe6fe40727d 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java @@ -27,7 +27,7 @@ import java.util.Locale; import java.util.Objects; import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; -import javax.servlet.ServletException; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -142,6 +142,22 @@ public class CustomRequestLogTest _server.stop(); } + @Test + public void testRequestFilter() throws Exception + { + AtomicReference logRequest = new AtomicReference<>(); + testHandlerServerStart("RequestPath: %U"); + _log.setFilter((request, response) -> logRequest.get()); + + logRequest.set(true); + _connector.getResponse("GET /path HTTP/1.0\n\n"); + assertThat(_entries.poll(5, TimeUnit.SECONDS), is("RequestPath: /path")); + + logRequest.set(false); + _connector.getResponse("GET /path HTTP/1.0\n\n"); + assertNull(_entries.poll(1, TimeUnit.SECONDS)); + } + @Test public void testLogRemoteUser() throws Exception { From 61ead137b24e303f91cd8ae7197483e82bcf2185 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 15 Apr 2021 19:04:30 +1000 Subject: [PATCH 2/3] Fix intellij warnings in CustomRequestLogTest. Signed-off-by: Lachlan Roberts --- .../jetty/test/CustomRequestLogTest.java | 62 +++++++++++-------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java index fe6fe40727d..84c8f0cb1af 100644 --- a/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java +++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.test; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; import java.net.InetAddress; import java.net.NetworkInterface; @@ -53,6 +52,7 @@ import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.DateCache; +import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Credential; import org.junit.jupiter.api.AfterEach; @@ -66,22 +66,24 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThanOrEqualTo; import static org.hamcrest.Matchers.not; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.fail; public class CustomRequestLogTest { - CustomRequestLog _log; - Server _server; - LocalConnector _connector; - BlockingQueue _entries = new BlockingArrayQueue<>(); - BlockingQueue requestTimes = new BlockingArrayQueue<>(); - ServerConnector _serverConnector; - URI _serverURI; + private final BlockingQueue _entries = new BlockingArrayQueue<>(); + private final BlockingQueue requestTimes = new BlockingArrayQueue<>(); + private CustomRequestLog _log; + private Server _server; + private LocalConnector _connector; + private ServerConnector _serverConnector; + private URI _serverURI; private static final long DELAY = 2000; @BeforeEach - public void before() throws Exception + public void before() { _server = new Server(); _connector = new LocalConnector(_server); @@ -111,6 +113,7 @@ public class CustomRequestLogTest _serverURI = new URI(String.format("http://%s:%d/", host, localPort)); } + @SuppressWarnings("SameParameterValue") private static SecurityHandler getSecurityHandler(String username, String password, String realm) { HashLoginService loginService = new HashLoginService(); @@ -213,16 +216,16 @@ public class CustomRequestLogTest "%{server}a|%{server}p|" + "%{client}a|%{client}p"); - Enumeration e = NetworkInterface.getNetworkInterfaces(); + Enumeration e = NetworkInterface.getNetworkInterfaces(); while (e.hasMoreElements()) { - NetworkInterface n = (NetworkInterface)e.nextElement(); + NetworkInterface n = e.nextElement(); if (n.isLoopback()) { - Enumeration ee = n.getInetAddresses(); + Enumeration ee = n.getInetAddresses(); while (ee.hasMoreElements()) { - InetAddress i = (InetAddress)ee.nextElement(); + InetAddress i = ee.nextElement(); try (Socket client = newSocket(i.getHostAddress(), _serverURI.getPort())) { OutputStream os = client.getOutputStream(); @@ -233,7 +236,7 @@ public class CustomRequestLogTest os.write(request.getBytes(StandardCharsets.ISO_8859_1)); os.flush(); - String[] log = _entries.poll(5, TimeUnit.SECONDS).split("\\|"); + String[] log = Objects.requireNonNull(_entries.poll(5, TimeUnit.SECONDS)).split("\\|"); assertThat(log.length, is(8)); String localAddr = log[0]; @@ -444,7 +447,7 @@ public class CustomRequestLogTest _connector.getResponse("GET / HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); - long requestTime = requestTimes.poll(5, TimeUnit.SECONDS); + long requestTime = getTimeRequestReceived(); DateCache dateCache = new DateCache(CustomRequestLog.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT"); assertThat(log, is("RequestTime: [" + dateCache.format(requestTime) + "]")); } @@ -458,7 +461,8 @@ public class CustomRequestLogTest _connector.getResponse("GET / HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); - long requestTime = requestTimes.poll(5, TimeUnit.SECONDS); + assertNotNull(log); + long requestTime = getTimeRequestReceived(); DateCache dateCache1 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "GMT"); DateCache dateCache2 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "EST"); @@ -477,7 +481,8 @@ public class CustomRequestLogTest _connector.getResponse("GET /delay HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); - long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS); + assertNotNull(log); + long lowerBound = getTimeRequestReceived(); long upperBound = System.currentTimeMillis(); long measuredDuration = Long.parseLong(log); @@ -495,7 +500,8 @@ public class CustomRequestLogTest _connector.getResponse("GET /delay HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); - long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS); + assertNotNull(log); + long lowerBound = getTimeRequestReceived(); long upperBound = System.currentTimeMillis(); long measuredDuration = Long.parseLong(log); @@ -513,7 +519,8 @@ public class CustomRequestLogTest _connector.getResponse("GET /delay HTTP/1.0\n\n"); String log = _entries.poll(5, TimeUnit.SECONDS); - long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS); + assertNotNull(log); + long lowerBound = getTimeRequestReceived(); long upperBound = System.currentTimeMillis(); long measuredDuration = Long.parseLong(log); @@ -591,11 +598,6 @@ public class CustomRequestLogTest fail(log); } - protected Socket newSocket() throws Exception - { - return newSocket(_serverURI.getHost(), _serverURI.getPort()); - } - protected Socket newSocket(String host, int port) throws Exception { Socket socket = new Socket(host, port); @@ -620,10 +622,17 @@ public class CustomRequestLogTest } } + private long getTimeRequestReceived() throws InterruptedException + { + Long requestTime = requestTimes.poll(5, TimeUnit.SECONDS); + assertNotNull(requestTime); + return requestTime; + } + private class TestServlet extends HttpServlet { @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { Request baseRequest = Objects.requireNonNull(Request.getBaseRequest(request)); @@ -668,8 +677,7 @@ public class CustomRequestLogTest if (request.getContentLength() > 0) { - InputStream in = request.getInputStream(); - while (in.read() > 0); + IO.readBytes(request.getInputStream()); } } } From 988e158db3d0a3bd2218a21839fad063ada4c7b6 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 26 Apr 2021 16:46:53 +1000 Subject: [PATCH 3/3] Improve javadoc for CustomRequestLog.setFilter Signed-off-by: Lachlan Roberts --- .../src/main/java/org/eclipse/jetty/server/CustomRequestLog.java | 1 + 1 file changed, 1 insertion(+) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java index a0da252bff3..9cfd28e557c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java @@ -315,6 +315,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog /** * This allows you to set a custom filter to decide whether to log a request or omit it from the request log. + * This filter is evaluated after path filtering is applied from {@link #setIgnorePaths(String[])}. * @param filter - a BiPredicate which returns true if this request should be logged. */ public void setFilter(BiPredicate filter)