From 04cc21fe40816b0bab0581960097eb92d2121874 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 23 Mar 2020 16:43:09 +1100 Subject: [PATCH] Issue #4691 - use static methods with javadoc to get MethodHandle lookup Signed-off-by: Lachlan Roberts --- .../common/JavaxWebSocketFrameHandler.java | 4 +- .../JavaxWebSocketFrameHandlerFactory.java | 59 ++++++++++++++++--- .../messages/DecodedBinaryMessageSink.java | 4 +- .../DecodedBinaryStreamMessageSink.java | 4 +- .../messages/DecodedTextMessageSink.java | 4 +- .../DecodedTextStreamMessageSink.java | 4 +- .../messages/AbstractMessageSinkTest.java | 4 +- .../tests/CompletableFutureMethodHandle.java | 4 +- .../JettyWebSocketFrameHandlerFactory.java | 55 +++++++++++++++-- .../common/OutgoingMessageCapture.java | 2 +- 10 files changed, 118 insertions(+), 26 deletions(-) diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java index dcd6ad1189bf..8bb3718724d2 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java @@ -374,7 +374,7 @@ public void addMessageHandler(JavaxWebSocketSession session, Class clazz, try { // TODO: move methodhandle lookup to container? - MethodHandles.Lookup lookup = MethodHandles.publicLookup(); + MethodHandles.Lookup lookup = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup(); MethodHandle partialMessageHandler = lookup .findVirtual(MessageHandler.Partial.class, "onMessage", MethodType.methodType(void.class, Object.class, boolean.class)); partialMessageHandler = partialMessageHandler.bindTo(handler); @@ -432,7 +432,7 @@ public void addMessageHandler(JavaxWebSocketSession session, Class clazz, try { // TODO: move MethodHandle lookup to container? - MethodHandles.Lookup lookup = MethodHandles.publicLookup(); + MethodHandles.Lookup lookup = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup(); MethodHandle wholeMsgMethodHandle = lookup.findVirtual(MessageHandler.Whole.class, "onMessage", MethodType.methodType(void.class, Object.class)); wholeMsgMethodHandle = wholeMsgMethodHandle.bindTo(handler); diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java index 659fd033a41c..679d653960f8 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java @@ -124,7 +124,7 @@ public abstract class JavaxWebSocketFrameHandlerFactory { try { - FILTER_RETURN_TYPE_METHOD = MethodHandles.lookup() + FILTER_RETURN_TYPE_METHOD = getServerMethodHandleLookup() .findVirtual(JavaxWebSocketSession.class, "filterReturnType", MethodType.methodType(void.class, Object.class)); } catch (Throwable e) @@ -322,16 +322,17 @@ public static MessageSink createMessageSink(JavaxWebSocketSession session, Messa try { + MethodHandles.Lookup lookup = getServerMethodHandleLookup(); if (DecodedMessageSink.class.isAssignableFrom(msgMetadata.sinkClass)) { - MethodHandle ctorHandle = MethodHandles.lookup().findConstructor(msgMetadata.sinkClass, + MethodHandle ctorHandle = lookup.findConstructor(msgMetadata.sinkClass, MethodType.methodType(void.class, CoreSession.class, msgMetadata.registeredDecoder.interfaceType, MethodHandle.class)); Decoder decoder = session.getDecoders().getInstanceOf(msgMetadata.registeredDecoder); return (MessageSink)ctorHandle.invoke(session.getCoreSession(), decoder, msgMetadata.handle); } else { - MethodHandle ctorHandle = MethodHandles.lookup().findConstructor(msgMetadata.sinkClass, + MethodHandle ctorHandle = lookup.findConstructor(msgMetadata.sinkClass, MethodType.methodType(void.class, CoreSession.class, MethodHandle.class)); return (MessageSink)ctorHandle.invoke(session.getCoreSession(), msgMetadata.handle); } @@ -388,7 +389,7 @@ private MethodHandle toMethodHandle(MethodHandles.Lookup lookup, Method method) protected JavaxWebSocketFrameHandlerMetadata createEndpointMetadata(Class endpointClass, EndpointConfig endpointConfig) { JavaxWebSocketFrameHandlerMetadata metadata = new JavaxWebSocketFrameHandlerMetadata(endpointConfig); - MethodHandles.Lookup lookup = getMethodHandleLookup(endpointClass); + MethodHandles.Lookup lookup = getApplicationMethodHandleLookup(endpointClass); Method openMethod = ReflectUtils.findMethod(endpointClass, "onOpen", javax.websocket.Session.class, javax.websocket.EndpointConfig.class); @@ -410,7 +411,7 @@ protected JavaxWebSocketFrameHandlerMetadata createEndpointMetadata(Class endpointClass, JavaxWebSocketFrameHandlerMetadata metadata) { - MethodHandles.Lookup lookup = getMethodHandleLookup(endpointClass); + MethodHandles.Lookup lookup = getApplicationMethodHandleLookup(endpointClass); Method onmethod; // OnOpen [0..1] @@ -704,9 +705,53 @@ private void assertSignatureValid(Class endpointClass, Method method, Class endpointClass) + /** + *

+ * Gives a {@link MethodHandles.Lookup} instance to be used to find methods in server classes. + * For lookups on application classes use {@link #getApplicationMethodHandleLookup(Class)} ()} instead. + *

+ *

+ * This uses the caller sensitive {@link MethodHandles#lookup()}, this will allow MethodHandle access + * to server classes we need to use and will give access permissions to private methods as well. + *

+ * + * @return + */ + public static MethodHandles.Lookup getServerMethodHandleLookup() + { + return MethodHandles.lookup(); + } + + /** + *

+ * Gives a {@link MethodHandles.Lookup} instance to be used to find public methods in application classes. + * For lookups on server classes use {@link #getServerMethodHandleLookup()} instead. + *

+ *

+ * This uses {@link MethodHandles#publicLookup()} as we only need access to public method of the lookupClass. + * To look up a method on the lookupClass, it must be public and the class must be accessible from this + * module, so if the lookupClass is in a JPMS module it must be exported so that the public methods + * of the lookupClass are accessible outside of the module. + *

+ *

+ * The {@link java.lang.invoke.MethodHandles.Lookup#in(Class)} allows us to search specifically + * in the endpoint Class to avoid any potential linkage errors which could occur if the same + * class is present in multiple web apps. Unlike using {@link MethodHandles#publicLookup()} + * using {@link MethodHandles#lookup()} with {@link java.lang.invoke.MethodHandles.Lookup#in(Class)} + * will cause the lookup to lose its public access to the lookup class if they are in different modules. + *

+ *

+ * {@link MethodHandles#privateLookupIn(Class, MethodHandles.Lookup)} is also unsuitable because it + * requires the caller module to read the target module, and the target module to open reflective + * access to the lookupClasses private methods. This is possible but requires extra configuration + * to provide private access which is not necessary for the purpose of accessing the public methods. + *

+ * @param lookupClass the desired lookup class for the new lookup object. + * @return a lookup object to be used to find methods on the lookupClass. + */ + public static MethodHandles.Lookup getApplicationMethodHandleLookup(Class lookupClass) { - return MethodHandles.publicLookup().in(endpointClass); + return MethodHandles.publicLookup().in(lookupClass); } private static class DecodedArgs diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryMessageSink.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryMessageSink.java index dae52a2e1981..7932300479ab 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryMessageSink.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryMessageSink.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.websocket.javax.common.messages; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.nio.ByteBuffer; import javax.websocket.CloseReason; @@ -28,6 +27,7 @@ import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.exception.CloseException; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.util.messages.ByteBufferMessageSink; import org.eclipse.jetty.websocket.util.messages.MessageSink; @@ -44,7 +44,7 @@ public DecodedBinaryMessageSink(CoreSession session, @Override protected MethodHandle newRawMethodHandle() throws NoSuchMethodException, IllegalAccessException { - return MethodHandles.lookup().findVirtual(DecodedBinaryMessageSink.class, + return JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedBinaryMessageSink.class, "onWholeMessage", MethodType.methodType(void.class, ByteBuffer.class)) .bindTo(this); } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java index 653f35491be0..32a736c4acb4 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java @@ -20,7 +20,6 @@ import java.io.InputStream; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import javax.websocket.CloseReason; import javax.websocket.DecodeException; @@ -28,6 +27,7 @@ import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.exception.CloseException; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.util.messages.InputStreamMessageSink; import org.eclipse.jetty.websocket.util.messages.MessageSink; @@ -44,7 +44,7 @@ public DecodedBinaryStreamMessageSink(CoreSession session, @Override protected MethodHandle newRawMethodHandle() throws NoSuchMethodException, IllegalAccessException { - return MethodHandles.lookup().findVirtual(DecodedBinaryStreamMessageSink.class, + return JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedBinaryStreamMessageSink.class, "onStreamStart", MethodType.methodType(void.class, InputStream.class)) .bindTo(this); } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java index 69c6f3005d75..593e48c945f2 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java @@ -19,7 +19,6 @@ package org.eclipse.jetty.websocket.javax.common.messages; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import javax.websocket.CloseReason; import javax.websocket.DecodeException; @@ -27,6 +26,7 @@ import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.exception.CloseException; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.util.messages.MessageSink; import org.eclipse.jetty.websocket.util.messages.StringMessageSink; @@ -43,7 +43,7 @@ public DecodedTextMessageSink(CoreSession session, @Override protected MethodHandle newRawMethodHandle() throws NoSuchMethodException, IllegalAccessException { - return MethodHandles.lookup().findVirtual(DecodedTextMessageSink.class, + return JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedTextMessageSink.class, "onWholeMessage", MethodType.methodType(void.class, String.class)) .bindTo(this); } diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java index 3955f14069f8..58a51a4a2e13 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java @@ -20,7 +20,6 @@ import java.io.Reader; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import javax.websocket.CloseReason; import javax.websocket.DecodeException; @@ -28,6 +27,7 @@ import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.exception.CloseException; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.util.messages.MessageSink; import org.eclipse.jetty.websocket.util.messages.ReaderMessageSink; @@ -44,7 +44,7 @@ public DecodedTextStreamMessageSink(CoreSession session, @Override protected MethodHandle newRawMethodHandle() throws NoSuchMethodException, IllegalAccessException { - return MethodHandles.lookup().findVirtual(DecodedTextStreamMessageSink.class, + return JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(DecodedTextStreamMessageSink.class, "onStreamStart", MethodType.methodType(void.class, Reader.class)) .bindTo(this); } diff --git a/jetty-websocket/websocket-javax-common/src/test/java/org/eclipse/jetty/websocket/javax/common/messages/AbstractMessageSinkTest.java b/jetty-websocket/websocket-javax-common/src/test/java/org/eclipse/jetty/websocket/javax/common/messages/AbstractMessageSinkTest.java index e056e88b9f47..2965cc662f38 100644 --- a/jetty-websocket/websocket-javax-common/src/test/java/org/eclipse/jetty/websocket/javax/common/messages/AbstractMessageSinkTest.java +++ b/jetty-websocket/websocket-javax-common/src/test/java/org/eclipse/jetty/websocket/javax/common/messages/AbstractMessageSinkTest.java @@ -19,11 +19,11 @@ package org.eclipse.jetty.websocket.javax.common.messages; import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.util.function.Consumer; import org.eclipse.jetty.websocket.javax.common.AbstractSessionTest; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; public abstract class AbstractMessageSinkTest extends AbstractSessionTest { @@ -34,7 +34,7 @@ public MethodHandle getAcceptHandle(Consumer copy, Class type) Class refc = copy.getClass(); String name = "accept"; MethodType methodType = MethodType.methodType(void.class, type); - MethodHandle handle = MethodHandles.lookup().findVirtual(refc, name, methodType); + MethodHandle handle = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(refc, name, methodType); return handle.bindTo(copy); } catch (NoSuchMethodException | IllegalAccessException e) diff --git a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/CompletableFutureMethodHandle.java b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/CompletableFutureMethodHandle.java index 3924b2a65528..c7008dea7801 100644 --- a/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/CompletableFutureMethodHandle.java +++ b/jetty-websocket/websocket-javax-tests/src/main/java/org/eclipse/jetty/websocket/javax/tests/CompletableFutureMethodHandle.java @@ -23,6 +23,7 @@ import java.lang.reflect.Method; import java.util.concurrent.CompletableFuture; +import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory; import org.eclipse.jetty.websocket.util.InvokerUtils; import org.eclipse.jetty.websocket.util.ReflectUtils; @@ -31,7 +32,8 @@ public class CompletableFutureMethodHandle public static MethodHandle of(Class type, CompletableFuture future) { Method method = ReflectUtils.findMethod(CompletableFuture.class, "complete", type); - MethodHandle completeHandle = InvokerUtils.mutatedInvoker(MethodHandles.lookup(), CompletableFuture.class, method, new InvokerUtils.Arg(type)); + MethodHandles.Lookup lookup = JavaxWebSocketFrameHandlerFactory.getServerMethodHandleLookup(); + MethodHandle completeHandle = InvokerUtils.mutatedInvoker(lookup, CompletableFuture.class, method, new InvokerUtils.Arg(type)); return completeHandle.bindTo(future); } } diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java index ef001076b9b4..51298633c1e4 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java @@ -157,7 +157,8 @@ public static MessageSink createMessageSink(MethodHandle msgHandle, Class endpointClass) { JettyWebSocketFrameHandlerMetadata metadata = new JettyWebSocketFrameHandlerMetadata(); - MethodHandles.Lookup lookup = getMethodHandleLookup(endpointClass); + MethodHandles.Lookup lookup = JettyWebSocketFrameHandlerFactory.getApplicationMethodHandleLookup(endpointClass); Method openMethod = ReflectUtils.findMethod(endpointClass, "onWebSocketConnect", Session.class); MethodHandle open = toMethodHandle(lookup, openMethod); @@ -273,7 +274,7 @@ private JettyWebSocketFrameHandlerMetadata createAnnotatedMetadata(WebSocket ann metadata.setIdleTimeout(Duration.ofMillis(max)); metadata.setBatchMode(anno.batchMode()); - MethodHandles.Lookup lookup = getMethodHandleLookup(endpointClass); + MethodHandles.Lookup lookup = getApplicationMethodHandleLookup(endpointClass); Method onmethod; // OnWebSocketConnect [0..1] @@ -456,9 +457,53 @@ private void assertSignatureValid(Class endpointClass, Method method, Class endpointClass) + /** + *

+ * Gives a {@link MethodHandles.Lookup} instance to be used to find methods in server classes. + * For lookups on application classes use {@link #getApplicationMethodHandleLookup(Class)} ()} instead. + *

+ *

+ * This uses the caller sensitive {@link MethodHandles#lookup()}, this will allow MethodHandle access + * to server classes we need to use and will give access permissions to private methods as well. + *

+ * + * @return + */ + public static MethodHandles.Lookup getServerMethodHandleLookup() { - return MethodHandles.publicLookup().in(endpointClass); + return MethodHandles.lookup(); + } + + /** + *

+ * Gives a {@link MethodHandles.Lookup} instance to be used to find public methods in application classes. + * For lookups on server classes use {@link #getServerMethodHandleLookup()} instead. + *

+ *

+ * This uses {@link MethodHandles#publicLookup()} as we only need access to public method of the lookupClass. + * To look up a method on the lookupClass, it must be public and the class must be accessible from this + * module, so if the lookupClass is in a JPMS module it must be exported so that the public methods + * of the lookupClass are accessible outside of the module. + *

+ *

+ * The {@link java.lang.invoke.MethodHandles.Lookup#in(Class)} allows us to search specifically + * in the endpoint Class to avoid any potential linkage errors which could occur if the same + * class is present in multiple web apps. Unlike using {@link MethodHandles#publicLookup()} + * using {@link MethodHandles#lookup()} with {@link java.lang.invoke.MethodHandles.Lookup#in(Class)} + * will cause the lookup to lose its public access to the lookup class if they are in different modules. + *

+ *

+ * {@link MethodHandles#privateLookupIn(Class, MethodHandles.Lookup)} is also unsuitable because it + * requires the caller module to read the target module, and the target module to open reflective + * access to the lookupClasses private methods. This is possible but requires extra configuration + * to provide private access which is not necessary for the purpose of accessing the public methods. + *

+ * @param lookupClass the desired lookup class for the new lookup object. + * @return a lookup object to be used to find methods on the lookupClass. + */ + public static MethodHandles.Lookup getApplicationMethodHandleLookup(Class lookupClass) + { + return MethodHandles.publicLookup().in(lookupClass); } @Override diff --git a/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java b/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java index 7976f7d66ebb..366827f169f1 100644 --- a/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java +++ b/jetty-websocket/websocket-jetty-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java @@ -52,7 +52,7 @@ public class OutgoingMessageCapture extends CoreSession.Empty implements CoreSes public OutgoingMessageCapture() { - MethodHandles.Lookup lookup = MethodHandles.lookup(); + MethodHandles.Lookup lookup = JettyWebSocketFrameHandlerFactory.getApplicationMethodHandleLookup(this.getClass()); try { MethodHandle text = lookup.findVirtual(this.getClass(), "onWholeText", MethodType.methodType(Void.TYPE, String.class));