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

Jump to bottom

Issue #6328 - avoid binding WebSocket MethodHandles #10750

Open
wants to merge 6 commits into
base: jetty-12.0.x
Choose a base branch
from
Open

Issue #6328 - avoid binding WebSocket MethodHandles #10750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0509f55
add MethodHolder abstraction to avoid binding WebSocket MethodHandles
lachlan-roberts Oct 4, 2023
d3a2705
remove signature validation from MessageSinks
lachlan-roberts Oct 5, 2023
4cab677
fix MethodHolder doInvoke implementation
lachlan-roberts Oct 9, 2023
31d87e4
use MethodHolder in signature lookup of MessageSink constructor
lachlan-roberts Oct 16, 2023
42b95c6
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…
lachlan-roberts Oct 18, 2023
b6e3e87
fix jetty-jmh pom file after merge
lachlan-roberts Oct 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions ...e-common/src/main/java/org/eclipse/jetty/websocket/core/messages/AbstractMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;
import java.util.Objects;

import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>Abstract implementation of {@link MessageSink}.</p>
Expand All @@ -42,21 +42,21 @@
public abstract class AbstractMessageSink implements MessageSink
{
private final CoreSession session;
private final MethodHandle methodHandle;
private final MethodHolder methodHandle;
private final boolean autoDemand;

/**
* Creates a new {@link MessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke
* @param methodHolder the application function to invoke
* @param autoDemand whether this {@link MessageSink} manages demand automatically
* as explained in {@link AbstractMessageSink}
*/
public AbstractMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public AbstractMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
this.session = Objects.requireNonNull(session, "CoreSession");
this.methodHandle = Objects.requireNonNull(methodHandle, "MethodHandle");
this.methodHandle = Objects.requireNonNull(methodHolder, "MethodHolder");
this.autoDemand = autoDemand;
}

Expand All @@ -73,7 +73,7 @@ public CoreSession getCoreSession()
* Get the application function.
* @return the application function
*/
public MethodHandle getMethodHandle()
public MethodHolder getMethodHolder()
{
return methodHandle;
}
Expand Down
20 changes: 6 additions & 14 deletions ...-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteArrayMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,15 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
import java.nio.ByteBuffer;

import org.eclipse.jetty.io.ByteBufferCallbackAccumulator;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException;
import org.eclipse.jetty.websocket.core.exception.MessageTooLargeException;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A {@link MessageSink} implementation that accumulates BINARY frames
Expand All @@ -38,18 +36,12 @@ public class ByteArrayMessageSink extends AbstractMessageSink
* Creates a new {@link ByteArrayMessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke when a new message has been assembled
* @param methodHolder the application function to invoke when a new message has been assembled
* @param autoDemand whether this {@link MessageSink} manages demand automatically
*/
public ByteArrayMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public ByteArrayMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);

// This uses the offset length byte array signature not supported by jakarta websocket.
// The jakarta layer instead uses decoders for whole byte array messages instead of this message sink.
MethodType onMessageType = MethodType.methodType(Void.TYPE, byte[].class, int.class, int.class);
if (methodHandle.type().changeReturnType(void.class) != onMessageType.changeReturnType(void.class))
throw InvalidSignatureException.build(onMessageType, methodHandle.type());
super(session, methodHolder, autoDemand);
}

@Override
Expand All @@ -69,7 +61,7 @@ public void accept(Frame frame, Callback callback)
if (frame.isFin() && accumulator == null)
{
byte[] buf = BufferUtil.toArray(payload);
getMethodHandle().invoke(buf, 0, buf.length);
getMethodHolder().invoke(buf, 0, buf.length);
callback.succeeded();
autoDemand();
return;
Expand All @@ -91,7 +83,7 @@ public void accept(Frame frame, Callback callback)
// Do not complete twice the callback if the invocation fails.
callback = Callback.NOOP;
byte[] buf = accumulator.takeByteArray();
getMethodHandle().invoke(buf, 0, buf.length);
getMethodHolder().invoke(buf, 0, buf.length);
autoDemand();
}
else
Expand Down
30 changes: 8 additions & 22 deletions ...common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteBufferMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodType;
import java.nio.ByteBuffer;

import org.eclipse.jetty.io.ByteBufferCallbackAccumulator;
Expand All @@ -23,8 +21,8 @@
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException;
import org.eclipse.jetty.websocket.core.exception.MessageTooLargeException;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A {@link MessageSink} implementation that accumulates BINARY frames
Expand All @@ -39,24 +37,12 @@ public class ByteBufferMessageSink extends AbstractMessageSink
* Creates a new {@link ByteBufferMessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke when a new message has been assembled
* @param methodHolder the application function to invoke when a new message has been assembled
* @param autoDemand whether this {@link MessageSink} manages demand automatically
*/
public ByteBufferMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public ByteBufferMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
this(session, methodHandle, autoDemand, true);
}

protected ByteBufferMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand, boolean validateSignature)
{
super(session, methodHandle, autoDemand);

if (validateSignature)
{
MethodType onMessageType = MethodType.methodType(Void.TYPE, ByteBuffer.class);
if (methodHandle.type() != onMessageType)
throw InvalidSignatureException.build(onMessageType, methodHandle.type());
}
super(session, methodHolder, autoDemand);
}

@Override
Expand All @@ -74,7 +60,7 @@ public void accept(Frame frame, Callback callback)

if (frame.isFin() && accumulator == null)
{
invoke(getMethodHandle(), frame.getPayload(), callback);
invoke(getMethodHolder(), frame.getPayload(), callback);
autoDemand();
return;
}
Expand All @@ -97,7 +83,7 @@ public void accept(Frame frame, Callback callback)
ByteBuffer byteBuffer = buffer.getByteBuffer();
accumulator.writeTo(byteBuffer);
callback = Callback.from(buffer::release);
invoke(getMethodHandle(), byteBuffer, callback);
invoke(getMethodHolder(), byteBuffer, callback);
autoDemand();
}
else
Expand Down Expand Up @@ -126,9 +112,9 @@ public void fail(Throwable failure)
accumulator.fail(failure);
}

protected void invoke(MethodHandle methodHandle, ByteBuffer byteBuffer, Callback callback) throws Throwable
protected void invoke(MethodHolder methodHolder, ByteBuffer byteBuffer, Callback callback) throws Throwable
{
methodHandle.invoke(byteBuffer);
methodHolder.invoke(byteBuffer);
callback.succeeded();
}
}
7 changes: 4 additions & 3 deletions ...common/src/main/java/org/eclipse/jetty/websocket/core/messages/DispatchedMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A partial implementation of {@link MessageSink} for methods that consume WebSocket
Expand All @@ -49,9 +50,9 @@ public abstract class DispatchedMessageSink extends AbstractMessageSink
private volatile CompletableFuture<Void> dispatchComplete;
private MessageSink typeSink;

public DispatchedMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public DispatchedMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
if (!autoDemand)
throw new IllegalArgumentException("%s must be auto-demanding".formatted(getClass().getSimpleName()));
executor = session.getWebSocketComponents().getExecutor();
Expand All @@ -72,7 +73,7 @@ public void accept(Frame frame, final Callback callback)
{
try
{
getMethodHandle().invoke(typeSink);
getMethodHolder().invoke(typeSink);
if (typeSink instanceof Closeable closeable)
IO.close(closeable);
dispatchComplete.complete(null);
Expand Down
7 changes: 3 additions & 4 deletions ...ommon/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;

import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

public class InputStreamMessageSink extends DispatchedMessageSink
{
public InputStreamMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public InputStreamMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
}

@Override
Expand Down
11 changes: 5 additions & 6 deletions .../src/main/java/org/eclipse/jetty/websocket/core/messages/PartialByteArrayMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;

import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A {@link MessageSink} implementation that delivers BINARY frames
Expand All @@ -31,12 +30,12 @@ public class PartialByteArrayMessageSink extends AbstractMessageSink
* Creates a new {@link PartialByteArrayMessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke when a new frame has arrived
* @param methodHolder the application function to invoke when a new frame has arrived
* @param autoDemand whether this {@link MessageSink} manages demand automatically
*/
public PartialByteArrayMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public PartialByteArrayMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
}

@Override
Expand All @@ -47,7 +46,7 @@ public void accept(Frame frame, Callback callback)
if (frame.hasPayload() || frame.isFin())
{
byte[] buffer = BufferUtil.toArray(frame.getPayload());
getMethodHandle().invoke(buffer, frame.isFin());
getMethodHolder().invoke(buffer, frame.isFin());
callback.succeeded();
autoDemand();
}
Expand Down
13 changes: 7 additions & 6 deletions ...src/main/java/org/eclipse/jetty/websocket/core/messages/PartialByteBufferMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A {@link MessageSink} implementation that delivers BINARY frames
Expand All @@ -31,12 +32,12 @@ public class PartialByteBufferMessageSink extends AbstractMessageSink
* Creates a new {@link PartialByteBufferMessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke when a new frame has arrived
* @param methodHolder the application function to invoke when a new frame has arrived
* @param autoDemand whether this {@link MessageSink} manages demand automatically
*/
public PartialByteBufferMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public PartialByteBufferMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
}

@Override
Expand All @@ -46,7 +47,7 @@ public void accept(Frame frame, Callback callback)
{
if (frame.hasPayload() || frame.isFin())
{
invoke(getMethodHandle(), frame.getPayload(), frame.isFin(), callback);
invoke(getMethodHolder(), frame.getPayload(), frame.isFin(), callback);
autoDemand();
}
else
Expand All @@ -61,9 +62,9 @@ public void accept(Frame frame, Callback callback)
}
}

protected void invoke(MethodHandle methodHandle, ByteBuffer byteBuffer, boolean fin, Callback callback) throws Throwable
protected void invoke(MethodHolder methodHolder, ByteBuffer byteBuffer, boolean fin, Callback callback) throws Throwable
{
methodHandle.invoke(byteBuffer, fin);
methodHolder.invoke(byteBuffer, fin);
callback.succeeded();
}
}
11 changes: 6 additions & 5 deletions ...mon/src/main/java/org/eclipse/jetty/websocket/core/messages/PartialStringMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.Frame;
import org.eclipse.jetty.websocket.core.exception.BadPayloadException;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

/**
* <p>A {@link MessageSink} implementation that delivers TEXT frames
Expand All @@ -34,12 +35,12 @@ public class PartialStringMessageSink extends AbstractMessageSink
* Creates a new {@link PartialStringMessageSink}.
*
* @param session the WebSocket session
* @param methodHandle the application function to invoke when a new frame has arrived
* @param methodHolder the application function to invoke when a new frame has arrived
* @param autoDemand whether this {@link MessageSink} manages demand automatically
*/
public PartialStringMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public PartialStringMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
}

@Override
Expand All @@ -55,12 +56,12 @@ public void accept(Frame frame, Callback callback)
if (frame.isFin())
{
String complete = accumulator.takeCompleteString(BadPayloadException.InvalidUtf8::new);
getMethodHandle().invoke(complete, true);
getMethodHolder().invoke(complete, true);
}
else
{
String partial = accumulator.takePartialString(BadPayloadException.InvalidUtf8::new);
getMethodHandle().invoke(partial, false);
getMethodHolder().invoke(partial, false);
}

callback.succeeded();
Expand Down
7 changes: 3 additions & 4 deletions ...ore-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@

package org.eclipse.jetty.websocket.core.messages;

import java.lang.invoke.MethodHandle;

import org.eclipse.jetty.websocket.core.CoreSession;
import org.eclipse.jetty.websocket.core.util.MethodHolder;

public class ReaderMessageSink extends DispatchedMessageSink
{
public ReaderMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand)
public ReaderMessageSink(CoreSession session, MethodHolder methodHolder, boolean autoDemand)
{
super(session, methodHandle, autoDemand);
super(session, methodHolder, autoDemand);
}

@Override
Expand Down