From d3bc0b931a636c48115cae242f77782520644bc1 Mon Sep 17 00:00:00 2001 From: Lachlan Date: Tue, 13 Aug 2019 14:47:39 +1000 Subject: [PATCH] Issue #3957 - fix bad usage of MethodHandles.lookup() (#3962) * Issue #3957 - fix bad usage of MethodHandles.lookup() Signed-off-by: Lachlan Roberts * Issue #3957 CustomRequestLog remove unnecessary local string variables Signed-off-by: Lachlan Roberts --- .../requestlog/jmh/RequestLogBenchmark.java | 9 +- .../jetty/server/CustomRequestLog.java | 82 ++++++++----------- 2 files changed, 37 insertions(+), 54 deletions(-) diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java index 14aa4efcb81..f8498ae1c3f 100644 --- a/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java +++ b/jetty-jmh/src/main/java/org/eclipse/jetty/requestlog/jmh/RequestLogBenchmark.java @@ -87,10 +87,11 @@ public class RequestLogBenchmark { MethodType logType = methodType(Void.TYPE, StringBuilder.class, String.class); - MethodHandle append = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class)); - MethodHandle logURI = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logURI", logType); - MethodHandle logAddr = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logAddr", logType); - MethodHandle logLength = MethodHandles.lookup().findStatic(RequestLogBenchmark.class, "logLength", logType); + MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodHandle append = lookup.findStatic(RequestLogBenchmark.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class)); + MethodHandle logURI = lookup.findStatic(RequestLogBenchmark.class, "logURI", logType); + MethodHandle logAddr = lookup.findStatic(RequestLogBenchmark.class, "logAddr", logType); + MethodHandle logLength = lookup.findStatic(RequestLogBenchmark.class, "logLength", logType); // setup iteration iteratedLog = new Object[] 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 3f7c0422875..8135dfbc5b8 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 @@ -441,8 +441,9 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog private MethodHandle getLogHandle(String formatString) throws NoSuchMethodException, IllegalAccessException { - MethodHandle append = MethodHandles.lookup().findStatic(CustomRequestLog.class, "append", methodType(Void.TYPE, String.class, StringBuilder.class)); - MethodHandle logHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, "logNothing", methodType(Void.TYPE, StringBuilder.class, Request.class, Response.class)); + 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)); List tokens = getTokens(formatString); Collections.reverse(tokens); @@ -452,7 +453,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (t.isLiteralString()) logHandle = updateLogHandle(logHandle, append, t.literal); else - logHandle = updateLogHandle(logHandle, append, t.code, t.arg, t.modifiers, t.negated); + logHandle = updateLogHandle(logHandle, append, lookup, t.code, t.arg, t.modifiers, t.negated); } return logHandle; @@ -578,7 +579,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog } } - private MethodHandle updateLogHandle(MethodHandle logHandle, MethodHandle append, String code, String arg, List modifiers, boolean negated) throws NoSuchMethodException, IllegalAccessException + 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); @@ -621,7 +622,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog throw new IllegalArgumentException("Invalid arg for %a"); } - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -654,7 +655,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog throw new IllegalArgumentException("Invalid arg for %p"); } - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -668,7 +669,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog else throw new IllegalArgumentException("Invalid argument for %I"); - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -682,7 +683,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog else throw new IllegalArgumentException("Invalid argument for %O"); - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -696,7 +697,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog else throw new IllegalArgumentException("Invalid argument for %S"); - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -704,13 +705,11 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog { if (arg == null || arg.isEmpty()) { - String method = "logRequestCookies"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestCookies", logType); } else { - String method = "logRequestCookie"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestCookie", logTypeArg); specificHandle = specificHandle.bindTo(arg); } break; @@ -718,8 +717,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog case "D": { - String method = "logLatencyMicroseconds"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logLatencyMicroseconds", logType); break; } @@ -728,23 +726,20 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (arg == null || arg.isEmpty()) throw new IllegalArgumentException("No arg for %e"); - String method = "logEnvironmentVar"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logEnvironmentVar", logTypeArg); specificHandle = specificHandle.bindTo(arg); break; } case "f": { - String method = "logFilename"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logFilename", logType); break; } case "H": { - String method = "logRequestProtocol"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestProtocol", logType); break; } @@ -753,23 +748,20 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (arg == null || arg.isEmpty()) throw new IllegalArgumentException("No arg for %i"); - String method = "logRequestHeader"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestHeader", logTypeArg); specificHandle = specificHandle.bindTo(arg); break; } case "k": { - String method = "logKeepAliveRequests"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logKeepAliveRequests", logType); break; } case "m": { - String method = "logRequestMethod"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestMethod", logType); break; } @@ -778,37 +770,32 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (arg == null || arg.isEmpty()) throw new IllegalArgumentException("No arg for %o"); - String method = "logResponseHeader"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseHeader", logTypeArg); specificHandle = specificHandle.bindTo(arg); break; } case "q": { - String method = "logQueryString"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logQueryString", logType); break; } case "r": { - String method = "logRequestFirstLine"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestFirstLine", logType); break; } case "R": { - String method = "logRequestHandler"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestHandler", logType); break; } case "s": { - String method = "logResponseStatus"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseStatus", logType); break; } @@ -845,9 +832,8 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog DateCache logDateCache = new DateCache(format, locale, timeZone); - String method = "logRequestTime"; MethodType logTypeDateCache = methodType(Void.TYPE, DateCache.class, StringBuilder.class, Request.class, Response.class); - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeDateCache); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTime", logTypeDateCache); specificHandle = specificHandle.bindTo(logDateCache); break; } @@ -873,7 +859,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog throw new IllegalArgumentException("Invalid arg for %T"); } - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } @@ -885,21 +871,19 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog else method = "logRequestAuthentication"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, method, logType); break; } case "U": { - String method = "logUrlRequestPath"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logUrlRequestPath", logType); break; } case "X": { - String method = "logConnectionStatus"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logType); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logConnectionStatus", logType); break; } @@ -908,8 +892,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (arg == null || arg.isEmpty()) throw new IllegalArgumentException("No arg for %ti"); - String method = "logRequestTrailer"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logRequestTrailer", logTypeArg); specificHandle = specificHandle.bindTo(arg); break; } @@ -919,8 +902,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (arg == null || arg.isEmpty()) throw new IllegalArgumentException("No arg for %to"); - String method = "logResponseTrailer"; - specificHandle = MethodHandles.lookup().findStatic(CustomRequestLog.class, method, logTypeArg); + specificHandle = lookup.findStatic(CustomRequestLog.class, "logResponseTrailer", logTypeArg); specificHandle = specificHandle.bindTo(arg); break; } @@ -931,7 +913,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog if (modifiers != null && !modifiers.isEmpty()) { - MethodHandle modifierTest = MethodHandles.lookup().findStatic(CustomRequestLog.class, "modify", methodType(Boolean.TYPE, List.class, Boolean.class, StringBuilder.class, Request.class, Response.class)); + MethodHandle modifierTest = lookup.findStatic(CustomRequestLog.class, "modify", methodType(Boolean.TYPE, List.class, Boolean.class, StringBuilder.class, Request.class, Response.class)); MethodHandle dash = updateLogHandle(logHandle, append, "-"); MethodHandle log = foldArguments(logHandle, specificHandle);