Issue #4691 - use static methods with javadoc to get MethodHandle lookup

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Mar 23, 2020 · 04cc21f · 04cc21f
1 parent d4c2893
commit 04cc21f
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 26 deletions.
diff --git a/...on/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java b/...on/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java
@@ -374,7 +374,7 @@ public <T> void addMessageHandler(JavaxWebSocketSession session, Class<T> 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 <T> void addMessageHandler(JavaxWebSocketSession session, Class<T> 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/...main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandlerFactory.java b/...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<? extends javax.websocket.Endpoint> 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<? exte
 
     protected JavaxWebSocketFrameHandlerMetadata discoverJavaxFrameHandlerMetadata(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<?
         }
     }
 
-    private MethodHandles.Lookup getMethodHandleLookup(Class<?> endpointClass)
+    /**
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     *
+     * @return
+     */
+    public static MethodHandles.Lookup getServerMethodHandleLookup()
+    {
+        return MethodHandles.lookup();
+    }
+
+    /**
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * {@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.
+     * </p>
+     * @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/...main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryMessageSink.java b/...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/...ava/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java b/...ava/org/eclipse/jetty/websocket/javax/common/messages/DecodedBinaryStreamMessageSink.java
@@ -20,14 +20,14 @@
 
 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;
 import javax.websocket.Decoder;
 
 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/...c/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java b/...c/main/java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextMessageSink.java
@@ -19,14 +19,14 @@
 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;
 import javax.websocket.Decoder;
 
 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/.../java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java b/.../java/org/eclipse/jetty/websocket/javax/common/messages/DecodedTextStreamMessageSink.java
@@ -20,14 +20,14 @@
 
 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;
 import javax.websocket.Decoder;
 
 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/.../test/java/org/eclipse/jetty/websocket/javax/common/messages/AbstractMessageSinkTest.java b/.../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 <T> MethodHandle getAcceptHandle(Consumer<T> copy, Class<T> 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/.../src/main/java/org/eclipse/jetty/websocket/javax/tests/CompletableFutureMethodHandle.java b/.../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 <T> MethodHandle of(Class<T> type, CompletableFuture<T> 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/...n/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java b/...n/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandlerFactory.java
@@ -157,7 +157,8 @@ public static MessageSink createMessageSink(MethodHandle msgHandle, Class<? exte
 
         try
         {
-            MethodHandle ctorHandle = MethodHandles.lookup().findConstructor(sinkClass,
+            MethodHandles.Lookup lookup = JettyWebSocketFrameHandlerFactory.getServerMethodHandleLookup();
+            MethodHandle ctorHandle = lookup.findConstructor(sinkClass,
                 MethodType.methodType(void.class, CoreSession.class, MethodHandle.class));
             return (MessageSink)ctorHandle.invoke(session.getCoreSession(), msgHandle);
         }
@@ -194,7 +195,7 @@ private MethodHandle toMethodHandle(MethodHandles.Lookup lookup, Method method)
     private JettyWebSocketFrameHandlerMetadata createListenerMetadata(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<?
         throw new InvalidSignatureException(err.toString());
     }
 
-    private MethodHandles.Lookup getMethodHandleLookup(Class<?> endpointClass)
+    /**
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     *
+     * @return
+     */
+    public static MethodHandles.Lookup getServerMethodHandleLookup()
     {
-        return MethodHandles.publicLookup().in(endpointClass);
+        return MethodHandles.lookup();
+    }
+
+    /**
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * 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.
+     * </p>
+     * <p>
+     * {@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.
+     * </p>
+     * @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-common/src/test/java/org/eclipse/jetty/websocket/common/OutgoingMessageCapture.java b/...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));