add deployment exception for non Jakarta WebSocket endpoints used in …

…ServerEndpointConfig (#11032) * Issue #11009 - add test for bad Jakarta endpoint * Issue #11009 - ensure endpoint deployable before adding ServerEndpointConfig * add same test and fix for ee9 Signed-off-by: Lachlan Roberts <[email protected]>
jetty · Dec 17, 2023 · 62210d3 · 62210d3
1 parent fbf8ddd
commit 62210d3
Show file tree

Hide file tree

Showing 7 changed files with 144 additions and 1 deletion.
diff --git a/...ver/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java b/...ver/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java
@@ -65,7 +65,11 @@ public FrameHandler negotiate(ServerUpgradeRequest request, ServerUpgradeRespons
 
         if (websocketPojo == null)
             return null;
-        return factory.newFrameHandler(websocketPojo, request, response);
+
+        FrameHandler frameHandler = factory.newFrameHandler(websocketPojo, request, response);
+        if (frameHandler == null)
+            callback.failed(new IllegalStateException("No WebSocket FrameHandler was created"));
+        return frameHandler;
     }
 
     @Override

diff --git a/...java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/...java/org/eclipse/jetty/ee10/websocket/jakarta/server/JakartaWebSocketServerContainer.java
@@ -24,6 +24,7 @@
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import jakarta.websocket.ClientEndpoint;
 import jakarta.websocket.DeploymentException;
 import jakarta.websocket.server.ServerEndpoint;
 import jakarta.websocket.server.ServerEndpointConfig;
@@ -177,6 +178,13 @@ private void validateEndpointConfig(ServerEndpointConfig config) throws Deployme
             throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
         }
 
+        if (!(jakarta.websocket.Endpoint.class.isAssignableFrom(endpointClass)) &&
+            endpointClass.getAnnotation(ServerEndpoint.class) == null &&
+            endpointClass.getAnnotation(ClientEndpoint.class) == null)
+        {
+            throw new DeploymentException("Unable to deploy unknown endpoint class: " + endpointClass.getName());
+        }
+
         if (!Modifier.isPublic(endpointClass.getModifiers()))
         {
             throw new DeploymentException("Class is not public: " + endpointClass.getName());

diff --git a/...t/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java b/...t/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java
@@ -20,8 +20,10 @@
 import jakarta.websocket.DeploymentException;
 import jakarta.websocket.server.ServerContainer;
 import jakarta.websocket.server.ServerEndpoint;
+import jakarta.websocket.server.ServerEndpointConfig;
 import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
 import org.eclipse.jetty.ee10.websocket.jakarta.server.config.JakartaWebSocketServletContainerInitializer;
+import org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets.BadEndpoint;
 import org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets.InvalidCloseIntSocket;
 import org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets.InvalidErrorErrorSocket;
 import org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets.InvalidErrorIntSocket;
@@ -33,6 +35,7 @@
 import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -106,4 +109,29 @@ public void testDeployInvalidSignature(Class<?> pojo) throws Exception
             context.stop();
         }
     }
+
+    @Test
+    public void testDeploymentException() throws Exception
+    {
+        ServletContextHandler context = new ServletContextHandler();
+        context.setServer(server);
+        JakartaWebSocketServletContainerInitializer.configure(context, null);
+
+        contexts.addHandler(context);
+        try
+        {
+            context.start();
+            ServerContainer serverContainer = (ServerContainer)context.getServletContext().getAttribute(ServerContainer.class.getName());
+
+            // We cannot deploy this because it does not extend Endpoint and has no @ServerEndpoint/@ClientEndpoint annotation.
+            assertThrows(DeploymentException.class, () ->
+                serverContainer.addEndpoint(BadEndpoint.class));
+            assertThrows(DeploymentException.class, () ->
+                serverContainer.addEndpoint(ServerEndpointConfig.Builder.create(BadEndpoint.class, "/ws").build()));
+        }
+        finally
+        {
+            context.stop();
+        }
+    }
 }
diff --git a/.../test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java b/.../test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java
@@ -0,0 +1,34 @@
+//
+// ========================================================================
+// 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 org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets;
+
+import jakarta.websocket.EndpointConfig;
+import jakarta.websocket.MessageHandler;
+
+public class BadEndpoint
+{
+    public void onOpen(jakarta.websocket.Session session, EndpointConfig config)
+    {
+        try
+        {
+            session.addMessageHandler((MessageHandler.Whole<Object>)System.out::println);
+            System.out.println("server open");
+            session.getBasicRemote().sendText("connected");
+        }
+        catch (Throwable t)
+        {
+            t.printStackTrace(System.err);
+        }
+    }
+}
diff --git a/.../java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/.../java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java
@@ -24,6 +24,7 @@
 import jakarta.servlet.ServletContext;
 import jakarta.servlet.http.HttpServletRequest;
 import jakarta.servlet.http.HttpServletResponse;
+import jakarta.websocket.ClientEndpoint;
 import jakarta.websocket.DeploymentException;
 import jakarta.websocket.server.ServerEndpoint;
 import jakarta.websocket.server.ServerEndpointConfig;
@@ -178,6 +179,13 @@ private void validateEndpointConfig(ServerEndpointConfig config) throws Deployme
             throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
         }
 
+        if (!(jakarta.websocket.Endpoint.class.isAssignableFrom(endpointClass)) &&
+            endpointClass.getAnnotation(ServerEndpoint.class) == null &&
+            endpointClass.getAnnotation(ClientEndpoint.class) == null)
+        {
+            throw new DeploymentException("Unable to deploy unknown endpoint class: " + endpointClass.getName());
+        }
+
         if (!Modifier.isPublic(endpointClass.getModifiers()))
         {
             throw new DeploymentException("Class is not public: " + endpointClass.getName());

diff --git a/...tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/BadEndpoint.java b/...tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/BadEndpoint.java
@@ -0,0 +1,34 @@
+//
+// ========================================================================
+// 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 org.eclipse.jetty.ee9.websocket.jakarta.tests.server;
+
+import jakarta.websocket.EndpointConfig;
+import jakarta.websocket.MessageHandler;
+
+public class BadEndpoint
+{
+    public void onOpen(jakarta.websocket.Session session, EndpointConfig config)
+    {
+        try
+        {
+            session.addMessageHandler((MessageHandler.Whole<Object>)System.out::println);
+            System.out.println("server open");
+            session.getBasicRemote().sendText("connected");
+        }
+        catch (Throwable t)
+        {
+            t.printStackTrace(System.err);
+        }
+    }
+}
diff --git a/...st/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/DeploymentExceptionTest.java b/...st/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/DeploymentExceptionTest.java
@@ -20,6 +20,7 @@
 import jakarta.websocket.DeploymentException;
 import jakarta.websocket.server.ServerContainer;
 import jakarta.websocket.server.ServerEndpoint;
+import jakarta.websocket.server.ServerEndpointConfig;
 import org.eclipse.jetty.ee9.nested.ContextHandler;
 import org.eclipse.jetty.ee9.nested.HandlerCollection;
 import org.eclipse.jetty.ee9.servlet.ServletContextHandler;
@@ -34,6 +35,7 @@
 import org.eclipse.jetty.websocket.core.exception.InvalidSignatureException;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
@@ -109,4 +111,29 @@ public void testDeployInvalidSignature(Class<?> pojo) throws Exception
             context.stop();
         }
     }
+
+    @Test
+    public void testDeploymentException() throws Exception
+    {
+        ServletContextHandler context = new ServletContextHandler();
+        context.setServer(server);
+        JakartaWebSocketServletContainerInitializer.configure(context, null);
+
+        contexts.addHandler(context);
+        try
+        {
+            context.start();
+            ServerContainer serverContainer = (ServerContainer)context.getServletContext().getAttribute(ServerContainer.class.getName());
+
+            // We cannot deploy this because it does not extend Endpoint and has no @ServerEndpoint/@ClientEndpoint annotation.
+            assertThrows(DeploymentException.class, () ->
+                serverContainer.addEndpoint(BadEndpoint.class));
+            assertThrows(DeploymentException.class, () ->
+                serverContainer.addEndpoint(ServerEndpointConfig.Builder.create(BadEndpoint.class, "/ws").build()));
+        }
+        finally
+        {
+            context.stop();
+        }
+    }
 }