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 8135dfbc5b8..5a58abc84cf 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
@@ -31,12 +31,14 @@ import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.http.QuotedCSV;
import org.eclipse.jetty.http.pathmap.PathMappings;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.DateCache;
+import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.annotation.ManagedAttribute;
import org.eclipse.jetty.util.annotation.ManagedObject;
import org.eclipse.jetty.util.component.ContainerLifeCycle;
@@ -233,8 +235,9 @@ import static java.lang.invoke.MethodType.methodType;
*
* | %{d}u |
*
- * Remote user if the request was authenticated. May be bogus if return status (%s) is 401 (unauthorized).
- * Optional parameter d, with this parameter deferred authentication will also be checked.
+ * Remote user if the request was authenticated with servlet authentication. May be bogus if return status (%s) is 401 (unauthorized).
+ * Optional parameter d, with this parameter deferred authentication will also be checked,
+ * this is equivalent to {@link HttpServletRequest#getRemoteUser()}.
* |
*
*
@@ -294,11 +297,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
{
_logHandle = getLogHandle(formatString);
}
- catch (NoSuchMethodException e)
- {
- throw new IllegalStateException(e);
- }
- catch (IllegalAccessException e)
+ catch (NoSuchMethodException | IllegalAccessException e)
{
throw new IllegalStateException(e);
}
@@ -357,20 +356,14 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
protected static String getAuthentication(Request request, boolean checkDeferred)
{
Authentication authentication = request.getAuthentication();
+ if (checkDeferred && authentication instanceof Authentication.Deferred)
+ authentication = ((Authentication.Deferred)authentication).authenticate(request);
String name = null;
-
- boolean deferred = false;
- if (checkDeferred && authentication instanceof Authentication.Deferred)
- {
- authentication = ((Authentication.Deferred)authentication).authenticate(request);
- deferred = true;
- }
-
if (authentication instanceof Authentication.User)
name = ((Authentication.User)authentication).getUserIdentity().getUserPrincipal().getName();
- return (name == null) ? null : (deferred ? ("?" + name) : name);
+ return name;
}
/**
@@ -415,9 +408,9 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
if (_ignorePaths != null && _ignorePaths.length > 0)
{
_ignorePathMap = new PathMappings<>();
- for (int i = 0; i < _ignorePaths.length; i++)
+ for (String ignorePath : _ignorePaths)
{
- _ignorePathMap.put(_ignorePaths[i], _ignorePaths[i]);
+ _ignorePathMap.put(ignorePath, ignorePath);
}
}
else
@@ -442,8 +435,8 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
private MethodHandle getLogHandle(String formatString) throws NoSuchMethodException, IllegalAccessException
{
MethodHandles.Lookup lookup = MethodHandles.lookup();
- MethodHandle append = lookup.findStatic(CustomRequestLog.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class));
- MethodHandle logHandle = lookup.findStatic(CustomRequestLog.class, "logNothing", methodType(Void.TYPE, StringBuilder.class, Request.class, Response.class));
+ MethodHandle append = lookup.findStatic(CustomRequestLog.class, "append", methodType(void.class, String.class, StringBuilder.class));
+ MethodHandle logHandle = lookup.findStatic(CustomRequestLog.class, "logNothing", methodType(void.class, StringBuilder.class, Request.class, Response.class));
List tokens = getTokens(formatString);
Collections.reverse(tokens);
@@ -486,7 +479,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
String arg = m.group("ARG");
String modifierString = m.group("MOD");
- Boolean negated = false;
+ boolean negated = false;
if (modifierString != null)
{
if (modifierString.startsWith("!"))
@@ -581,8 +574,8 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append, MethodHandles.Lookup lookup, String code, String arg, List modifiers, boolean negated) throws NoSuchMethodException, IllegalAccessException
{
- MethodType logType = methodType(Void.TYPE, StringBuilder.class, Request.class, Response.class);
- MethodType logTypeArg = methodType(Void.TYPE, String.class, StringBuilder.class, Request.class, Response.class);
+ MethodType logType = methodType(void.class, StringBuilder.class, Request.class, Response.class);
+ MethodType logTypeArg = methodType(void.class, String.class, StringBuilder.class, Request.class, Response.class);
//TODO should we throw IllegalArgumentExceptions when given arguments for codes which do not take them
MethodHandle specificHandle;
@@ -596,7 +589,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "a":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
arg = "server";
String method;
@@ -628,7 +621,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "p":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
arg = "server";
String method;
@@ -662,7 +655,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "I":
{
String method;
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
method = "logBytesReceived";
else if (arg.equalsIgnoreCase("clf"))
method = "logBytesReceivedCLF";
@@ -676,7 +669,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "O":
{
String method;
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
method = "logBytesSent";
else if (arg.equalsIgnoreCase("clf"))
method = "logBytesSentCLF";
@@ -690,7 +683,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "S":
{
String method;
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
method = "logBytesTransferred";
else if (arg.equalsIgnoreCase("clf"))
method = "logBytesTransferredCLF";
@@ -703,7 +696,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "C":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
{
specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestCookies", logType);
}
@@ -723,7 +716,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "e":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
throw new IllegalArgumentException("No arg for %e");
specificHandle = lookup.findStatic(CustomRequestLog.class, "logEnvironmentVar", logTypeArg);
@@ -745,7 +738,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "i":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
throw new IllegalArgumentException("No arg for %i");
specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestHeader", logTypeArg);
@@ -767,7 +760,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "o":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
throw new IllegalArgumentException("No arg for %o");
specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseHeader", logTypeArg);
@@ -832,7 +825,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
DateCache logDateCache = new DateCache(format, locale, timeZone);
- MethodType logTypeDateCache = methodType(Void.TYPE, DateCache.class, StringBuilder.class, Request.class, Response.class);
+ MethodType logTypeDateCache = methodType(void.class, DateCache.class, StringBuilder.class, Request.class, Response.class);
specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTime", logTypeDateCache);
specificHandle = specificHandle.bindTo(logDateCache);
break;
@@ -866,10 +859,12 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "u":
{
String method;
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
+ method = "logRequestAuthentication";
+ else if ("d".equals(arg))
method = "logRequestAuthenticationWithDeferred";
else
- method = "logRequestAuthentication";
+ throw new IllegalArgumentException("Invalid arg for %u: " + arg);
specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType);
break;
@@ -889,7 +884,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "ti":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
throw new IllegalArgumentException("No arg for %ti");
specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTrailer", logTypeArg);
@@ -899,7 +894,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
case "to":
{
- if (arg == null || arg.isEmpty())
+ if (StringUtil.isEmpty(arg))
throw new IllegalArgumentException("No arg for %to");
specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseTrailer", logTypeArg);
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java
similarity index 87%
rename from jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java
rename to tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java
index b2a816e887d..32769fc0fd9 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/handler/CustomRequestLogTest.java
+++ b/tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java
@@ -16,7 +16,7 @@
// ========================================================================
//
-package org.eclipse.jetty.server.handler;
+package org.eclipse.jetty.test;
import java.io.IOException;
import java.io.InputStream;
@@ -26,15 +26,25 @@ import java.net.NetworkInterface;
import java.net.Socket;
import java.net.URI;
import java.nio.charset.StandardCharsets;
+import java.util.Base64;
import java.util.Enumeration;
import java.util.Locale;
+import java.util.Objects;
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.TimeUnit;
import javax.servlet.ServletException;
import javax.servlet.ServletOutputStream;
+import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.security.ConstraintMapping;
+import org.eclipse.jetty.security.ConstraintSecurityHandler;
+import org.eclipse.jetty.security.HashLoginService;
+import org.eclipse.jetty.security.SecurityHandler;
+import org.eclipse.jetty.security.UserStore;
+import org.eclipse.jetty.security.authentication.BasicAuthenticator;
import org.eclipse.jetty.server.CustomRequestLog;
import org.eclipse.jetty.server.ForwardedRequestCustomizer;
import org.eclipse.jetty.server.HttpConnectionFactory;
@@ -44,8 +54,12 @@ import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.RequestLog;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
+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.security.Constraint;
+import org.eclipse.jetty.util.security.Credential;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
@@ -88,24 +102,70 @@ public class CustomRequestLogTest
TestRequestLogWriter writer = new TestRequestLogWriter();
_log = new CustomRequestLog(writer, formatString);
_server.setRequestLog(_log);
- _server.setHandler(new TestHandler());
+ ServletContextHandler contextHandler = new ServletContextHandler();
+ contextHandler.setSecurityHandler(getSecurityHandler("username", "password", "testRealm"));
+ contextHandler.addServlet(new ServletHolder(new TestServlet()), "/");
+ _server.setHandler(contextHandler);
_server.start();
String host = _serverConnector.getHost();
if (host == null)
- {
host = "localhost";
- }
+
int localPort = _serverConnector.getLocalPort();
_serverURI = new URI(String.format("http://%s:%d/", host, localPort));
}
+ private static SecurityHandler getSecurityHandler(String username, String password, String realm)
+ {
+ HashLoginService loginService = new HashLoginService();
+ UserStore userStore = new UserStore();
+ userStore.addUser(username, Credential.getCredential(password), new String[]{"user"});
+ loginService.setUserStore(userStore);
+ loginService.setName(realm);
+
+ Constraint constraint = new Constraint();
+ constraint.setName("auth");
+ constraint.setAuthenticate(true);
+ constraint.setRoles(new String[]{"**"});
+
+ ConstraintMapping mapping = new ConstraintMapping();
+ mapping.setPathSpec("/secure/*");
+ mapping.setConstraint(constraint);
+
+ ConstraintSecurityHandler security = new ConstraintSecurityHandler();
+ security.addConstraintMapping(mapping);
+ security.setAuthenticator(new BasicAuthenticator());
+ security.setLoginService(loginService);
+
+ return security;
+ }
+
@AfterEach
public void after() throws Exception
{
_server.stop();
}
+ @Test
+ public void testLogRemoteUser() throws Exception
+ {
+ String authHeader = HttpHeader.AUTHORIZATION + ": Basic " + Base64.getEncoder().encodeToString("username:password".getBytes());
+ testHandlerServerStart("%u %{d}u");
+
+ _connector.getResponse("GET / HTTP/1.0\n\n\n");
+ String log = _entries.poll(5, TimeUnit.SECONDS);
+ assertThat(log, is("- -"));
+
+ _connector.getResponse("GET / HTTP/1.0\n" + authHeader + "\n\n\n");
+ log = _entries.poll(5, TimeUnit.SECONDS);
+ assertThat(log, is("- username"));
+
+ _connector.getResponse("GET /secure HTTP/1.0\n" + authHeader + "\n\n\n");
+ log = _entries.poll(5, TimeUnit.SECONDS);
+ assertThat(log, is("username username"));
+ }
+
@Test
public void testModifier() throws Exception
{
@@ -374,7 +434,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);
- DateCache dateCache = new DateCache(_log.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT");
+ DateCache dateCache = new DateCache(CustomRequestLog.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT");
assertThat(log, is("RequestTime: [" + dateCache.format(requestTime) + "]"));
}
@@ -549,11 +609,13 @@ public class CustomRequestLogTest
}
}
- private class TestHandler extends AbstractHandler
+ private class TestServlet extends HttpServlet
{
@Override
- public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
+ Request baseRequest = Objects.requireNonNull(Request.getBaseRequest(request));
+
if (request.getRequestURI().contains("error404"))
{
response.setStatus(404);
@@ -596,10 +658,7 @@ public class CustomRequestLogTest
if (request.getContentLength() > 0)
{
InputStream in = request.getInputStream();
- while (in.read() > 0)
- {
- ;
- }
+ while (in.read() > 0);
}
}
}