From 863a835171f3082613867b5d2e8555ae09e1a113 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 23 Jan 2025 12:09:39 +1100 Subject: [PATCH 01/31] fixes AvailableDecoders and AvailableEncoders Signed-off-by: Lachlan Roberts --- .../common/decoders/AvailableDecoders.java | 19 ------------- .../common/encoders/AvailableEncoders.java | 28 +------------------ 2 files changed, 1 insertion(+), 46 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java index d7eaa00fc068..67c47bb7e8a8 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java @@ -137,25 +137,6 @@ private void add(Class decoder, Class inte throw new InvalidWebSocketException(err); } - // Validate the decoder to be added against the existing registered decoders. - for (RegisteredDecoder registered : registeredDecoders) - { - if (!registered.primitive && objectType.equals(registered.objectType)) - { - // Streaming decoders can only have one decoder per object type. - if (interfaceClass.equals(Decoder.TextStream.class) || interfaceClass.equals(Decoder.BinaryStream.class)) - throw new InvalidWebSocketException("Multiple decoders for objectType" + objectType); - - // If we have the same objectType, then the interfaceTypes must be the same to form a decoder list. - if (!registered.interfaceType.equals(interfaceClass)) - throw new InvalidWebSocketException("Multiple decoders with different interface types for objectType " + objectType); - } - - // If this decoder is already registered for this interface type we can skip adding a duplicate. - if (registered.decoder.equals(decoder) && registered.interfaceType.equals(interfaceClass)) - return; - } - registeredDecoders.add(new RegisteredDecoder(decoder, interfaceClass, objectType, config, components)); } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java index 49f0fe286d58..cb08e02b440e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java @@ -160,33 +160,7 @@ private void add(Class encoder, Class inte throw new InvalidWebSocketException(err.toString()); } - try - { - RegisteredEncoder conflicts = registeredEncoders.stream() - .filter(registered -> registered.isType(objectType)) - .filter(registered -> !registered.primitive) - .findFirst() - .get(); - - if (conflicts.encoder.equals(encoder) && conflicts.implementsInterface(interfaceClass)) - { - // Same encoder as what is there already, don't bother adding it again. - return; - } - - StringBuilder err = new StringBuilder(); - err.append("Duplicate Encoder Object type "); - err.append(objectType.getName()); - err.append(" in "); - err.append(encoder.getName()); - err.append(", previously declared in "); - err.append(conflicts.encoder.getName()); - throw new InvalidWebSocketException(err.toString()); - } - catch (NoSuchElementException e) - { - registeredEncoders.addFirst(new RegisteredEncoder(encoder, interfaceClass, objectType)); - } + registeredEncoders.addFirst(new RegisteredEncoder(encoder, interfaceClass, objectType)); } public List supporting(Class interfaceType) From 6356b1faf7877a422b146b7746c81490be69e637 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 30 Jan 2025 11:48:45 +1100 Subject: [PATCH 02/31] Allow parameterized types for jakarta websocket Decoders Signed-off-by: Lachlan Roberts --- .../websocket/core/util/ReflectUtils.java | 65 +++++++++++++++++++ .../JakartaWebSocketFrameHandlerFactory.java | 11 +++- .../common/decoders/AvailableDecoders.java | 3 +- .../common/decoders/RegisteredDecoder.java | 16 +++-- .../common/encoders/AvailableEncoders.java | 3 +- .../common/encoders/RegisteredEncoder.java | 16 +++-- .../messages/AbstractMessageSinkTest.java | 2 +- .../tests/matchers/IsMessageHandlerType.java | 2 +- .../IsMessageHandlerTypeRegistered.java | 2 +- 9 files changed, 101 insertions(+), 19 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java index 7b0d50bebf42..b0bbc756fc27 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java @@ -15,7 +15,9 @@ import java.lang.annotation.Annotation; import java.lang.invoke.MethodType; +import java.lang.reflect.Array; import java.lang.reflect.Constructor; +import java.lang.reflect.GenericArrayType; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; @@ -172,6 +174,15 @@ public static Class findGenericClassFor(Class baseClass, Class ifaceCla return null; } + public static Type findGenericTypeFor(Class baseClass, Class ifaceClass) + { + GenericRef ref = new GenericRef(baseClass, ifaceClass); + if (resolveGenericRef(ref, baseClass)) + return ref.genericType; + + return null; + } + private static int findTypeParameterIndex(Class clazz, TypeVariable needVar) { TypeVariable[] params = clazz.getTypeParameters(); @@ -368,4 +379,58 @@ public static void append(StringBuilder str, MethodType methodType) } str.append(")"); } + + /** + * Check if a type is assignable from another type. + * This only handles Class, ParameterizedType, and GenericArrayType, and does not handle wildcard types or type variables. + * + * @param superType the superType. + * @param subType the subType. + * @return true if the superType is assignable from the subType. + */ + public static boolean isAssignableFrom(Type superType, Type subType) + { + if (superType instanceof Class superClass && subType instanceof Class subClass) + return superClass.isAssignableFrom(subClass); + + if (superType instanceof ParameterizedType pSuperType && subType instanceof ParameterizedType pSubType) + { + if (!((Class)pSubType.getRawType()).isAssignableFrom((Class)pSuperType.getRawType())) + return false; + + Type[] subTypeArgs = pSubType.getActualTypeArguments(); + Type[] superTypeArgs = pSuperType.getActualTypeArguments(); + if (subTypeArgs.length != superTypeArgs.length) + return false; + + for (int i = 0; i < subTypeArgs.length; i++) + { + if (!isAssignableFrom(subTypeArgs[i], superTypeArgs[i])) + return false; + } + return true; + } + + if (superType instanceof GenericArrayType superTypeArray && subType instanceof GenericArrayType subTypeArray) + return isAssignableFrom(superTypeArray.getGenericComponentType(), subTypeArray.getGenericComponentType()); + + return false; + } + + public static Class getClassFromType(Type type) + { + if (type instanceof Class) + return (Class)type; + + if (type instanceof ParameterizedType) + return (Class)((ParameterizedType)type).getRawType(); + + if (type instanceof GenericArrayType gType) + { + Class componentClass = getClassFromType(gType.getGenericComponentType()); + return componentClass != null ? Array.newInstance(componentClass, 0).getClass() : null; + } + + return null; + } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index dd90ab38a138..3fb5c785c320 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -20,6 +20,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; +import java.lang.reflect.Type; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; @@ -71,6 +72,11 @@ public abstract class JakartaWebSocketFrameHandlerFactory } } + static InvokerUtils.Arg[] getArgsFor(Type objectType) + { + return getArgsFor(ReflectUtils.getClassFromType(objectType)); + } + static InvokerUtils.Arg[] getArgsFor(Class objectType) { return new InvokerUtils.Arg[]{new InvokerUtils.Arg(Session.class), new InvokerUtils.Arg(objectType).required()}; @@ -416,12 +422,13 @@ private boolean matchDecoders(Method onMsg, JakartaWebSocketFrameHandlerMetadata msgMetadata.setRegisteredDecoders(decoders); // Get the general methodHandle which applies to all the decoders in the list. - Class objectType = firstDecoder.objectType; + Type objectType = firstDecoder.objectType; for (RegisteredDecoder decoder : decoders) { - if (decoder.objectType.isAssignableFrom(objectType)) + if (ReflectUtils.isAssignableFrom(objectType, decoder.objectType)) objectType = decoder.objectType; } + MethodHandle methodHandle = getMethodHandle.apply(getArgsFor(objectType)); msgMetadata.setMethodHandle(methodHandle); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java index 67c47bb7e8a8..d90699a5d7ef 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/AvailableDecoders.java @@ -16,6 +16,7 @@ import java.io.Closeable; import java.io.InputStream; import java.io.Reader; +import java.lang.reflect.Type; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Iterator; @@ -129,7 +130,7 @@ private void registerAll(List> decoders) private void add(Class decoder, Class interfaceClass) { - Class objectType = ReflectUtils.findGenericClassFor(decoder, interfaceClass); + Type objectType = ReflectUtils.findGenericTypeFor(decoder, interfaceClass); if (objectType == null) { String err = "Unknown Decoder Object type declared for interface " + diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java index afd9fd890723..c74f0a43fd54 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.ee10.websocket.jakarta.common.decoders; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Type; import jakarta.websocket.Decoder; import jakarta.websocket.EndpointConfig; @@ -22,6 +23,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.websocket.core.util.ReflectUtils.isAssignableFrom; + public class RegisteredDecoder { private static final Logger LOG = LoggerFactory.getLogger(RegisteredDecoder.class); @@ -30,19 +33,19 @@ public class RegisteredDecoder public final Class decoder; // The jakarta.websocket.Decoder.* type (eg: Decoder.Binary, Decoder.BinaryStream, Decoder.Text, Decoder.TextStream) public final Class interfaceType; - public final Class objectType; + public final Type objectType; public final boolean primitive; public final EndpointConfig config; private final WebSocketComponents components; private Decoder instance; - public RegisteredDecoder(Class decoder, Class interfaceType, Class objectType, EndpointConfig endpointConfig, WebSocketComponents components) + public RegisteredDecoder(Class decoder, Class interfaceType, Type objectType, EndpointConfig endpointConfig, WebSocketComponents components) { this(decoder, interfaceType, objectType, endpointConfig, components, false); } - public RegisteredDecoder(Class decoder, Class interfaceType, Class objectType, EndpointConfig endpointConfig, WebSocketComponents components, boolean primitive) + public RegisteredDecoder(Class decoder, Class interfaceType, Type objectType, EndpointConfig endpointConfig, WebSocketComponents components, boolean primitive) { this.decoder = decoder; this.interfaceType = interfaceType; @@ -57,11 +60,12 @@ public boolean implementsInterface(Class type) return interfaceType.isAssignableFrom(type); } - public boolean isType(Class type) + public boolean isType(Type type) { - return objectType.isAssignableFrom(type); + return isAssignableFrom(type, objectType); } + @SuppressWarnings("unchecked") public T getInstance() { if (instance == null) @@ -105,7 +109,7 @@ public String toString() str.append(RegisteredDecoder.class.getSimpleName()); str.append('[').append(decoder.getName()); str.append(',').append(interfaceType.getName()); - str.append(',').append(objectType.getName()); + str.append(',').append(objectType.getTypeName()); if (primitive) { str.append(",PRIMITIVE"); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java index cb08e02b440e..5c6b5831bcaa 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/AvailableEncoders.java @@ -15,6 +15,7 @@ import java.io.Closeable; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Type; import java.nio.ByteBuffer; import java.util.LinkedList; import java.util.List; @@ -149,7 +150,7 @@ public void registerAll(List> encoders) private void add(Class encoder, Class interfaceClass) { - Class objectType = ReflectUtils.findGenericClassFor(encoder, interfaceClass); + Type objectType = ReflectUtils.findGenericTypeFor(encoder, interfaceClass); if (objectType == null) { StringBuilder err = new StringBuilder(); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java index 57549f7a583b..e0d782ac49ae 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java @@ -13,26 +13,30 @@ package org.eclipse.jetty.ee10.websocket.jakarta.common.encoders; +import java.lang.reflect.Type; + import jakarta.websocket.Encoder; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.websocket.core.util.ReflectUtils.isAssignableFrom; + public class RegisteredEncoder { private static final Logger LOG = LoggerFactory.getLogger(RegisteredEncoder.class); public final Class encoder; public final Class interfaceType; - public final Class objectType; + public final Type objectType; public final boolean primitive; public Encoder instance; - public RegisteredEncoder(Class encoder, Class interfaceType, Class objectType) + public RegisteredEncoder(Class encoder, Class interfaceType, Type objectType) { this(encoder, interfaceType, objectType, false); } - public RegisteredEncoder(Class encoder, Class interfaceType, Class objectType, boolean primitive) + public RegisteredEncoder(Class encoder, Class interfaceType, Type objectType, boolean primitive) { this.encoder = encoder; this.interfaceType = interfaceType; @@ -45,9 +49,9 @@ public boolean implementsInterface(Class type) return interfaceType.isAssignableFrom(type); } - public boolean isType(Class type) + public boolean isType(Type type) { - return objectType.isAssignableFrom(type); + return isAssignableFrom(type, objectType); } public void destroyInstance() @@ -74,7 +78,7 @@ public String toString() str.append(RegisteredEncoder.class.getSimpleName()); str.append('[').append(encoder.getName()); str.append(',').append(interfaceType.getName()); - str.append(',').append(objectType.getName()); + str.append(',').append(objectType.getTypeName()); if (primitive) { str.append(",PRIMITIVE"); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractMessageSinkTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractMessageSinkTest.java index aa59d5114b2f..0b769960a282 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractMessageSinkTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractMessageSinkTest.java @@ -40,7 +40,7 @@ else if (Decoder.BinaryStream.class.isAssignableFrom(clazz)) else throw new IllegalStateException(); - return List.of(new RegisteredDecoder(clazz, interfaceType, objectType, ClientEndpointConfig.Builder.create().build(), components)); + return List.of(new RegisteredDecoder(clazz, interfaceType, objectType, ClientEndpointConfig.Builder.create().build(), components, false)); } public MethodHandle getAcceptHandle(Consumer copy, Class type) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerType.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerType.java index f75af1e32513..7a47ff72780f 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerType.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerType.java @@ -81,7 +81,7 @@ else if (MessageHandler.Partial.class.isAssignableFrom(handlerClass)) switch (expectedType) { case PONG: - return PongMessage.class.isAssignableFrom(registeredDecoder.objectType); + return registeredDecoder.isType(PongMessage.class); case BINARY: return (Decoder.Binary.class.isAssignableFrom(registeredDecoder.interfaceType) || Decoder.BinaryStream.class.isAssignableFrom(registeredDecoder.interfaceType)); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerTypeRegistered.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerTypeRegistered.java index 5c6c0ee0d6d8..2acd9e450cb5 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerTypeRegistered.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/matchers/IsMessageHandlerTypeRegistered.java @@ -63,7 +63,7 @@ protected boolean matchesSafely(JakartaWebSocketSession session) if (expectedType == MessageType.PONG) { - if (PongMessage.class.isAssignableFrom(registeredDecoder.objectType)) + if (registeredDecoder.isType(PongMessage.class)) return true; continue; } From c5a3c107ba9470684c61b0530491011e0f731706 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 30 Jan 2025 12:52:43 +1100 Subject: [PATCH 03/31] fixes for RegisteredDecoder/RegisteredEncoder isType Signed-off-by: Lachlan Roberts --- .../websocket/jakarta/common/decoders/RegisteredDecoder.java | 2 +- .../websocket/jakarta/common/encoders/RegisteredEncoder.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java index c74f0a43fd54..c16c612dd5ce 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/RegisteredDecoder.java @@ -62,7 +62,7 @@ public boolean implementsInterface(Class type) public boolean isType(Type type) { - return isAssignableFrom(type, objectType); + return isAssignableFrom(objectType, type); } @SuppressWarnings("unchecked") diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java index e0d782ac49ae..7321b1bd7dcd 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/encoders/RegisteredEncoder.java @@ -51,7 +51,7 @@ public boolean implementsInterface(Class type) public boolean isType(Type type) { - return isAssignableFrom(type, objectType); + return isAssignableFrom(objectType, type); } public void destroyInstance() From bbd9035e85b8c96fc9d3bdffff2cba42377bfe89 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 30 Jan 2025 13:24:07 +1100 Subject: [PATCH 04/31] throw DeploymentException instead of InvalidSignatureException for jakarta websocket Signed-off-by: Lachlan Roberts --- ...kartaWebSocketClientFrameHandlerFactory.java | 3 ++- .../internal/JakartaClientUpgradeRequest.java | 3 ++- .../common/JakartaWebSocketContainer.java | 3 ++- .../JakartaWebSocketFrameHandlerFactory.java | 17 +++++++++-------- ...bstractJakartaWebSocketFrameHandlerTest.java | 3 ++- .../common/DummyFrameHandlerFactory.java | 3 ++- ...kartaWebSocketServerFrameHandlerFactory.java | 13 +++++++++++-- 7 files changed, 30 insertions(+), 15 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientFrameHandlerFactory.java index 79cc374c41b5..4117bdf33143 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientFrameHandlerFactory.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.ee10.websocket.jakarta.client; import jakarta.websocket.ClientEndpoint; +import jakarta.websocket.DeploymentException; import jakarta.websocket.EndpointConfig; import org.eclipse.jetty.ee10.websocket.jakarta.client.internal.BasicClientEndpointConfig; import org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketContainer; @@ -40,7 +41,7 @@ public EndpointConfig newDefaultEndpointConfig(Class endpointClass) } @Override - public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) + public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) throws DeploymentException { if (jakarta.websocket.Endpoint.class.isAssignableFrom(endpointClass)) return createEndpointMetadata(endpointConfig); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/internal/JakartaClientUpgradeRequest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/internal/JakartaClientUpgradeRequest.java index fe5d9e72a5ba..ce7ea438bc98 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/internal/JakartaClientUpgradeRequest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/internal/JakartaClientUpgradeRequest.java @@ -16,6 +16,7 @@ import java.net.URI; import java.security.Principal; +import jakarta.websocket.DeploymentException; import org.eclipse.jetty.ee10.websocket.jakarta.client.JakartaWebSocketClientContainer; import org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketFrameHandler; import org.eclipse.jetty.ee10.websocket.jakarta.common.UpgradeRequest; @@ -27,7 +28,7 @@ public class JakartaClientUpgradeRequest extends CoreClientUpgradeRequest implem { private final JakartaWebSocketFrameHandler frameHandler; - public JakartaClientUpgradeRequest(JakartaWebSocketClientContainer clientContainer, WebSocketCoreClient coreClient, URI requestURI, Object websocketPojo) + public JakartaClientUpgradeRequest(JakartaWebSocketClientContainer clientContainer, WebSocketCoreClient coreClient, URI requestURI, Object websocketPojo) throws DeploymentException { super(coreClient, requestURI); frameHandler = clientContainer.newFrameHandler(websocketPojo, this); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java index c915d64cf816..45cf85175609 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketContainer.java @@ -22,6 +22,7 @@ import java.util.concurrent.Executor; import java.util.function.Consumer; +import jakarta.websocket.DeploymentException; import jakarta.websocket.Extension; import jakarta.websocket.WebSocketContainer; import org.eclipse.jetty.io.ByteBufferPool; @@ -155,7 +156,7 @@ public Set getOpenSessions() return sessionTracker.getSessions(); } - public JakartaWebSocketFrameHandler newFrameHandler(Object websocketPojo, UpgradeRequest upgradeRequest) + public JakartaWebSocketFrameHandler newFrameHandler(Object websocketPojo, UpgradeRequest upgradeRequest) throws DeploymentException { return getFrameHandlerFactory().newJakartaWebSocketFrameHandler(websocketPojo, upgradeRequest); } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index 3fb5c785c320..21c3c942d413 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -29,6 +29,7 @@ import jakarta.websocket.CloseReason; import jakarta.websocket.Decoder; +import jakarta.websocket.DeploymentException; import jakarta.websocket.Endpoint; import jakarta.websocket.EndpointConfig; import jakarta.websocket.OnClose; @@ -116,11 +117,11 @@ public JakartaWebSocketFrameHandlerFactory(JakartaWebSocketContainer container, this.paramIdentifier = paramIdentifier == null ? InvokerUtils.PARAM_IDENTITY : paramIdentifier; } - public abstract JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig); + public abstract JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) throws DeploymentException; public abstract EndpointConfig newDefaultEndpointConfig(Class endpointClass); - public JakartaWebSocketFrameHandler newJakartaWebSocketFrameHandler(Object endpointInstance, UpgradeRequest upgradeRequest) + public JakartaWebSocketFrameHandler newJakartaWebSocketFrameHandler(Object endpointInstance, UpgradeRequest upgradeRequest) throws DeploymentException { Object endpoint; EndpointConfig config; @@ -276,7 +277,7 @@ protected JakartaWebSocketFrameHandlerMetadata createEndpointMetadata(EndpointCo return metadata; } - protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetadata(Class endpointClass, JakartaWebSocketFrameHandlerMetadata metadata) + protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetadata(Class endpointClass, JakartaWebSocketFrameHandlerMetadata metadata) throws DeploymentException { MethodHandles.Lookup lookup = getApplicationMethodHandleLookup(endpointClass); Method onmethod; @@ -351,7 +352,7 @@ protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetada continue; // Not a valid @OnMessage declaration signature. - throw InvalidSignatureException.build(endpointClass, OnMessage.class, onMsg); + throw new DeploymentException("Invalid @OnMessage method signature", InvalidSignatureException.build(endpointClass, OnMessage.class, onMsg)); } } @@ -457,7 +458,7 @@ else if (interfaceType.equals(Decoder.BinaryStream.class)) return true; } - private void assertSignatureValid(Class endpointClass, Method method, Class annotationClass) + private void assertSignatureValid(Class endpointClass, Method method, Class annotationClass) throws DeploymentException { // Test modifiers int mods = method.getModifiers(); @@ -467,7 +468,7 @@ private void assertSignatureValid(Class endpointClass, Method method, Class endpointClass, Method method, Class endpointClass, Method method, Class(); } - protected JakartaWebSocketFrameHandler newJakartaFrameHandler(Object websocket) + protected JakartaWebSocketFrameHandler newJakartaFrameHandler(Object websocket) throws DeploymentException { JakartaWebSocketFrameHandlerFactory factory = container.getFrameHandlerFactory(); ConfiguredEndpoint endpoint = new ConfiguredEndpoint(websocket, endpointConfig); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/DummyFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/DummyFrameHandlerFactory.java index 93e5d0f0dfcf..c49c40968682 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/DummyFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/DummyFrameHandlerFactory.java @@ -15,6 +15,7 @@ import jakarta.websocket.ClientEndpoint; import jakarta.websocket.ClientEndpointConfig; +import jakarta.websocket.DeploymentException; import jakarta.websocket.EndpointConfig; import org.eclipse.jetty.websocket.core.util.InvokerUtils; @@ -32,7 +33,7 @@ public EndpointConfig newDefaultEndpointConfig(Class endpointClass) } @Override - public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) + public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) throws DeploymentException { if (jakarta.websocket.Endpoint.class.isAssignableFrom(endpointClass)) { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerFrameHandlerFactory.java index b3366ade61d2..06385c393c7e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerFrameHandlerFactory.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.ee10.websocket.jakarta.server; +import jakarta.websocket.DeploymentException; import jakarta.websocket.EndpointConfig; import jakarta.websocket.server.ServerEndpoint; import org.eclipse.jetty.ee10.websocket.jakarta.client.JakartaWebSocketClientFrameHandlerFactory; @@ -22,6 +23,7 @@ import org.eclipse.jetty.ee10.websocket.jakarta.server.internal.PathParamIdentifier; import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec; import org.eclipse.jetty.websocket.core.FrameHandler; +import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.eclipse.jetty.websocket.core.server.FrameHandlerFactory; import org.eclipse.jetty.websocket.core.server.ServerUpgradeRequest; import org.eclipse.jetty.websocket.core.server.ServerUpgradeResponse; @@ -34,7 +36,7 @@ public JakartaWebSocketServerFrameHandlerFactory(JakartaWebSocketContainer conta } @Override - public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) + public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, EndpointConfig endpointConfig) throws DeploymentException { if (jakarta.websocket.Endpoint.class.isAssignableFrom(endpointClass)) return createEndpointMetadata(endpointConfig); @@ -52,6 +54,13 @@ public JakartaWebSocketFrameHandlerMetadata getMetadata(Class endpointClass, @Override public FrameHandler newFrameHandler(Object websocketPojo, ServerUpgradeRequest upgradeRequest, ServerUpgradeResponse upgradeResponse) { - return newJakartaWebSocketFrameHandler(websocketPojo, new JakartaServerUpgradeRequest(upgradeRequest)); + try + { + return newJakartaWebSocketFrameHandler(websocketPojo, new JakartaServerUpgradeRequest(upgradeRequest)); + } + catch (DeploymentException e) + { + throw new InvalidWebSocketException(e.getMessage(), e); + } } } From 57393c1982c5c9d220633c2b055db638cd12f5b0 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 31 Jan 2025 14:09:04 +1100 Subject: [PATCH 05/31] add toString for InvokerUtils.Arg to help debugging Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/util/InvokerUtils.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java index 6fc71474e630..e869ef6d710c 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java @@ -113,6 +113,16 @@ public boolean isRequired() { return required; } + + @Override + public String toString() + { + return String.format("Arg[%s]", + type.getSimpleName() + + (name != null ? ", name=" + name : "") + + (required ? ", required" : "") + + (convertible ? ", convertible" : "")); + } } public interface ParamIdentifier From 673879159b4b370224783a40df0b1680dd1df54e Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 31 Jan 2025 16:27:39 +1100 Subject: [PATCH 06/31] fix InvokerUtils for non-existent pathParams Signed-off-by: Lachlan Roberts --- .../websocket/core/util/InvokerUtils.java | 158 +++++++++++------- .../util/InvokerUtilsStaticParamsTest.java | 97 ++++++++++- 2 files changed, 196 insertions(+), 59 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java index e869ef6d710c..9e5f636e87d8 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/InvokerUtils.java @@ -35,6 +35,7 @@ public static class Arg private boolean required = false; private boolean convertible = false; private Class convertedType; + boolean toBeRemoved = false; public Arg(Class type) { @@ -217,45 +218,69 @@ public static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class return mutatedInvoker(lookup, targetClass, true, method, paramIdentifier, namedVariables, callingArgs); } - private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class targetClass, boolean throwOnFailure, - Method method, ParamIdentifier paramIdentifier, - String[] namedVariables, Arg... rawCallingArgs) + private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, + Class targetClass, + boolean throwOnFailure, + Method method, + ParamIdentifier paramIdentifier, + String[] namedVariables, + Arg... rawCallingArgs) { Class[] parameterTypes = method.getParameterTypes(); - // Construct Actual Calling Args. - // This is the array of args, arriving as all the named variables (usually static in nature), - // then the raw calling arguments (very dynamic in nature) - Arg[] callingArgs = new Arg[rawCallingArgs.length + (namedVariables == null ? 0 : namedVariables.length)]; + // Build "parameterArgs" to represent the method's actual parameters. + // Where parameterArgs[0] always represents the targetClass instance. + boolean hasNamedParamArgs = false; + Arg[] parameterArgs = new Arg[parameterTypes.length + 1]; + parameterArgs[0] = new Arg(targetClass); + for (int i = 0; i < parameterTypes.length; i++) { - int callingArgIdx = 0; - if (namedVariables != null) - { - for (String namedVariable : namedVariables) - { - callingArgs[callingArgIdx++] = new Arg(String.class, namedVariable).convertible(); - } - } + Arg paramArg = paramIdentifier.getParamArg(method, parameterTypes[i], i); + if (paramArg.name != null) + hasNamedParamArgs = true; + parameterArgs[i + 1] = paramArg; + } + + // Construct the calling args array which combine namedVariables and rawCallingArgs. + int namedCount = (namedVariables == null) ? 0 : namedVariables.length; + Arg[] callingArgs = new Arg[namedCount + rawCallingArgs.length]; + int idx = 0; - for (Arg rawCallingArg : rawCallingArgs) + if (namedVariables != null) + { + for (String nv : namedVariables) { - callingArgs[callingArgIdx++] = rawCallingArg; + callingArgs[idx++] = new Arg(String.class, nv).convertible(); } } - // Build up Arg list representing the MethodHandle parameters - // ParamIdentifier is used to find named parameters (like jakarta.websocket's @PathParam declaration) - boolean hasNamedParamArgs = false; - Arg[] parameterArgs = new Arg[parameterTypes.length + 1]; - parameterArgs[0] = new Arg(targetClass); // first type is always the calling object instance type - for (int i = 0; i < parameterTypes.length; i++) + for (Arg rawArg : rawCallingArgs) { - Arg arg = paramIdentifier.getParamArg(method, parameterTypes[i], i); - if (arg.name != null) + callingArgs[idx++] = rawArg; + } + + // If a parameter arg is named but does not match a calling arg, it should be added to calling args + // but marked that it should be removed and given a null value. + for (Arg paramArg : parameterArgs) + { + if (paramArg.name == null) + continue; + + boolean found = false; + for (Arg callingArg : callingArgs) { - hasNamedParamArgs = true; + if (callingArg.matches(paramArg)) + { + found = true; + break; + } + } + if (!found) + { + callingArgs = Arrays.copyOf(callingArgs, callingArgs.length + 1); + callingArgs[callingArgs.length - 1] = paramArg; + paramArg.toBeRemoved = true; } - parameterArgs[i + 1] = arg; } // Parameter to Calling Argument mapping. @@ -273,20 +298,18 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class throw new InvalidSignatureException(err.toString()); } - // Establish MethodType for supplied calling args + // Establish MethodType for supplied calling args. boolean hasNamedCallingArgs = false; boolean hasConvertibleTypes = false; - List> cTypes = new ArrayList<>(); + List> callingTypes = new ArrayList<>(1 + callingArgs.length); + callingTypes.add(targetClass); // param0 = instance + for (Arg arg : callingArgs) { - cTypes.add(targetClass); // targetClass always at index 0 - for (Arg arg : callingArgs) - { - if (arg.name != null) - hasNamedCallingArgs = true; - if (arg.convertible) - hasConvertibleTypes = true; - cTypes.add(arg.getType()); - } + if (arg.name != null) + hasNamedCallingArgs = true; + if (arg.convertible) + hasConvertibleTypes = true; + callingTypes.add(arg.getType()); } try @@ -296,12 +319,12 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class // the calling 'refc' type of where the method is declared, not the targetClass. // That behavior of #unreflect() results in a MethodType referring to the // base/abstract/interface where the method is declared, and not the targetClass - MethodType callingType = MethodType.methodType(method.getReturnType(), cTypes); - MethodType rawType = MethodType.methodType(method.getReturnType(), method.getParameterTypes()); + MethodType callingType = MethodType.methodType(method.getReturnType(), callingTypes); + MethodType rawType = MethodType.methodType(method.getReturnType(), parameterTypes); MethodHandle methodHandle = lookup.findVirtual(targetClass, method.getName(), rawType); // If callingType and rawType are the same (and there's no named args), - // then there's no need to reorder / permute / drop args + // then there's no need to reorder / permute / drop args. if (!hasNamedCallingArgs && !hasNamedParamArgs && rawType.equals(callingType)) return methodHandle; @@ -318,24 +341,25 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class boolean[] usedCallingArgs = new boolean[callingArgs.length]; Arrays.fill(usedCallingArgs, false); - // Iterate through each parameterArg and attempt to find an associated callingArg + // Iterate through each parameterArg and attempt to find an associated callingArg. for (int pi = 1; pi < parameterArgs.length; pi++) { - int ref = -1; + int matchIndex = -1; + Arg paramArg = parameterArgs[pi]; - // Find a reference to argument in callArgs + // Look for an unused callingArg that matches this paramArg. for (int ci = 0; ci < callingArgs.length; ci++) { - if (!usedCallingArgs[ci] && callingArgs[ci].matches(parameterArgs[pi])) + if (!usedCallingArgs[ci] && callingArgs[ci].matches(paramArg)) { - ref = ci + 1; // add 1 to compensate for parameter 0 + matchIndex = ci + 1; // add 1 to compensate for parameter 0 usedCallingArgs[ci] = true; break; } } // Didn't find an unused callingArg that fits this parameterArg - if (ref < 0) + if (matchIndex < 0) { if (!throwOnFailure) return null; @@ -350,12 +374,11 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class throw new InvalidSignatureException(err.toString()); } - - reorderMap[pi] = ref; + reorderMap[pi] = matchIndex; } // Remaining unused callingArgs are to be placed at end of specified reorderMap - for (int ri = parameterArgs.length; ri <= reorderMap.length; ri++) + for (int ri = parameterArgs.length; ri < reorderMap.length; ) { for (int uci = 0; uci < usedCallingArgs.length; uci++) { @@ -381,7 +404,7 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class } } - // Drop excess (not mapped to a method parameter) calling args + // Drop excess (not mapped to a method parameter) calling args. int idxDrop = parameterArgs.length; int dropLength = reorderMap.length - idxDrop; if (dropLength > 0) @@ -398,19 +421,42 @@ private static MethodHandle mutatedInvoker(MethodHandles.Lookup lookup, Class if (hasConvertibleTypes) { // Use converted Types for callingArgs - cTypes = new ArrayList<>(); - cTypes.add(targetClass); // targetClass always at index 0 + callingTypes = new ArrayList<>(); + callingTypes.add(targetClass); // targetClass always at index 0 for (Arg arg : callingArgs) { - cTypes.add(arg.getConvertedType()); + callingTypes.add(arg.getConvertedType()); } - callingType = MethodType.methodType(method.getReturnType(), cTypes); + callingType = MethodType.methodType(method.getReturnType(), callingTypes); } // Reorder calling args to parameter args methodHandle = MethodHandles.permuteArguments(methodHandle, callingType, reorderMap); - // Return method handle + // Bind any named parameters not in the namedVariables list to be null. + // We go from the highest methodHandleIndex because insertArguments will remove that index. + for (int methodHandleIndex = reorderMap.length - 1; methodHandleIndex >= 0; methodHandleIndex--) + { + // find the index of the parameter arg + int parameterArgIndex = -1; + for (int j = 0; j < reorderMap.length; j++) + { + if (reorderMap[j] == methodHandleIndex) + { + parameterArgIndex = j; + break; + } + } + + // Now we know parameterArgIndex is the corresponding arg to this index on the methodHandle. + if (parameterArgIndex < parameterArgs.length) + { + Arg arg = parameterArgs[parameterArgIndex]; + if (arg.toBeRemoved) + methodHandle = MethodHandles.insertArguments(methodHandle, methodHandleIndex, (String)null); + } + } + return methodHandle; } catch (IllegalAccessException | NoSuchMethodException e) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/util/InvokerUtilsStaticParamsTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/util/InvokerUtilsStaticParamsTest.java index 578a5ae83e6b..f26d82c79cf7 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/util/InvokerUtilsStaticParamsTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/util/InvokerUtilsStaticParamsTest.java @@ -13,21 +13,28 @@ package org.eclipse.jetty.ee10.websocket.jakarta.common.util; +import java.lang.annotation.Annotation; import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.reflect.Method; import java.util.HashMap; import java.util.Map; +import jakarta.websocket.EndpointConfig; +import jakarta.websocket.OnOpen; import jakarta.websocket.Session; +import jakarta.websocket.server.PathParam; import org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketFrameHandlerFactory; import org.eclipse.jetty.util.annotation.Name; +import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException; import org.eclipse.jetty.websocket.core.util.InvokerUtils; import org.eclipse.jetty.websocket.core.util.ReflectUtils; import org.junit.jupiter.api.Test; +import static org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketFrameHandlerFactory.bindTemplateVariables; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNull; public class InvokerUtilsStaticParamsTest { @@ -36,7 +43,7 @@ public static class Foo { public String onFruit(@Name("fruit") String fruit) { - return String.format("onFruit('%s')", fruit); + return String.format("onFruit('%s')", (fruit == null) ? "null" : fruit); } public String onCount(@Name("count") int count) @@ -106,7 +113,7 @@ public void testOnlyParamInt() throws Throwable templateValues.put("count", "2222"); // Bind the static values for the variables, in same order as the variables were declared - methodHandle = JakartaWebSocketFrameHandlerFactory.bindTemplateVariables(methodHandle, namedVariables, templateValues); + methodHandle = bindTemplateVariables(methodHandle, namedVariables, templateValues); // Assign an instance to call. Foo foo = new Foo(); @@ -137,7 +144,7 @@ public void testLabeledParamStringInt() throws Throwable templateValues.put("count", "444"); // Bind the static values for the variables, in same order as the variables were declared - methodHandle = JakartaWebSocketFrameHandlerFactory.bindTemplateVariables(methodHandle, namedVariables, templateValues); + methodHandle = bindTemplateVariables(methodHandle, namedVariables, templateValues); // Assign an instance to call. Foo foo = new Foo(); @@ -147,4 +154,88 @@ public void testLabeledParamStringInt() throws Throwable String result = (String)methodHandle.invoke("cherry"); assertThat("Result", result, is("onLabeledCount('cherry', 444)")); } + + @Test + public void testNonExistentParams() throws Throwable + { + class PathParamEndpoint + { + private String pathParam; + private String nonExistent1; + private String nonExistent2; + private String nonExistent3; + + @OnOpen + public void onOpen(@PathParam("non-existent1") String nonExistent1, + EndpointConfig config, + @PathParam("non-existent2") String nonExistent2, + @PathParam("param1") String pathParam, + Session session, + @PathParam("non-existent3") String nonExistent3 + ) + { + this.pathParam = pathParam; + this.nonExistent1 = nonExistent1; + this.nonExistent2 = nonExistent2; + this.nonExistent3 = nonExistent3; + } + } + + Method method = ReflectUtils.findAnnotatedMethod(PathParamEndpoint.class, OnOpen.class); + + // Declared Variable Names + final String[] namedVariables = new String[]{ + "param1", + "param2", + "param3" + }; + + // Some point later an actual instance is needed, which has static named parameters + Map templateValues = new HashMap<>(); + templateValues.put("param1", "value1"); + templateValues.put("param2", "value2"); + templateValues.put("param3", "value3"); + + // Bind the static values, in same order as declared + final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); + final InvokerUtils.Arg ENDPOINT_CONFIG = new InvokerUtils.Arg(EndpointConfig.class); + MethodHandle methodHandle = InvokerUtils.mutatedInvoker(lookup, PathParamEndpoint.class, method, new PathParamIdentifier(), namedVariables, SESSION, ENDPOINT_CONFIG); + + methodHandle = bindTemplateVariables(methodHandle, namedVariables, templateValues); + + // Assign an instance to call. + PathParamEndpoint foo = new PathParamEndpoint(); + methodHandle = methodHandle.bindTo(foo).bindTo(null).bindTo(null); + + // Call method against instance + methodHandle.invoke(); + assertThat(foo.pathParam, is("value1")); + assertNull(foo.nonExistent1); + assertNull(foo.nonExistent2); + assertNull(foo.nonExistent3); + } + + @SuppressWarnings("unused") + public static class PathParamIdentifier implements InvokerUtils.ParamIdentifier + { + @Override + public InvokerUtils.Arg getParamArg(Method method, Class paramType, int idx) + { + Annotation[] annos = method.getParameterAnnotations()[idx]; + if (annos != null || (annos.length > 0)) + { + for (Annotation anno : annos) + { + if (anno.annotationType().equals(PathParam.class)) + { + if (!String.class.isAssignableFrom(paramType)) + throw new InvalidSignatureException("Unsupported PathParam Type: " + paramType); + PathParam pathParam = (PathParam)anno; + return new InvokerUtils.Arg(paramType, pathParam.value()); + } + } + } + return new InvokerUtils.Arg(paramType); + } + } } From 65070340ebfae5a4fc5c5ec7ee7f570c19763d11 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 31 Jan 2025 17:04:53 +1100 Subject: [PATCH 07/31] wrap return type of jakarta websocket PongMessage Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketFrameHandler.java | 2 + .../websocket/PongSocketStringReturn.java | 40 +++++++++++++++++++ .../jakarta/tests/server/PingPongTest.java | 16 ++++++++ 3 files changed, 58 insertions(+) create mode 100644 jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/com/acme/websocket/PongSocketStringReturn.java diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java index 86f110b4ed69..45baf3c08b39 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java @@ -140,6 +140,8 @@ public void onOpen(CoreSession coreSession, Callback callback) closeHandle = InvokerUtils.bindTo(closeHandle, session); errorHandle = InvokerUtils.bindTo(errorHandle, session); pongHandle = InvokerUtils.bindTo(pongHandle, session); + if (pongHandle != null) + pongHandle = JakartaWebSocketFrameHandlerFactory.wrapNonVoidReturnType(pongHandle, session); JakartaWebSocketMessageMetadata actualTextMetadata = JakartaWebSocketMessageMetadata.copyOf(textMetadata); if (actualTextMetadata != null) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/com/acme/websocket/PongSocketStringReturn.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/com/acme/websocket/PongSocketStringReturn.java new file mode 100644 index 000000000000..8503c57819cb --- /dev/null +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/com/acme/websocket/PongSocketStringReturn.java @@ -0,0 +1,40 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package com.acme.websocket; + +import jakarta.websocket.EndpointConfig; +import jakarta.websocket.OnMessage; +import jakarta.websocket.OnOpen; +import jakarta.websocket.PongMessage; +import jakarta.websocket.Session; +import jakarta.websocket.server.ServerEndpoint; +import org.eclipse.jetty.util.BufferUtil; + +@ServerEndpoint(value = "/pong-socket-string-return", configurator = PongContextListener.Config.class) +public class PongSocketStringReturn +{ + private String path; + + @OnOpen + public void onOpen(Session session, EndpointConfig config) + { + path = (String)config.getUserProperties().get("path"); + } + + @OnMessage + public String onPong(PongMessage pong) + { + return "PongSocket.onPong(PongMessage)[" + path + "]:" + BufferUtil.toString(pong.getApplicationData()); + } +} diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/PingPongTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/PingPongTest.java index 7e3d92ba4d79..0088e7f5bc50 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/PingPongTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/PingPongTest.java @@ -23,10 +23,12 @@ import com.acme.websocket.PongContextListener; import com.acme.websocket.PongMessageEndpoint; import com.acme.websocket.PongSocket; +import com.acme.websocket.PongSocketStringReturn; import org.eclipse.jetty.ee10.websocket.jakarta.tests.Timeouts; import org.eclipse.jetty.ee10.websocket.jakarta.tests.WSServer; import org.eclipse.jetty.ee10.websocket.jakarta.tests.framehandlers.FrameHandlerTracker; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; +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; @@ -56,6 +58,8 @@ public static void startServer() throws Exception app.copyClass(PongContextListener.class); app.copyClass(PongMessageEndpoint.class); app.copyClass(PongSocket.class); + app.copyClass(PongSocketStringReturn.class); + app.copyClass(BufferUtil.class); app.deploy(); server.start(); @@ -123,4 +127,16 @@ public void testPongSocket() throws Exception }, "PongSocket.onPong(PongMessage)[/pong-socket]:hello"); }); } + + @Test + public void testPongSocketReturnsString() throws Exception + { + assertTimeout(Duration.ofMillis(6000), () -> + { + assertEcho("/app/pong-socket-string-return", (session) -> + { + session.sendFrame(new Frame(OpCode.PONG).setPayload("hello"), Callback.NOOP, false); + }, "PongSocket.onPong(PongMessage)[/pong-socket-string-return]:hello"); + }); + } } From fe4bbf7c91b79df2cc1dde5b9fc8c42ddc26344b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Feb 2025 14:41:33 +1100 Subject: [PATCH 08/31] fix isOpen for the JakartaWebSocketSession Signed-off-by: Lachlan Roberts --- .../ee10/websocket/jakarta/common/JakartaWebSocketSession.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketSession.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketSession.java index 87e048b817db..869d5e9540e2 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketSession.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketSession.java @@ -540,7 +540,7 @@ public Map getUserProperties() @Override public boolean isOpen() { - return coreSession.isOutputOpen(); + return coreSession.isOutputOpen() || coreSession.isInputOpen(); } /** From 6b1b7dedac86342605d65f20befa19885ef95711 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Feb 2025 14:42:09 +1100 Subject: [PATCH 09/31] WebSocket Jakarta HandshakeRequest should include pathParams in getParameterMap Signed-off-by: Lachlan Roberts --- .../internal/JakartaWebSocketCreator.java | 2 + .../server/internal/JsrHandshakeRequest.java | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java index 994ec93a9162..32c783ab311d 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketContainer; import org.eclipse.jetty.ee10.websocket.jakarta.common.JakartaWebSocketExtension; import org.eclipse.jetty.ee10.websocket.jakarta.common.ServerEndpointConfigWrapper; +import org.eclipse.jetty.ee10.websocket.jakarta.server.JakartaWebSocketServerContainer; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec; @@ -143,6 +144,7 @@ public Map getUserProperties() // Wrap the config with the path spec information. config = new PathParamServerEndpointConfig(config, pathParams); + request.setAttribute(JakartaWebSocketServerContainer.PATH_PARAM_ATTRIBUTE, pathParams); } else { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JsrHandshakeRequest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JsrHandshakeRequest.java index 6bb10533fd28..9d98ad999a83 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JsrHandshakeRequest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JsrHandshakeRequest.java @@ -15,6 +15,8 @@ import java.net.URI; import java.security.Principal; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -61,13 +63,47 @@ public Map> getParameterMap() { if (parameterMap == null) { - Fields requestParams = Request.extractQueryParameters(delegate); parameterMap = new HashMap<>(); + + // Add query parameters to the parameter map. + Fields requestParams = Request.extractQueryParameters(delegate); for (String name : requestParams.getNames()) { - parameterMap.put(name, requestParams.getValues(name)); + parameterMap.compute(name, (key, values) -> + { + if (values == null) + values = new ArrayList<>(); + values.addAll(requestParams.getValues(name)); + return values; + }); + } + + // Add path parameters to the parameter map. + Map pathParams = getPathParams(); + if (pathParams != null) + { + for (Map.Entry entry : pathParams.entrySet()) + { + parameterMap.compute(entry.getKey(), (key, values) -> + { + if (values == null) + values = new ArrayList<>(); + values.add(entry.getValue()); + return values; + }); + } } + + // Make the lists unmodifiable. + for (Map.Entry> entry : parameterMap.entrySet()) + { + entry.setValue(Collections.unmodifiableList(entry.getValue())); + } + + // The map should be unmodifiable according to the spec. + parameterMap = Collections.unmodifiableMap(parameterMap); } + return parameterMap; } From 37bf06b682b240755bc5ce3bb6371a761d2c14b8 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Feb 2025 15:55:03 +1100 Subject: [PATCH 10/31] strip {} from pathParam values in PathParamIdentifier Signed-off-by: Lachlan Roberts --- .../jakarta/server/internal/PathParamIdentifier.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/PathParamIdentifier.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/PathParamIdentifier.java index 0439411b5813..fc0505d3500e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/PathParamIdentifier.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/PathParamIdentifier.java @@ -38,7 +38,11 @@ public InvokerUtils.Arg getParamArg(Method method, Class paramType, int idx) { validateType(paramType); PathParam pathParam = (PathParam)anno; - return new InvokerUtils.Arg(paramType, pathParam.value()); + String value = pathParam.value(); + // WebSocket TCK requires us to strip any remaining { and } from the value. + if (value != null && value.startsWith("{") && value.endsWith("}")) + value = value.substring(1, value.length() - 1); + return new InvokerUtils.Arg(paramType, value); } } } From 4c2d269a4bae450467b84a23dc1a73cb73288a0a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Feb 2025 17:20:47 +1100 Subject: [PATCH 11/31] allow jakarta websocket endpoints to continue after an error Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketFrameHandler.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java index 45baf3c08b39..976dddd51929 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java @@ -234,6 +234,23 @@ public Map getUserProperties() @Override public void onFrame(Frame frame, Callback callback) { + Callback frameCallback = callback; + callback = Callback.from(frameCallback::succeeded, x -> + { + // If it is a recoverable error, we can continue processing frames. + if (session.isOpen()) + { + if (x instanceof WebSocketException webSocketException) + x = webSocketException.getCause(); + + onError(x, frameCallback); + coreSession.demand(); + return; + } + + frameCallback.failed(x); + }); + switch (frame.getOpCode()) { case OpCode.TEXT: From d278e06702b9f3fed83fa6f8691a1169e8229ea0 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 Feb 2025 12:02:14 +1100 Subject: [PATCH 12/31] fix encoding issues with JakartaWebSocketAsyncRemote Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketAsyncRemote.java | 44 ++++++++----------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java index db15e132a9b8..4f1a42431709 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java @@ -93,81 +93,73 @@ public Future sendObject(Object data) return future; } - @SuppressWarnings( - {"rawtypes", "unchecked"}) + @SuppressWarnings({"rawtypes", "unchecked"}) @Override public void sendObject(Object data, SendHandler handler) { assertMessageNotNull(data); assertSendHandlerNotNull(handler); if (LOG.isDebugEnabled()) - { LOG.debug("sendObject({},{})", data, handler); - } Encoder encoder = session.getEncoders().getInstanceFor(data.getClass()); if (encoder == null) - { throw new IllegalArgumentException("No encoder for type: " + data.getClass()); - } - if (encoder instanceof Encoder.Text) + if (encoder instanceof Encoder.Text textEncoder) { - Encoder.Text etxt = (Encoder.Text)encoder; try { - String msg = etxt.encode(data); + String msg = textEncoder.encode(data); sendText(msg, handler); - return; } catch (EncodeException e) { handler.onResult(new SendResult(e)); } + return; } - else if (encoder instanceof Encoder.TextStream) + else if (encoder instanceof Encoder.TextStream textStreamEncoder) { - Encoder.TextStream etxt = (Encoder.TextStream)encoder; - SendHandlerCallback callback = new SendHandlerCallback(handler); - try (MessageWriter writer = newMessageWriter()) + try { - writer.setCallback(callback); - etxt.encode(data, writer); - return; + MessageWriter writer = newMessageWriter(); + writer.setCallback(new SendHandlerCallback(handler)); + textStreamEncoder.encode(data, writer); } catch (EncodeException | IOException e) { handler.onResult(new SendResult(e)); } + return; } - else if (encoder instanceof Encoder.Binary) + else if (encoder instanceof Encoder.Binary binaryEncoder) { - Encoder.Binary ebin = (Encoder.Binary)encoder; try { - ByteBuffer buf = ebin.encode(data); + ByteBuffer buf = binaryEncoder.encode(data); sendBinary(buf, handler); - return; } catch (EncodeException e) { handler.onResult(new SendResult(e)); } + return; } - else if (encoder instanceof Encoder.BinaryStream) + else if (encoder instanceof Encoder.BinaryStream binaryStreamEncoder) { - Encoder.BinaryStream ebin = (Encoder.BinaryStream)encoder; SendHandlerCallback callback = new SendHandlerCallback(handler); - try (MessageOutputStream out = newMessageOutputStream()) + try { + MessageOutputStream out = newMessageOutputStream(); out.setCallback(callback); - ebin.encode(data, out); - return; + binaryStreamEncoder.encode(data, out); } catch (EncodeException | IOException e) { handler.onResult(new SendResult(e)); } + return; } throw new IllegalArgumentException("Unknown encoder type: " + encoder); From 30143913c461e524ba2beef8052bf3d62f923fda Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 Feb 2025 19:27:58 +1100 Subject: [PATCH 13/31] jakarta remote endpoint classes should not send message in case of encoding failure Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketAsyncRemote.java | 2 + .../JakartaWebSocketRemoteEndpoint.java | 39 +++++++------------ 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java index 4f1a42431709..6c9ed7bb4e45 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java @@ -126,6 +126,7 @@ else if (encoder instanceof Encoder.TextStream textStreamEncoder) MessageWriter writer = newMessageWriter(); writer.setCallback(new SendHandlerCallback(handler)); textStreamEncoder.encode(data, writer); + writer.close(); } catch (EncodeException | IOException e) { @@ -154,6 +155,7 @@ else if (encoder instanceof Encoder.BinaryStream binaryStreamEncoder) MessageOutputStream out = newMessageOutputStream(); out.setCallback(callback); binaryStreamEncoder.encode(data, out); + out.close(); } catch (EncodeException | IOException e) { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java index afc024253466..86d35674a7c8 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java @@ -148,57 +148,48 @@ else if ((messageType == OpCode.TEXT) && (opcode == OpCode.BINARY)) } } + @SuppressWarnings({"rawtypes", "unchecked"}) public void sendObject(Object data, Callback callback) throws IOException, EncodeException { try { assertMessageNotNull(data); if (LOG.isDebugEnabled()) - { LOG.debug("sendObject({}, {})", data, callback); - } Encoder encoder = session.getEncoders().getInstanceFor(data.getClass()); if (encoder == null) - { throw new IllegalArgumentException("No encoder for type: " + data.getClass()); - } - if (encoder instanceof Encoder.Text) + if (encoder instanceof Encoder.Text textEncoder) { - Encoder.Text text = (Encoder.Text)encoder; - String msg = text.encode(data); + String msg = textEncoder.encode(data); sendFrame(new Frame(OpCode.TEXT).setPayload(msg), callback, batch); return; } - if (encoder instanceof Encoder.TextStream) + if (encoder instanceof Encoder.TextStream textStreamEncoder) { - Encoder.TextStream etxt = (Encoder.TextStream)encoder; - try (MessageWriter writer = newMessageWriter()) - { - writer.setCallback(callback); - etxt.encode(data, writer); - } + MessageWriter writer = newMessageWriter(); + writer.setCallback(callback); + textStreamEncoder.encode(data, writer); + writer.close(); return; } - if (encoder instanceof Encoder.Binary) + if (encoder instanceof Encoder.Binary binaryEncoder) { - Encoder.Binary ebin = (Encoder.Binary)encoder; - ByteBuffer buf = ebin.encode(data); + ByteBuffer buf = binaryEncoder.encode(data); sendFrame(new Frame(OpCode.BINARY).setPayload(buf), callback, batch); return; } - if (encoder instanceof Encoder.BinaryStream) + if (encoder instanceof Encoder.BinaryStream binaryStreamEncoder) { - Encoder.BinaryStream ebin = (Encoder.BinaryStream)encoder; - try (MessageOutputStream out = newMessageOutputStream()) - { - out.setCallback(callback); - ebin.encode(data, out); - } + MessageOutputStream out = newMessageOutputStream(); + out.setCallback(callback); + binaryStreamEncoder.encode(data, out); + out.close(); return; } From cb23776ac17ba94e6b05c028b787fbf136556bba Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 Feb 2025 19:28:30 +1100 Subject: [PATCH 14/31] give optional failure from errors for dispatched message sinks Signed-off-by: Lachlan Roberts --- .../core/messages/DispatchedMessageSink.java | 33 +++++++++++++++++-- .../core/messages/InputStreamMessageSink.java | 6 ++++ .../core/messages/ReaderMessageSink.java | 6 ++++ .../common/JakartaWebSocketFrameHandler.java | 18 ++++++++++ .../JakartaWebSocketFrameHandlerFactory.java | 11 ++++++- .../messages/AbstractDecodedMessageSink.java | 15 ++++++++- .../DecodedBinaryStreamMessageSink.java | 10 ++++-- .../DecodedTextStreamMessageSink.java | 10 ++++-- 8 files changed, 100 insertions(+), 9 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/DispatchedMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/DispatchedMessageSink.java index 7deb4ed60d86..a72ba0b83616 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/DispatchedMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/DispatchedMessageSink.java @@ -20,6 +20,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.IO; @@ -48,8 +50,10 @@ public abstract class DispatchedMessageSink extends AbstractMessageSink { private final Executor executor; + private final AtomicBoolean wasCallbackFailed = new AtomicBoolean(false); private volatile CompletableFuture dispatchComplete; private MessageSink typeSink; + private Consumer onError; public DispatchedMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand) { @@ -59,6 +63,15 @@ public DispatchedMessageSink(CoreSession session, MethodHandle methodHandle, boo executor = session.getWebSocketComponents().getExecutor(); } + public DispatchedMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand, Consumer onError) + { + super(session, methodHandle, autoDemand); + if (!autoDemand) + throw new IllegalArgumentException("%s must be auto-demanding".formatted(getClass().getSimpleName())); + this.executor = session.getWebSocketComponents().getExecutor(); + this.onError = onError; + } + public abstract MessageSink newMessageSink(); public void accept(Frame frame, final Callback callback) @@ -105,17 +118,31 @@ public void accept(Frame frame, final Callback callback) { autoDemand(); } - else + // We only need to handle the error here if none of the callbacks were ever failed. + else if (!wasCallbackFailed.get()) { if (failure instanceof CompletionException completionException) failure = completionException.getCause(); - CloseStatus closeStatus = new CloseStatus(CloseStatus.SERVER_ERROR, failure); - getCoreSession().close(closeStatus, Callback.NOOP); + if (onError == null) + { + CloseStatus closeStatus = new CloseStatus(CloseStatus.SERVER_ERROR, failure); + getCoreSession().close(closeStatus, Callback.NOOP); + } + else + { + onError.accept(failure); + } } }); } + frameCallback = Callback.from(frameCallback::succeeded, throwable -> + { + if (throwable != null) + wasCallbackFailed.set(true); + callback.failed(throwable); + }); typeSink.accept(frame, frameCallback); } diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java index 72c65802387f..efb82931164c 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.websocket.core.messages; import java.lang.invoke.MethodHandle; +import java.util.function.Consumer; import org.eclipse.jetty.websocket.core.CoreSession; @@ -24,6 +25,11 @@ public InputStreamMessageSink(CoreSession session, MethodHandle methodHandle, bo super(session, methodHandle, autoDemand); } + public InputStreamMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand, Consumer onError) + { + super(session, methodHandle, autoDemand, onError); + } + @Override public MessageSink newMessageSink() { diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java index 8f8e4f1759a1..fd38f23938e8 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.websocket.core.messages; import java.lang.invoke.MethodHandle; +import java.util.function.Consumer; import org.eclipse.jetty.websocket.core.CoreSession; @@ -24,6 +25,11 @@ public ReaderMessageSink(CoreSession session, MethodHandle methodHandle, boolean super(session, methodHandle, autoDemand); } + public ReaderMessageSink(CoreSession session, MethodHandle methodHandle, boolean autoDemand, Consumer onError) + { + super(session, methodHandle, autoDemand, onError); + } + @Override public MessageReader newMessageSink() { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java index 976dddd51929..412550bd12d9 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java @@ -36,6 +36,7 @@ import org.eclipse.jetty.ee10.websocket.jakarta.common.messages.DecodedBinaryStreamMessageSink; import org.eclipse.jetty.ee10.websocket.jakarta.common.messages.DecodedTextMessageSink; import org.eclipse.jetty.ee10.websocket.jakarta.common.messages.DecodedTextStreamMessageSink; +import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.thread.AutoLock; @@ -231,6 +232,23 @@ public Map getUserProperties() return wrappedConfig; } + public void onError(Throwable error) + { + try + { + Blocker.Callback callback = Blocker.callback(); + onError(error, callback); + callback.block(); + coreSession.demand(); + } + catch (Throwable t) + { + t.addSuppressed(error); + CloseStatus closeStatus = new CloseStatus(CloseStatus.SERVER_ERROR, t); + getSession().getCoreSession().close(closeStatus, Callback.NOOP); + } + } + @Override public void onFrame(Frame frame, Callback callback) { diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index 21c3c942d413..2e5973ec5a12 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.function.Consumer; import java.util.function.Function; import jakarta.websocket.CloseReason; @@ -194,7 +195,15 @@ public static MessageSink createMessageSink(JakartaWebSocketSession session, Jak try { MethodHandles.Lookup lookup = getServerMethodHandleLookup(); - if (AbstractDecodedMessageSink.class.isAssignableFrom(msgMetadata.getSinkClass())) + if (AbstractDecodedMessageSink.Stream.class.isAssignableFrom(msgMetadata.getSinkClass())) + { + MethodHandle ctorHandle = lookup.findConstructor(msgMetadata.getSinkClass(), + MethodType.methodType(void.class, CoreSession.class, MethodHandle.class, List.class, Consumer.class)); + List registeredDecoders = msgMetadata.getRegisteredDecoders(); + Consumer onError = session.getFrameHandler()::onError; + return (MessageSink)ctorHandle.invoke(session.getCoreSession(), msgMetadata.getMethodHandle(), registeredDecoders, onError); + } + else if (AbstractDecodedMessageSink.class.isAssignableFrom(msgMetadata.getSinkClass())) { MethodHandle ctorHandle = lookup.findConstructor(msgMetadata.getSinkClass(), MethodType.methodType(void.class, CoreSession.class, MethodHandle.class, List.class)); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java index a62d243fb891..f7ce57144f84 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java @@ -15,6 +15,7 @@ import java.lang.invoke.MethodHandle; import java.util.List; +import java.util.function.Consumer; import java.util.stream.Collectors; import jakarta.websocket.CloseReason; @@ -34,10 +35,17 @@ public abstract class AbstractDecodedMessageSink implements MessageSink private final MethodHandle _methodHandle; private final MessageSink _messageSink; + protected final Consumer _onError; public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHandle) + { + this(coreSession, methodHandle, null); + } + + public AbstractDecodedMessageSink(CoreSession coreSession, MethodHandle methodHandle, Consumer onError) { _methodHandle = methodHandle; + _onError = onError; try { @@ -107,7 +115,12 @@ public abstract static class Stream extends AbstractDecodedMe public Stream(CoreSession coreSession, MethodHandle methodHandle, List decoders) { - super(coreSession, methodHandle); + this(coreSession, methodHandle, decoders, null); + } + + public Stream(CoreSession coreSession, MethodHandle methodHandle, List decoders, Consumer onError) + { + super(coreSession, methodHandle, onError); if (decoders.size() != 1) throw new IllegalArgumentException("Require exactly one decoder for " + this.getClass()); _decoder = decoders.get(0).getInstance(); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedBinaryStreamMessageSink.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedBinaryStreamMessageSink.java index 6e7b34c3ce0a..9d7b3d01a27a 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedBinaryStreamMessageSink.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedBinaryStreamMessageSink.java @@ -18,6 +18,7 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodType; import java.util.List; +import java.util.function.Consumer; import jakarta.websocket.CloseReason; import jakarta.websocket.DecodeException; @@ -33,7 +34,12 @@ public class DecodedBinaryStreamMessageSink extends AbstractDecodedMessageSin { public DecodedBinaryStreamMessageSink(CoreSession session, MethodHandle methodHandle, List decoders) { - super(session, methodHandle, decoders); + this(session, methodHandle, decoders, null); + } + + public DecodedBinaryStreamMessageSink(CoreSession session, MethodHandle methodHandle, List decoders, Consumer onError) + { + super(session, methodHandle, decoders, onError); } @Override @@ -42,7 +48,7 @@ MessageSink newMessageSink(CoreSession coreSession) throws Exception MethodHandle methodHandle = JakartaWebSocketFrameHandlerFactory.getServerMethodHandleLookup() .findVirtual(DecodedBinaryStreamMessageSink.class, "onStreamStart", MethodType.methodType(void.class, InputStream.class)) .bindTo(this); - return new InputStreamMessageSink(coreSession, methodHandle, true); + return new InputStreamMessageSink(coreSession, methodHandle, true, _onError); } public void onStreamStart(InputStream stream) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedTextStreamMessageSink.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedTextStreamMessageSink.java index 59bed6e8b854..2b5d9be981d6 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedTextStreamMessageSink.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/DecodedTextStreamMessageSink.java @@ -18,6 +18,7 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodType; import java.util.List; +import java.util.function.Consumer; import jakarta.websocket.CloseReason; import jakarta.websocket.DecodeException; @@ -33,7 +34,12 @@ public class DecodedTextStreamMessageSink extends AbstractDecodedMessageSink. { public DecodedTextStreamMessageSink(CoreSession session, MethodHandle methodHandle, List decoders) { - super(session, methodHandle, decoders); + this(session, methodHandle, decoders, null); + } + + public DecodedTextStreamMessageSink(CoreSession session, MethodHandle methodHandle, List decoders, Consumer onError) + { + super(session, methodHandle, decoders, onError); } @Override @@ -42,7 +48,7 @@ MessageSink newMessageSink(CoreSession coreSession) throws Exception MethodHandle methodHandle = JakartaWebSocketFrameHandlerFactory.getServerMethodHandleLookup() .findVirtual(DecodedTextStreamMessageSink.class, "onStreamStart", MethodType.methodType(void.class, Reader.class)) .bindTo(this); - return new ReaderMessageSink(coreSession, methodHandle, true); + return new ReaderMessageSink(coreSession, methodHandle, true, _onError); } public void onStreamStart(Reader reader) From cf144cc87fbe9fbbb26777c79b10c1737bc9eb72 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 00:58:27 +1100 Subject: [PATCH 15/31] error handling fixes for Jakarta websocket RemoteEndpoints Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketAsyncRemote.java | 22 ++++++++++--------- .../JakartaWebSocketRemoteEndpoint.java | 6 +++++ 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java index 6c9ed7bb4e45..f47193f3f89f 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketAsyncRemote.java @@ -13,11 +13,9 @@ package org.eclipse.jetty.ee10.websocket.jakarta.common; -import java.io.IOException; import java.nio.ByteBuffer; import java.util.concurrent.Future; -import jakarta.websocket.EncodeException; import jakarta.websocket.Encoder; import jakarta.websocket.SendHandler; import jakarta.websocket.SendResult; @@ -86,6 +84,10 @@ public Future sendObject(Object data) { sendObject(data, future); } + catch (IllegalArgumentException e) + { + throw e; + } catch (Throwable t) { future.failed(t); @@ -113,9 +115,9 @@ public void sendObject(Object data, SendHandler handler) String msg = textEncoder.encode(data); sendText(msg, handler); } - catch (EncodeException e) + catch (Throwable t) { - handler.onResult(new SendResult(e)); + handler.onResult(new SendResult(t)); } return; } @@ -128,9 +130,9 @@ else if (encoder instanceof Encoder.TextStream textStreamEncoder) textStreamEncoder.encode(data, writer); writer.close(); } - catch (EncodeException | IOException e) + catch (Throwable t) { - handler.onResult(new SendResult(e)); + handler.onResult(new SendResult(t)); } return; } @@ -141,9 +143,9 @@ else if (encoder instanceof Encoder.Binary binaryEncoder) ByteBuffer buf = binaryEncoder.encode(data); sendBinary(buf, handler); } - catch (EncodeException e) + catch (Throwable t) { - handler.onResult(new SendResult(e)); + handler.onResult(new SendResult(t)); } return; } @@ -157,9 +159,9 @@ else if (encoder instanceof Encoder.BinaryStream binaryStreamEncoder) binaryStreamEncoder.encode(data, out); out.close(); } - catch (EncodeException | IOException e) + catch (Throwable t) { - handler.onResult(new SendResult(e)); + handler.onResult(new SendResult(t)); } return; } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java index 86d35674a7c8..42e0c6f609a8 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketRemoteEndpoint.java @@ -213,6 +213,9 @@ public void sendPing(ByteBuffer data) throws IOException, IllegalArgumentExcepti if (LOG.isDebugEnabled()) LOG.debug("sendPing({})", BufferUtil.toDetailString(data)); + if (BufferUtil.remaining(data) > Frame.MAX_CONTROL_PAYLOAD) + throw new IllegalArgumentException("Pong payload is too large"); + FutureCallback b = new FutureCallback(); sendFrame(new Frame(OpCode.PING).setPayload(data), b, batch); b.block(); @@ -224,6 +227,9 @@ public void sendPong(ByteBuffer data) throws IOException, IllegalArgumentExcepti if (LOG.isDebugEnabled()) LOG.debug("sendPong({})", BufferUtil.toDetailString(data)); + if (BufferUtil.remaining(data) > Frame.MAX_CONTROL_PAYLOAD) + throw new IllegalArgumentException("Pong payload is too large"); + FutureCallback b = new FutureCallback(); sendFrame(new Frame(OpCode.PONG).setPayload(data), b, batch); b.block(); From 5a8b039561070f3967ae39ff290e1c72c8999548 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 15:34:52 +1100 Subject: [PATCH 16/31] fixes for error handling in JakartaWebSocketFrameHandler Signed-off-by: Lachlan Roberts --- .../common/JakartaWebSocketFrameHandler.java | 27 ++++++++++++------- .../JakartaWebSocketFrameHandlerFactory.java | 2 +- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java index 412550bd12d9..f409e1efd6a8 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandler.java @@ -45,6 +45,7 @@ import org.eclipse.jetty.websocket.core.Frame; import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.core.OpCode; +import org.eclipse.jetty.websocket.core.exception.CloseException; import org.eclipse.jetty.websocket.core.exception.ProtocolException; import org.eclipse.jetty.websocket.core.exception.WebSocketException; import org.eclipse.jetty.websocket.core.messages.MessageSink; @@ -232,14 +233,12 @@ public Map getUserProperties() return wrappedConfig; } - public void onError(Throwable error) + public void handleError(Throwable error) { - try + try (Blocker.Callback callback = Blocker.callback()) { - Blocker.Callback callback = Blocker.callback(); onError(error, callback); callback.block(); - coreSession.demand(); } catch (Throwable t) { @@ -249,6 +248,20 @@ public void onError(Throwable error) } } + public void handleError(Throwable error, Callback callback) + { + Throwable unwrappedError = error; + if (unwrappedError instanceof WebSocketException webSocketException && webSocketException.getCause() != null) + unwrappedError = webSocketException.getCause(); + onError(unwrappedError, callback); + if (error instanceof CloseException closeException) + { + CloseStatus closeStatus = new CloseStatus(closeException.getStatusCode(), closeException); + getSession().getCoreSession().close(closeStatus, Callback.NOOP); + } + coreSession.demand(); + } + @Override public void onFrame(Frame frame, Callback callback) { @@ -258,11 +271,7 @@ public void onFrame(Frame frame, Callback callback) // If it is a recoverable error, we can continue processing frames. if (session.isOpen()) { - if (x instanceof WebSocketException webSocketException) - x = webSocketException.getCause(); - - onError(x, frameCallback); - coreSession.demand(); + handleError(x, frameCallback); return; } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index 2e5973ec5a12..748cde3f3da5 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -200,7 +200,7 @@ public static MessageSink createMessageSink(JakartaWebSocketSession session, Jak MethodHandle ctorHandle = lookup.findConstructor(msgMetadata.getSinkClass(), MethodType.methodType(void.class, CoreSession.class, MethodHandle.class, List.class, Consumer.class)); List registeredDecoders = msgMetadata.getRegisteredDecoders(); - Consumer onError = session.getFrameHandler()::onError; + Consumer onError = session.getFrameHandler()::handleError; return (MessageSink)ctorHandle.invoke(session.getCoreSession(), msgMetadata.getMethodHandle(), registeredDecoders, onError); } else if (AbstractDecodedMessageSink.class.isAssignableFrom(msgMetadata.getSinkClass())) From 290a61c459ea9506603c9bd72a20bfd4cb61e59e Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 15:52:03 +1100 Subject: [PATCH 17/31] allow ReflectUtils.isAssignableFrom to match class subtype with parameterized superType Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/websocket/core/util/ReflectUtils.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java index b0bbc756fc27..417faa0ab930 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java @@ -411,6 +411,9 @@ public static boolean isAssignableFrom(Type superType, Type subType) return true; } + if (superType instanceof ParameterizedType pSuperType && subType instanceof Class subClass) + return ((Class)pSuperType.getRawType()).isAssignableFrom(subClass); + if (superType instanceof GenericArrayType superTypeArray && subType instanceof GenericArrayType subTypeArray) return isAssignableFrom(superTypeArray.getGenericComponentType(), subTypeArray.getGenericComponentType()); From ba7c8ec3ca12307d12d756cb8c7606cb318d6ccd Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 16:04:27 +1100 Subject: [PATCH 18/31] make HttpFieldsMap.containsKey case-insensitive Signed-off-by: Lachlan Roberts --- .../org/eclipse/jetty/http/HttpFieldsMap.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFieldsMap.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFieldsMap.java index 1f66b95a0e72..680df4b73bb5 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFieldsMap.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFieldsMap.java @@ -98,6 +98,14 @@ public List remove(Object key) return null; } + @Override + public boolean containsKey(Object key) + { + if (key instanceof String s) + return httpFields.contains(s); + return false; + } + @Override public Set>> entrySet() { @@ -178,6 +186,14 @@ public List remove(Object key) throw new UnsupportedOperationException(); } + @Override + public boolean containsKey(Object key) + { + if (key instanceof String s) + return httpFields.contains(s); + return false; + } + @Override public Set>> entrySet() { From 3c3a4bd61fc9ccfc8b2158019a1c3e9dee08065a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 17:03:01 +1100 Subject: [PATCH 19/31] ensure jakarta websocket endpoints are cleared on deployment exception Signed-off-by: Lachlan Roberts --- .../JakartaWebSocketServerContainer.java | 99 ++++++++++++------- 1 file changed, 62 insertions(+), 37 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java index c576cb79e27d..124467471a8b 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -138,6 +138,7 @@ public String toString() private final JakartaWebSocketServerFrameHandlerFactory frameHandlerFactory; private List> deferredEndpointClasses; private List deferredEndpointConfigs; + private boolean failed = false; /** * Main entry point for {@link JakartaWebSocketServletContainerInitializer}. @@ -202,64 +203,88 @@ private void validateEndpointConfig(ServerEndpointConfig config) throws Deployme @Override public void addEndpoint(Class endpointClass) throws DeploymentException { - if (endpointClass == null) + try { - throw new DeploymentException("Unable to deploy null endpoint class"); - } + if (failed) + throw new DeploymentException("Previous endpoint failed to deploy"); - if (isStarted() || isStarting()) - { - ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class); - if (anno == null) + if (endpointClass == null) { - throw new DeploymentException(String.format("Class must be @%s annotated: %s", ServerEndpoint.class.getName(), endpointClass.getName())); + throw new DeploymentException("Unable to deploy null endpoint class"); } - if (LOG.isDebugEnabled()) + if (isStarted() || isStarting()) { - LOG.debug("addEndpoint({})", endpointClass); + ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class); + if (anno == null) + { + throw new DeploymentException(String.format("Class must be @%s annotated: %s", ServerEndpoint.class.getName(), endpointClass.getName())); + } + + if (LOG.isDebugEnabled()) + { + LOG.debug("addEndpoint({})", endpointClass); + } + + ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno); + validateEndpointConfig(config); + addEndpointMapping(config); + } + else + { + if (deferredEndpointClasses == null) + deferredEndpointClasses = new ArrayList<>(); + deferredEndpointClasses.add(endpointClass); } - - ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno); - validateEndpointConfig(config); - addEndpointMapping(config); } - else + catch (DeploymentException e) { - if (deferredEndpointClasses == null) - deferredEndpointClasses = new ArrayList<>(); - deferredEndpointClasses.add(endpointClass); + webSocketMappings.clear(); + failed = true; + throw e; } } @Override public void addEndpoint(ServerEndpointConfig providedConfig) throws DeploymentException { - if (providedConfig == null) - throw new DeploymentException("ServerEndpointConfig is null"); - - if (isStarted() || isStarting()) + try { - // Decorate the provided Configurator. - components.getObjectFactory().decorate(providedConfig.getConfigurator()); + if (failed) + throw new DeploymentException("Previous endpoint failed to deploy"); + + if (providedConfig == null) + throw new DeploymentException("ServerEndpointConfig is null"); + + if (isStarted() || isStarting()) + { + // Decorate the provided Configurator. + components.getObjectFactory().decorate(providedConfig.getConfigurator()); - // If we have annotations merge the annotated ServerEndpointConfig with the provided one. - Class endpointClass = providedConfig.getEndpointClass(); - ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class); - ServerEndpointConfig config = (anno == null) ? providedConfig - : new AnnotatedServerEndpointConfig(this, endpointClass, anno, providedConfig); + // If we have annotations merge the annotated ServerEndpointConfig with the provided one. + Class endpointClass = providedConfig.getEndpointClass(); + ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class); + ServerEndpointConfig config = (anno == null) ? providedConfig + : new AnnotatedServerEndpointConfig(this, endpointClass, anno, providedConfig); - if (LOG.isDebugEnabled()) - LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), endpointClass); + if (LOG.isDebugEnabled()) + LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), endpointClass); - validateEndpointConfig(config); - addEndpointMapping(config); + validateEndpointConfig(config); + addEndpointMapping(config); + } + else + { + if (deferredEndpointConfigs == null) + deferredEndpointConfigs = new ArrayList<>(); + deferredEndpointConfigs.add(providedConfig); + } } - else + catch (DeploymentException e) { - if (deferredEndpointConfigs == null) - deferredEndpointConfigs = new ArrayList<>(); - deferredEndpointConfigs.add(providedConfig); + webSocketMappings.clear(); + failed = true; + throw e; } } From 749243d7e1817babdcd78c6f08a85aece0a0c981 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Feb 2025 17:17:57 +1100 Subject: [PATCH 20/31] JakartaWebSocketServerContainer should throw DE when duplicate endpoint mappings are added Signed-off-by: Lachlan Roberts --- .../jakarta/server/JakartaWebSocketServerContainer.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java index 124467471a8b..ce879149ff1d 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -295,6 +295,8 @@ private void addEndpointMapping(ServerEndpointConfig config) throws DeploymentEx frameHandlerFactory.getMetadata(config.getEndpointClass(), config); JakartaWebSocketCreator creator = new JakartaWebSocketCreator(this, config, getExtensionRegistry()); PathSpec pathSpec = new UriTemplatePathSpec(config.getPath()); + if (webSocketMappings.getWebSocketNegotiator(pathSpec) != null) + throw new DeploymentException("Duplicate WebSocket mapping for path: " + config.getPath()); webSocketMappings.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer); } catch (InvalidSignatureException e) From b13dab494aaf504c1ed2972f01cc1bbecbe25295 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 7 Feb 2025 11:56:44 +1100 Subject: [PATCH 21/31] avoid finding overriden methods in InvokerUtils.findAnnotatedMethods Signed-off-by: Lachlan Roberts --- .../websocket/core/util/ReflectUtils.java | 53 +++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java index 417faa0ab930..2ab662dd183d 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/util/ReflectUtils.java @@ -24,9 +24,12 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Objects; +import java.util.Set; import java.util.regex.Pattern; -import java.util.stream.Stream; import org.eclipse.jetty.websocket.core.exception.DuplicateAnnotationException; @@ -143,19 +146,49 @@ public static Method findAnnotatedMethod(Class pojo, Class pojo, Class anno) { - Class clazz = pojo; - List methods = new ArrayList<>(); - while ((clazz != null) && Object.class.isAssignableFrom(clazz)) + Set seenSignatures = new HashSet<>(); + List annotatedMethods = new ArrayList<>(); + + for (Class clazz = pojo; (clazz != null) && Object.class.isAssignableFrom(clazz); clazz = clazz.getSuperclass()) { - Stream.of(clazz.getDeclaredMethods()) - .filter(method -> !method.isSynthetic() && (method.getAnnotation(anno) != null)) - .forEach(methods::add); - clazz = clazz.getSuperclass(); + for (Method method : clazz.getDeclaredMethods()) + { + if (method.isSynthetic() || method.getAnnotation(anno) == null) + continue; + if (seenSignatures.add(new MethodSignature(method))) + annotatedMethods.add(method); + } } - if (methods.isEmpty()) + if (annotatedMethods.isEmpty()) return null; - return methods.toArray(new Method[0]); + return annotatedMethods.toArray(new Method[0]); + } + + private static class MethodSignature + { + private final String name; + private final Class[] parameterTypes; + + MethodSignature(Method method) + { + this.name = method.getName(); + this.parameterTypes = method.getParameterTypes(); + } + + @Override + public boolean equals(Object o) + { + if (o instanceof MethodSignature that) + return Objects.equals(name, that.name) && Arrays.equals(parameterTypes, that.parameterTypes); + return false; + } + + @Override + public int hashCode() + { + return 31 * name.hashCode() + Arrays.hashCode(parameterTypes); + } } /** From 3a8d8d2bda42b93b4d7fe5fc2a938df1e8453626 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 7 Feb 2025 11:57:22 +1100 Subject: [PATCH 22/31] failures in finding methods in Jakarta websocket should result in deployment exception Signed-off-by: Lachlan Roberts --- .../JakartaWebSocketFrameHandlerFactory.java | 129 +++++++++--------- 1 file changed, 68 insertions(+), 61 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index 748cde3f3da5..f69f41a68009 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -48,7 +48,6 @@ import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.WebSocketComponents; -import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException; import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.eclipse.jetty.websocket.core.messages.MessageSink; import org.eclipse.jetty.websocket.core.messages.PartialByteArrayMessageSink; @@ -291,79 +290,87 @@ protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetada MethodHandles.Lookup lookup = getApplicationMethodHandleLookup(endpointClass); Method onmethod; - // OnOpen [0..1] - onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnOpen.class); - if (onmethod != null) - { - assertSignatureValid(endpointClass, onmethod, OnOpen.class); - final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); - final InvokerUtils.Arg ENDPOINT_CONFIG = new InvokerUtils.Arg(EndpointConfig.class); - MethodHandle methodHandle = InvokerUtils - .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, ENDPOINT_CONFIG); - metadata.setOpenHandler(methodHandle, onmethod); - } - - // OnClose [0..1] - onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnClose.class); - if (onmethod != null) + try { - assertSignatureValid(endpointClass, onmethod, OnClose.class); - final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); - final InvokerUtils.Arg CLOSE_REASON = new InvokerUtils.Arg(CloseReason.class); - MethodHandle methodHandle = InvokerUtils - .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, CLOSE_REASON); - metadata.setCloseHandler(methodHandle, onmethod); - } + // OnOpen [0..1] + onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnOpen.class); + if (onmethod != null) + { + assertSignatureValid(endpointClass, onmethod, OnOpen.class); + final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); + final InvokerUtils.Arg ENDPOINT_CONFIG = new InvokerUtils.Arg(EndpointConfig.class); + MethodHandle methodHandle = InvokerUtils + .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, ENDPOINT_CONFIG); + metadata.setOpenHandler(methodHandle, onmethod); + } - // OnError [0..1] - onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnError.class); - if (onmethod != null) - { - assertSignatureValid(endpointClass, onmethod, OnError.class); - final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); - final InvokerUtils.Arg CAUSE = new InvokerUtils.Arg(Throwable.class).required(); - MethodHandle methodHandle = InvokerUtils - .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, CAUSE); - metadata.setErrorHandler(methodHandle, onmethod); - } + // OnClose [0..1] + onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnClose.class); + if (onmethod != null) + { + assertSignatureValid(endpointClass, onmethod, OnClose.class); + final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); + final InvokerUtils.Arg CLOSE_REASON = new InvokerUtils.Arg(CloseReason.class); + MethodHandle methodHandle = InvokerUtils + .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, CLOSE_REASON); + metadata.setCloseHandler(methodHandle, onmethod); + } - // OnMessage [0..2] - Method[] onMessages = ReflectUtils.findAnnotatedMethods(endpointClass, OnMessage.class); - if (onMessages != null && onMessages.length > 0) - { - for (Method onMsg : onMessages) + // OnError [0..1] + onmethod = ReflectUtils.findAnnotatedMethod(endpointClass, OnError.class); + if (onmethod != null) { - assertSignatureValid(endpointClass, onMsg, OnMessage.class); - OnMessage onMessageAnno = onMsg.getAnnotation(OnMessage.class); + assertSignatureValid(endpointClass, onmethod, OnError.class); + final InvokerUtils.Arg SESSION = new InvokerUtils.Arg(Session.class); + final InvokerUtils.Arg CAUSE = new InvokerUtils.Arg(Throwable.class).required(); + MethodHandle methodHandle = InvokerUtils + .mutatedInvoker(lookup, endpointClass, onmethod, paramIdentifier, metadata.getNamedTemplateVariables(), SESSION, CAUSE); + metadata.setErrorHandler(methodHandle, onmethod); + } - long annotationMaxMessageSize = onMessageAnno.maxMessageSize(); - if (annotationMaxMessageSize > Integer.MAX_VALUE) + // OnMessage [0..2] + Method[] onMessages = ReflectUtils.findAnnotatedMethods(endpointClass, OnMessage.class); + if (onMessages != null && onMessages.length > 0) + { + for (Method onMsg : onMessages) { - throw new InvalidWebSocketException(String.format("Value too large: %s#%s - @OnMessage.maxMessageSize=%,d > Integer.MAX_VALUE", - endpointClass.getName(), onMsg.getName(), annotationMaxMessageSize)); - } + assertSignatureValid(endpointClass, onMsg, OnMessage.class); + OnMessage onMessageAnno = onMsg.getAnnotation(OnMessage.class); - // Create MessageMetadata and set annotated maxMessageSize if it is not the default value. - JakartaWebSocketMessageMetadata msgMetadata = new JakartaWebSocketMessageMetadata(); - if (annotationMaxMessageSize != -1) - msgMetadata.setMaxMessageSize((int)annotationMaxMessageSize); + long annotationMaxMessageSize = onMessageAnno.maxMessageSize(); + if (annotationMaxMessageSize > Integer.MAX_VALUE) + { + throw new InvalidWebSocketException(String.format("Value too large: %s#%s - @OnMessage.maxMessageSize=%,d > Integer.MAX_VALUE", + endpointClass.getName(), onMsg.getName(), annotationMaxMessageSize)); + } - // Function to search for matching MethodHandle for the endpointClass given a signature. - Function getMethodHandle = (signature) -> - InvokerUtils.optionalMutatedInvoker(lookup, endpointClass, onMsg, paramIdentifier, metadata.getNamedTemplateVariables(), signature); + // Create MessageMetadata and set annotated maxMessageSize if it is not the default value. + JakartaWebSocketMessageMetadata msgMetadata = new JakartaWebSocketMessageMetadata(); + if (annotationMaxMessageSize != -1) + msgMetadata.setMaxMessageSize((int)annotationMaxMessageSize); - // Try to match from available decoders (includes primitive types). - if (matchDecoders(onMsg, metadata, msgMetadata, getMethodHandle)) - continue; + // Function to search for matching MethodHandle for the endpointClass given a signature. + Function getMethodHandle = (signature) -> + InvokerUtils.optionalMutatedInvoker(lookup, endpointClass, onMsg, paramIdentifier, metadata.getNamedTemplateVariables(), signature); - // No decoders matched try partial signatures and pong signatures. - if (matchOnMessage(onMsg, metadata, msgMetadata, getMethodHandle)) - continue; + // Try to match from available decoders (includes primitive types). + if (matchDecoders(onMsg, metadata, msgMetadata, getMethodHandle)) + continue; - // Not a valid @OnMessage declaration signature. - throw new DeploymentException("Invalid @OnMessage method signature", InvalidSignatureException.build(endpointClass, OnMessage.class, onMsg)); + // No decoders matched try partial signatures and pong signatures. + if (matchOnMessage(onMsg, metadata, msgMetadata, getMethodHandle)) + continue; + } } } + catch (DeploymentException e) + { + throw e; + } + catch (Throwable t) + { + throw new DeploymentException("Failed to deploy endpoint", t); + } return metadata; } From c39fe8b049f8472ee60af9fc76ddac6f7a05fe9a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 7 Feb 2025 12:00:59 +1100 Subject: [PATCH 23/31] allow CharacterDecoder to decode all length>0 to char Signed-off-by: Lachlan Roberts --- .../jakarta/common/decoders/CharacterDecoder.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/CharacterDecoder.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/CharacterDecoder.java index 2da6e7b3da86..2980e62f2e71 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/CharacterDecoder.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/decoders/CharacterDecoder.java @@ -15,6 +15,7 @@ import jakarta.websocket.DecodeException; import jakarta.websocket.Decoder; +import org.eclipse.jetty.util.StringUtil; /** * Default implementation of the {@link jakarta.websocket.Decoder.Text} Message to {@link Character} decoder @@ -32,15 +33,6 @@ public Character decode(String s) throws DecodeException @Override public boolean willDecode(String s) { - if (s == null) - { - return false; - } - if (s.length() == 1) - { - return true; - } - // can only parse 1 character - return false; + return !StringUtil.isEmpty(s); } } From 3ead428b6c69e925d241a83c8e56adae5757df63 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 7 Feb 2025 16:49:48 +1100 Subject: [PATCH 24/31] calling MessageOutputStream close multiple times should not throw Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/messages/MessageOutputStream.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/MessageOutputStream.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/MessageOutputStream.java index 055a07f89c16..7024207901a5 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/MessageOutputStream.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/MessageOutputStream.java @@ -176,6 +176,9 @@ public void close() throws IOException { try { + if (closed) + return; + flush(true); buffer.release(); if (LOG.isDebugEnabled()) From f7161ae2ef703f2cb689ca6032c6e75ece45f65a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 10 Feb 2025 14:04:16 +1100 Subject: [PATCH 25/31] throw if Jakarta WebSocket onMessage is invalid Signed-off-by: Lachlan Roberts --- .../jakarta/common/JakartaWebSocketFrameHandlerFactory.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index f69f41a68009..a2edd1238d66 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -360,6 +360,8 @@ protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetada // No decoders matched try partial signatures and pong signatures. if (matchOnMessage(onMsg, metadata, msgMetadata, getMethodHandle)) continue; + + throw new InvalidWebSocketException("Unable to match @OnMessage method: " + onMsg); } } } From 02e913aa4ef5ea41dfe18bba2a6796fb841ab962 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 10 Feb 2025 16:56:01 +1100 Subject: [PATCH 26/31] failure to upgrade jakarta websocket should throw deployment exception Signed-off-by: Lachlan Roberts --- .../JakartaWebSocketClientContainer.java | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java index e9773e4a9153..6e91bf3cb85e 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-client/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/client/JakartaWebSocketClientContainer.java @@ -140,7 +140,7 @@ private CompletableFuture connect(JakartaClientUpgradeRequest upgradeRe { if (error != null) { - futureSession.completeExceptionally(convertCause(error)); + futureSession.completeExceptionally(error); return; } @@ -156,18 +156,6 @@ private CompletableFuture connect(JakartaClientUpgradeRequest upgradeRe return futureSession; } - public static Throwable convertCause(Throwable error) - { - if (error instanceof UpgradeException || - error instanceof WebSocketTimeoutException) - return new IOException(error); - - if (error instanceof InvalidWebSocketException) - return new DeploymentException(error.getMessage(), error); - - return error; - } - private Session connect(ConfiguredEndpoint configuredEndpoint, URI destURI) throws IOException, DeploymentException { if (configuredEndpoint == null) @@ -188,7 +176,7 @@ private Session connect(ConfiguredEndpoint configuredEndpoint, URI destURI) thro upgradeRequest.addExtensions(new JakartaWebSocketExtensionConfig(ext)); } - if (clientEndpointConfig.getPreferredSubprotocols().size() > 0) + if (!clientEndpointConfig.getPreferredSubprotocols().isEmpty()) upgradeRequest.setSubProtocols(clientEndpointConfig.getPreferredSubprotocols()); } @@ -207,6 +195,13 @@ private Session connect(ConfiguredEndpoint configuredEndpoint, URI destURI) thro throw (DeploymentException)cause; if (cause instanceof IOException) throw (IOException)cause; + if (cause instanceof UpgradeException) + throw new DeploymentException(cause.getMessage(), cause); + if (cause instanceof WebSocketTimeoutException) + throw new IOException(cause); + if (cause instanceof InvalidWebSocketException) + throw new DeploymentException(e.getMessage(), e); + throw new IOException(cause); } catch (TimeoutException e) @@ -215,7 +210,7 @@ private Session connect(ConfiguredEndpoint configuredEndpoint, URI destURI) thro } catch (Throwable e) { - throw new IOException("Unable to connect to " + destURI, e); + throw new DeploymentException("Unable to connect to " + destURI, e); } } From d13f0c1fdc67f6ba9132a08a950ba40c99a82a57 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 27 Feb 2025 18:35:35 +1100 Subject: [PATCH 27/31] Fix failures in Jetty tests due to fixes for WebSocket TCK Signed-off-by: Lachlan Roberts --- .../JakartaWebSocketFrameHandlerFactory.java | 3 +- .../messages/AbstractDecodedMessageSink.java | 2 -- ...ebSocketFrameHandlerBadSignaturesTest.java | 7 ++-- ...SocketFrameHandlerOnMessageBinaryTest.java | 9 +++-- ...ebSocketFrameHandlerOnMessageTextTest.java | 12 ++++--- .../JakartaWebSocketServerContainer.java | 4 +++ .../jakarta/tests/JakartaOnCloseTest.java | 8 +++-- .../ProgrammaticWebSocketUpgradeTest.java | 6 ++-- .../tests/coders/AvailableDecodersTest.java | 16 ++++++--- .../tests/coders/AvailableEncodersTest.java | 26 ++++++++------ .../jakarta/tests/coders/DecoderListTest.java | 35 ++++++++----------- .../tests/handlers/MessageHandlerTest.java | 9 +++-- .../jakarta/tests/server/DeploymentTest.java | 4 +-- 13 files changed, 83 insertions(+), 58 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java index a2edd1238d66..a79314216b97 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerFactory.java @@ -48,6 +48,7 @@ import org.eclipse.jetty.http.pathmap.UriTemplatePathSpec; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.WebSocketComponents; +import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException; import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.eclipse.jetty.websocket.core.messages.MessageSink; import org.eclipse.jetty.websocket.core.messages.PartialByteArrayMessageSink; @@ -361,7 +362,7 @@ protected JakartaWebSocketFrameHandlerMetadata discoverJakartaFrameHandlerMetada if (matchOnMessage(onMsg, metadata, msgMetadata, getMethodHandle)) continue; - throw new InvalidWebSocketException("Unable to match @OnMessage method: " + onMsg); + throw new InvalidSignatureException("Unable to match @OnMessage " + onMsg); } } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java index f7ce57144f84..c22633aa7697 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java @@ -121,8 +121,6 @@ public Stream(CoreSession coreSession, MethodHandle methodHandle, List decoders, Consumer onError) { super(coreSession, methodHandle, onError); - if (decoders.size() != 1) - throw new IllegalArgumentException("Require exactly one decoder for " + this.getClass()); _decoder = decoders.get(0).getInstance(); } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerBadSignaturesTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerBadSignaturesTest.java index a11729fc935d..2ea270b5337c 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerBadSignaturesTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerBadSignaturesTest.java @@ -15,6 +15,7 @@ import jakarta.websocket.ClientEndpoint; import jakarta.websocket.CloseReason; +import jakarta.websocket.DeploymentException; import jakarta.websocket.OnClose; import jakarta.websocket.OnError; import jakarta.websocket.OnOpen; @@ -24,14 +25,16 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; public class JakartaWebSocketFrameHandlerBadSignaturesTest extends AbstractJakartaWebSocketFrameHandlerTest { private void assertBadSocket(Object socket, String expectedString) throws Exception { - Exception e = assertThrows(InvalidSignatureException.class, () -> newJakartaFrameHandler(socket)); - assertThat(e.getMessage(), containsString(expectedString)); + Exception e = assertThrows(DeploymentException.class, () -> newJakartaFrameHandler(socket)); + assertThat(e.getCause(), instanceOf(InvalidSignatureException.class)); + assertThat(e.getCause().getMessage(), containsString(expectedString)); } @SuppressWarnings("UnusedParameters") diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageBinaryTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageBinaryTest.java index edec0cfe53c7..19dceb371067 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageBinaryTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageBinaryTest.java @@ -18,6 +18,7 @@ import java.util.concurrent.TimeUnit; import jakarta.websocket.ClientEndpoint; +import jakarta.websocket.DeploymentException; import jakarta.websocket.OnMessage; import jakarta.websocket.Session; import org.eclipse.jetty.ee10.websocket.jakarta.common.sockets.TrackingSocket; @@ -26,12 +27,14 @@ import org.eclipse.jetty.websocket.core.Frame; import org.eclipse.jetty.websocket.core.OpCode; import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException; +import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.hamcrest.Matcher; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -68,9 +71,10 @@ public void onMessage() @Test public void testInvokeMessage() throws Exception { - assertThrows(InvalidSignatureException.class, () -> + Throwable t = assertThrows(DeploymentException.class, () -> assertOnMessageInvocation(new MessageSocket(), containsString("onMessage()")) ); + assertThat(t.getCause(), instanceOf(InvalidWebSocketException.class)); } @ClientEndpoint @@ -103,13 +107,14 @@ public void onMessage(Session session) @Test public void testInvokeMessageSession() throws Exception { - assertThrows(InvalidSignatureException.class, () -> + Throwable t = assertThrows(DeploymentException.class, () -> assertOnMessageInvocation(new MessageSessionSocket(), allOf( containsString("onMessage(JakartaWebSocketSession@"), containsString(MessageSessionSocket.class.getName()) )) ); + assertThat(t.getCause(), instanceOf(InvalidSignatureException.class)); } @ClientEndpoint diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageTextTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageTextTest.java index 36779212d24e..11b73fad7f29 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageTextTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/common/JakartaWebSocketFrameHandlerOnMessageTextTest.java @@ -18,6 +18,7 @@ import java.util.concurrent.TimeUnit; import jakarta.websocket.ClientEndpoint; +import jakarta.websocket.DeploymentException; import jakarta.websocket.OnMessage; import jakarta.websocket.Session; import org.eclipse.jetty.ee10.websocket.jakarta.common.sockets.TrackingSocket; @@ -32,6 +33,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; public class JakartaWebSocketFrameHandlerOnMessageTextTest extends AbstractJakartaWebSocketFrameHandlerTest @@ -72,8 +74,9 @@ public void onMessage() public void testAmbiguousEmptyMessage() throws Exception { MessageSocket socket = new MessageSocket(); - Exception e = assertThrows(InvalidSignatureException.class, () -> onText(socket, "Hello World")); - assertThat(e.getMessage(), containsString("@OnMessage public void " + MessageSocket.class.getName() + "#onMessage")); + Exception e = assertThrows(DeploymentException.class, () -> onText(socket, "Hello World")); + assertThat(e.getCause(), instanceOf(InvalidSignatureException.class)); + assertThat(e.getCause().getMessage(), containsString("@OnMessage public void " + MessageSocket.class.getName() + ".onMessage")); } @ClientEndpoint @@ -109,8 +112,9 @@ public void onMessage(Session session) public void testAmbiguousMessageSession() throws Exception { MessageSessionSocket socket = new MessageSessionSocket(); - Exception e = assertThrows(InvalidSignatureException.class, () -> onText(socket, "Hello World")); - assertThat(e.getMessage(), containsString("@OnMessage public void " + MessageSessionSocket.class.getName() + "#onMessage")); + Exception e = assertThrows(DeploymentException.class, () -> onText(socket, "Hello World")); + assertThat(e.getCause(), instanceOf(InvalidSignatureException.class)); + assertThat(e.getCause().getMessage(), containsString("@OnMessage public void " + MessageSessionSocket.class.getName() + ".onMessage")); } @ClientEndpoint diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java index ce879149ff1d..2d610f8f5218 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java @@ -303,6 +303,10 @@ private void addEndpointMapping(ServerEndpointConfig config) throws DeploymentEx { throw new DeploymentException(e.getMessage(), e); } + catch (DeploymentException e) + { + throw e; + } catch (Throwable t) { throw new DeploymentException("Unable to deploy: " + config.getEndpointClass().getName(), t); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/JakartaOnCloseTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/JakartaOnCloseTest.java index d39c86c41e57..33ce73d0076b 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/JakartaOnCloseTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/JakartaOnCloseTest.java @@ -39,6 +39,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.emptyString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -213,16 +214,17 @@ public void onErrorOccurringAfterOnClose() throws Exception // Initiate close on client to cause the server to throw in onClose. clientEndpoint.session.close(); - // Test the receives the normal close, and throws in onClose. + // The server ignores the error and closes normally once close callback is succeeded. assertTrue(serverEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); assertThat(serverEndpoint.closeReason.getCloseCode(), is(CloseCodes.NORMAL_CLOSURE)); assertTrue(serverEndpoint.errorLatch.await(5, TimeUnit.SECONDS)); assertThat(serverEndpoint.error, instanceOf(RuntimeException.class)); assertThat(serverEndpoint.error.getMessage(), containsString("trigger onError from server onClose")); + // The client also ignores the close error and has normal close status. assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); - assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseCodes.UNEXPECTED_CONDITION)); - assertThat(clientEndpoint.closeReason.getReasonPhrase(), containsString("trigger onError from server onClose")); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseCodes.NORMAL_CLOSURE)); + assertThat(clientEndpoint.closeReason.getReasonPhrase(), emptyString()); assertTrue(clientEndpoint.errorLatch.await(5, TimeUnit.SECONDS)); assertThat(clientEndpoint.error, instanceOf(RuntimeException.class)); assertThat(clientEndpoint.error.getMessage(), containsString("trigger onError from client onClose")); diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ProgrammaticWebSocketUpgradeTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ProgrammaticWebSocketUpgradeTest.java index 8e806398f092..a60c4ad863cd 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ProgrammaticWebSocketUpgradeTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/ProgrammaticWebSocketUpgradeTest.java @@ -190,11 +190,11 @@ public void testWebSocketUpgradeFailure() throws Exception try { client.connectToServer(socket, uri); - fail("expected IOException"); + fail("expected Exception"); } - catch (IOException ioe) + catch (DeploymentException e) { - assertInstanceOf(UpgradeException.class, ioe.getCause()); + assertInstanceOf(UpgradeException.class, e.getCause()); } } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableDecodersTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableDecodersTest.java index 443b9bd61018..d936926efb75 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableDecodersTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableDecodersTest.java @@ -18,6 +18,7 @@ import java.util.Arrays; import java.util.Calendar; import java.util.Date; +import java.util.List; import java.util.NoSuchElementException; import java.util.TimeZone; @@ -34,11 +35,9 @@ import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.junit.jupiter.api.Assertions.assertThrows; public class AvailableDecodersTest { @@ -322,9 +321,16 @@ public void testCustomDecoderValidDualBinary() throws DecodeException @Test public void testCustomDecoderRegisterDuplicate() { - // has duplicated support for the same target Type - Exception e = assertThrows(InvalidWebSocketException.class, () -> init(BadDualDecoder.class)); - assertThat(e.getMessage(), containsString("Multiple decoders with different interface types")); + init(BadDualDecoder.class); + System.err.println(availableDecoders); + + List binaryDecoders = availableDecoders.getBinaryDecoders(Fruit.class); + assertThat(binaryDecoders.size(), is(1)); + assertThat(binaryDecoders.get(0).decoder, equalTo(BadDualDecoder.class)); + + List textDecoders = availableDecoders.getTextDecoders(Fruit.class); + assertThat(textDecoders.size(), is(1)); + assertThat(textDecoders.get(0).decoder, equalTo(BadDualDecoder.class)); } @Test diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableEncodersTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableEncodersTest.java index d8c66d337b7b..ded69cab92ad 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableEncodersTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/AvailableEncodersTest.java @@ -29,15 +29,13 @@ import org.eclipse.jetty.ee10.websocket.jakarta.common.encoders.IntegerEncoder; import org.eclipse.jetty.toolchain.test.Hex; import org.eclipse.jetty.websocket.core.WebSocketComponents; -import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -import static org.junit.jupiter.api.Assertions.assertThrows; public class AvailableEncodersTest { @@ -278,18 +276,26 @@ public void testCustomEncoderValidDualBinary() throws IllegalAccessException, In public void testCustomEncoderRegisterDuplicate() { // has duplicated support for the same target Type - Exception e = assertThrows(InvalidWebSocketException.class, () -> encoders.register(BadDualEncoder.class)); - assertThat(e.getMessage(), containsString("Duplicate")); + encoders.register(BadDualEncoder.class); + assertThat(encoders.getEncoderFor(Integer.class), equalTo(BadDualEncoder.class)); } @Test - public void testCustomEncoderRegisterOtherDuplicate() + public void testCustomEncoderRegisterOtherDuplicate() throws Exception { - // Register DateEncoder (decodes java.util.Date) + // Register TimeEncoder (decodes to java.util.Date) + encoders.register(TimeEncoder.class); + + // Register DateEncoder (which also wants to decode to java.util.Date) encoders.register(DateEncoder.class); - // Register TimeEncoder (which also wants to decode java.util.Date) - Exception e = assertThrows(InvalidWebSocketException.class, () -> encoders.register(TimeEncoder.class)); - assertThat(e.getMessage(), containsString("Duplicate")); + Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("GMT")); + calendar.set(Calendar.YEAR, 2016); + calendar.set(Calendar.MONTH, Calendar.AUGUST); + calendar.set(Calendar.DAY_OF_MONTH, 22); + Date val = calendar.getTime(); + + // Because of the duplicate the implementation selects just one of the Encoder implementations. + assertTextEncoder(Date.class, val, "2016.08.22"); } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/DecoderListTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/DecoderListTest.java index 8c7a8783ed94..05a26eedfcc5 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/DecoderListTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/coders/DecoderListTest.java @@ -39,7 +39,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.websocket.core.exception.InvalidWebSocketException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -48,10 +47,7 @@ import org.junit.jupiter.params.provider.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertThrows; public class DecoderListTest { @@ -174,25 +170,22 @@ public void testDecoderOrder() throws Exception } @Test - public void testStreamDecoders() + public void testStreamDecoders() throws Exception { - // Stream decoders will not be able to form a decoder list as they don't implement willDecode(). - Throwable error = assertThrows(Throwable.class, () -> - start(container -> - { - ServerEndpointConfig endpointConfig = ServerEndpointConfig.Builder.create(TextDecoderListEndpoint.class, "/") - .decoders(List.of(TextStreamDecoder1.class, TextStreamDecoder2.class)) - .build(); - container.addEndpoint(endpointConfig); - }) - ); + start(container -> + { + ServerEndpointConfig endpointConfig = ServerEndpointConfig.Builder.create(TextDecoderListEndpoint.class, "/") + .decoders(List.of(TextStreamDecoder1.class, TextStreamDecoder2.class)) + .build(); + container.addEndpoint(endpointConfig); + }); - assertThat(error, instanceOf(RuntimeException.class)); - Throwable cause = error.getCause(); - assertThat(cause, instanceOf(DeploymentException.class)); - Throwable invalidWebSocketException = cause.getCause(); - assertThat(invalidWebSocketException, instanceOf(InvalidWebSocketException.class)); - assertThat(invalidWebSocketException.getMessage(), containsString("Multiple decoders for objectTypeclass java.lang.String")); + // The TextStreamDecoder1 should be the one used as it was first in the list. + EventSocket clientEndpoint = new EventSocket(); + Session session = client.connectToServer(clientEndpoint, serverUri); + session.getBasicRemote().sendText("message"); + String response = clientEndpoint.textMessages.poll(3, TimeUnit.SECONDS); + assertThat(response, is("Decoder1: message")); } public static class TextDecoderListEndpoint extends Endpoint diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/handlers/MessageHandlerTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/handlers/MessageHandlerTest.java index b83a7f11a788..48245b020e39 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/handlers/MessageHandlerTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/handlers/MessageHandlerTest.java @@ -15,7 +15,8 @@ import java.net.URI; import java.nio.ByteBuffer; -import java.util.List; +import java.util.LinkedHashSet; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -76,9 +77,11 @@ public void before() throws Exception Stream argumentsStream = Stream.concat(getBinaryHandlers(), getTextHandlers()); for (Class c : getClassListFromArguments(argumentsStream)) { + System.err.println("deploying " + "/" + c.getSimpleName()); container.addEndpoint(ServerEndpointConfig.Builder.create(c, "/" + c.getSimpleName()).build()); } + System.err.println("deploying " + "/" + LongMessageHandler.class.getSimpleName()); container.addEndpoint(ServerEndpointConfig.Builder.create(LongMessageHandler.class, "/" + LongMessageHandler.class.getSimpleName()).build()); }); @@ -155,8 +158,8 @@ public void testLongDecoderHandler() throws Exception assertThat(clientEndpoint.closeReason.getReasonPhrase(), is("standard close")); } - private List> getClassListFromArguments(Stream stream) + private Set> getClassListFromArguments(Stream stream) { - return stream.map(arguments -> (Class)arguments.get()[0]).collect(Collectors.toList()); + return stream.map(arguments -> (Class)arguments.get()[0]).collect(Collectors.toCollection(LinkedHashSet::new)); } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentTest.java index 7e2cb0240759..0099076ada86 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentTest.java @@ -13,7 +13,6 @@ package org.eclipse.jetty.ee10.websocket.jakarta.tests.server; -import java.io.IOException; import java.nio.file.Path; import java.util.concurrent.TimeUnit; @@ -21,6 +20,7 @@ import jakarta.websocket.ContainerProvider; import jakarta.websocket.DecodeException; import jakarta.websocket.Decoder; +import jakarta.websocket.DeploymentException; import jakarta.websocket.EndpointConfig; import jakarta.websocket.OnMessage; import jakarta.websocket.OnOpen; @@ -83,7 +83,7 @@ public void testBadPathParamSignature() throws Exception Throwable error = assertThrows(Throwable.class, () -> client.connectToServer(clientSocket, server.getWsUri().resolve(app1.getContextPath() + "/badonclose/a"))); - assertThat(error, Matchers.instanceOf(IOException.class)); + assertThat(error, Matchers.instanceOf(DeploymentException.class)); assertThat(error.getMessage(), Matchers.containsString("503 Service Unavailable")); } From c1a7fc3c298f9938741daabd3cff3fe3fc3b24e2 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 4 Mar 2025 13:03:58 +1100 Subject: [PATCH 28/31] Fixes for WSServer trying to copy a class from a JAR file Signed-off-by: Lachlan Roberts --- .../websocket/jakarta/tests/WSServer.java | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java index 943b63d645d6..c663c0396c85 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java @@ -15,10 +15,13 @@ import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Random; import org.eclipse.jetty.ee10.webapp.WebAppContext; @@ -141,8 +144,20 @@ public void copyClass(Class clazz) throws Exception assertThat("Class URL for: " + clazz, classUrl, notNullValue()); Path destFile = classesDir.resolve(endpointPath); FS.ensureDirExists(destFile.getParent()); - File srcFile = new File(classUrl.toURI()); - IO.copy(srcFile.toPath(), destFile); + URI uri = classUrl.toURI(); + + if ("jar".equalsIgnoreCase(uri.getScheme())) + { + try (InputStream in = classUrl.openStream()) + { + Files.copy(in, destFile, StandardCopyOption.REPLACE_EXISTING); + } + } + else + { + File srcFile = new File(uri); + IO.copy(srcFile.toPath(), destFile); + } } public void copyLib(Class clazz, String jarFileName) throws URISyntaxException, IOException From bdae57ae70f69f97e1631dc9c3838098d87a707c Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 5 Mar 2025 20:36:26 +1100 Subject: [PATCH 29/31] PR #12830 - changes from review Signed-off-by: Lachlan Roberts --- .../websocket/jakarta/tests/WSServer.java | 22 +++++-------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java index c663c0396c85..456f46a5dc43 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/WSServer.java @@ -15,13 +15,10 @@ import java.io.File; import java.io.IOException; -import java.io.InputStream; -import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.util.Random; import org.eclipse.jetty.ee10.webapp.WebAppContext; @@ -34,6 +31,7 @@ import org.eclipse.jetty.toolchain.test.JAR; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.resource.ResourceFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,6 +78,7 @@ protected Handler createRootHandler(Server server) public class WebApp { private final WebAppContext context; + private final ResourceFactory resourceFactory; private final Path contextDir; private final Path webInf; private final Path classesDir; @@ -106,6 +105,8 @@ private WebApp(String contextName) context.setBaseResourceAsPath(contextDir); context.setAttribute("org.eclipse.jetty.websocket.jakarta", Boolean.TRUE); context.addConfiguration(new JakartaWebSocketConfiguration()); + + resourceFactory = ResourceFactory.of(context); } public WebAppContext getWebAppContext() @@ -144,20 +145,7 @@ public void copyClass(Class clazz) throws Exception assertThat("Class URL for: " + clazz, classUrl, notNullValue()); Path destFile = classesDir.resolve(endpointPath); FS.ensureDirExists(destFile.getParent()); - URI uri = classUrl.toURI(); - - if ("jar".equalsIgnoreCase(uri.getScheme())) - { - try (InputStream in = classUrl.openStream()) - { - Files.copy(in, destFile, StandardCopyOption.REPLACE_EXISTING); - } - } - else - { - File srcFile = new File(uri); - IO.copy(srcFile.toPath(), destFile); - } + resourceFactory.newResource(classUrl).copyTo(destFile); } public void copyLib(Class clazz, String jarFileName) throws URISyntaxException, IOException From 59ebdb67312db359269ed1e9b6c15e691cf5e331 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 6 Mar 2025 10:57:22 +1100 Subject: [PATCH 30/31] PR #12875 - fix build issues Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/messages/InputStreamMessageSink.java | 1 + .../jetty/websocket/core/messages/ReaderMessageSink.java | 1 + .../jakarta/common/messages/AbstractDecodedMessageSink.java | 2 +- .../jakarta/common/messages/AbstractMessageSinkTest.java | 4 ++-- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java index 36ad6f46c0ff..00a9c7413598 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/InputStreamMessageSink.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.websocket.core.messages; import java.util.function.Consumer; + import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.util.MethodHolder; diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java index 646c85ec00ab..c0b407251469 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ReaderMessageSink.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.websocket.core.messages; import java.util.function.Consumer; + import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.util.MethodHolder; diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java index ad0fb77b901e..84cfd4f1962e 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractDecodedMessageSink.java @@ -115,7 +115,7 @@ public abstract static class Stream extends AbstractDecodedMe public Stream(CoreSession coreSession, MethodHolder methodHolder, List decoders) { - this(coreSession, methodHandle, decoders, null); + this(coreSession, methodHolder, decoders, null); } public Stream(CoreSession coreSession, MethodHolder methodHolder, List decoders, Consumer onError) diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractMessageSinkTest.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractMessageSinkTest.java index 7011b7f68858..27779ad297e7 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractMessageSinkTest.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/test/java/org/eclipse/jetty/ee11/websocket/jakarta/common/messages/AbstractMessageSinkTest.java @@ -44,7 +44,7 @@ else if (Decoder.BinaryStream.class.isAssignableFrom(clazz)) return List.of(new RegisteredDecoder(clazz, interfaceType, objectType, ClientEndpointConfig.Builder.create().build(), components, false)); } - public MethodHandle getAcceptHandle(Consumer copy, Class type) + public MethodHolder getAcceptHandle(Consumer copy, Class type) { try { @@ -52,7 +52,7 @@ public MethodHandle getAcceptHandle(Consumer copy, Class type) String name = "accept"; MethodType methodType = MethodType.methodType(void.class, type); MethodHandle handle = JakartaWebSocketFrameHandlerFactory.getServerMethodHandleLookup().findVirtual(refc, name, methodType); - return handle.bindTo(copy); + return MethodHolder.from(handle.bindTo(copy)); } catch (NoSuchMethodException | IllegalAccessException e) { From b1b2ab1de3c83b0df89ff3e88efa9222eaafc879 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 18 Mar 2025 11:26:01 +1100 Subject: [PATCH 31/31] invoke onError when sending ws abnormal status code Signed-off-by: Lachlan Roberts --- .../jakarta/common/JakartaWebSocketSession.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketSession.java b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketSession.java index 9e57c1e83003..13992dd56b9f 100644 --- a/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketSession.java +++ b/jetty-ee11/jetty-ee11-websocket/jetty-ee11-websocket-jakarta-common/src/main/java/org/eclipse/jetty/ee11/websocket/jakarta/common/JakartaWebSocketSession.java @@ -36,8 +36,10 @@ import org.eclipse.jetty.ee11.websocket.jakarta.common.decoders.AvailableDecoders; import org.eclipse.jetty.ee11.websocket.jakarta.common.encoders.AvailableEncoders; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.websocket.core.CloseStatus; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.ExtensionConfig; +import org.eclipse.jetty.websocket.core.exception.CloseException; import org.eclipse.jetty.websocket.core.util.ReflectUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -188,8 +190,12 @@ public void close(CloseReason closeReason) { try { - coreSession.close(closeReason.getCloseCode().getCode(), closeReason.getReasonPhrase(), Callback.NOOP); - } + int closeCode = closeReason.getCloseCode().getCode(); + coreSession.close(closeCode, closeReason.getReasonPhrase(), Callback.from(() -> + { + if (!CloseStatus.isOrdinary(closeCode)) + frameHandler.handleError(new CloseException(closeCode, "Abnormal Close")); + })); } catch (Throwable t) { LOG.trace("IGNORED", t);