Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #5817 - allow CustomRequestLog to be filtered with BiPredicate #6176

Merged
merged 3 commits into from
Apr 27, 2021
Merged

Issue #5817 - allow CustomRequestLog to be filtered with BiPredicate #6176

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
382aed8
Issue #5817 - allow CustomRequestLog to be filtered with BiPredicate
lachlan-roberts Apr 15, 2021
61ead13
Fix intellij warnings in CustomRequestLogTest.
lachlan-roberts Apr 15, 2021
988e158
Improve javadoc for CustomRequestLog.setFilter
lachlan-roberts Apr 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
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;
Expand Down Expand Up @@ -279,6 +280,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
private final String _formatString;
private transient PathMappings<String> _ignorePathMap;
private String[] _ignorePaths;
private BiPredicate<Request, Response> _filter;

public CustomRequestLog()
{
Expand Down Expand Up @@ -311,6 +313,15 @@ public CustomRequestLog(RequestLog.Writer writer, String formatString)
}
}

/**
* 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<Request, Response> filter)
{
_filter = filter;
}

@ManagedAttribute("The RequestLogWriter")
public RequestLog.Writer getWriter()
{
Expand All @@ -325,11 +336,14 @@ public RequestLog.Writer getWriter()
@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);

Expand Down
80 changes: 52 additions & 28 deletions tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -27,7 +26,7 @@
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;
Expand All @@ -53,6 +52,7 @@
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;
Expand All @@ -66,22 +66,24 @@
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<String> _entries = new BlockingArrayQueue<>();
BlockingQueue<Long> requestTimes = new BlockingArrayQueue<>();
ServerConnector _serverConnector;
URI _serverURI;
private final BlockingQueue<String> _entries = new BlockingArrayQueue<>();
private final BlockingQueue<Long> 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);
Expand Down Expand Up @@ -111,6 +113,7 @@ void testHandlerServerStart(String formatString) throws Exception
_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();
Expand Down Expand Up @@ -142,6 +145,22 @@ public void after() throws Exception
_server.stop();
}

@Test
public void testRequestFilter() throws Exception
{
AtomicReference<Boolean> 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
{
Expand Down Expand Up @@ -197,16 +216,16 @@ public void testLogAddress() throws Exception
"%{server}a|%{server}p|" +
"%{client}a|%{client}p");

Enumeration e = NetworkInterface.getNetworkInterfaces();
Enumeration<NetworkInterface> e = NetworkInterface.getNetworkInterfaces();
while (e.hasMoreElements())
{
NetworkInterface n = (NetworkInterface)e.nextElement();
NetworkInterface n = e.nextElement();
if (n.isLoopback())
{
Enumeration ee = n.getInetAddresses();
Enumeration<InetAddress> 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();
Expand All @@ -217,7 +236,7 @@ public void testLogAddress() throws Exception
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];
Expand Down Expand Up @@ -428,7 +447,7 @@ public void testLogRequestTime() throws Exception

_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) + "]"));
}
Expand All @@ -442,7 +461,8 @@ public void testLogRequestTimeCustomFormats() throws Exception

_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");
Expand All @@ -461,7 +481,8 @@ public void testLogLatencyMicroseconds() throws Exception

_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);
Expand All @@ -479,7 +500,8 @@ public void testLogLatencyMilliseconds() throws Exception

_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);
Expand All @@ -497,7 +519,8 @@ public void testLogLatencySeconds() throws Exception

_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);
Expand Down Expand Up @@ -575,11 +598,6 @@ public void testLogResponseTrailer() throws Exception
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);
Expand All @@ -604,10 +622,17 @@ public void write(String requestEntry)
}
}

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));

Expand Down Expand Up @@ -652,8 +677,7 @@ else if (request.getRequestURI().contains("delay"))

if (request.getContentLength() > 0)
{
InputStream in = request.getInputStream();
while (in.read() > 0);
IO.readBytes(request.getInputStream());
}
}
}
Expand Down