From a53caee0eb024c73e5265863540b4da56bc7330a Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 8 Dec 2023 09:09:51 +1100 Subject: [PATCH 1/5] Issue #11009 - add test for bad Jakarta endpoint Signed-off-by: Lachlan Roberts --- .../tests/server/DeploymentExceptionTest.java | 24 +++++++++++++ .../tests/server/sockets/BadEndpoint.java | 34 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java index 0707c4ddfd4..3465bacf9c1 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java @@ -22,6 +22,7 @@ import jakarta.websocket.server.ServerEndpoint; 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 +34,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 +108,26 @@ 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)); + } + finally + { + context.stop(); + } + } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java new file mode 100644 index 00000000000..daa764ab8c6 --- /dev/null +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/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)System.out::println); + System.out.println("server open"); + session.getBasicRemote().sendText("connected"); + } + catch (Throwable t) + { + t.printStackTrace(System.err); + } + } + } From 82cf6cb815ea6543a90761c732ee4dff9c0d3dcb Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 8 Dec 2023 09:21:52 +1100 Subject: [PATCH 2/5] Issue #11009 - fix test to reproduce issue Signed-off-by: Lachlan Roberts --- .../jakarta/tests/server/DeploymentExceptionTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java index 3465bacf9c1..41a7f81bf9b 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/DeploymentExceptionTest.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/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.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.ee10.websocket.jakarta.server.config.JakartaWebSocketServletContainerInitializer; import org.eclipse.jetty.ee10.websocket.jakarta.tests.server.sockets.BadEndpoint; @@ -123,7 +124,10 @@ public void testDeploymentException() throws Exception 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(BadEndpoint.class)); + assertThrows(DeploymentException.class, () -> + serverContainer.addEndpoint(ServerEndpointConfig.Builder.create(BadEndpoint.class, "/ws").build())); } finally { From 6331ed93f6be45d6019d2927e37f6764c375874c Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 8 Dec 2023 09:23:30 +1100 Subject: [PATCH 3/5] Issue #11009 - ensure endpoint deployable before adding ServerEndpointConfig Signed-off-by: Lachlan Roberts --- .../websocket/core/server/internal/CreatorNegotiator.java | 6 +++++- .../jakarta/server/JakartaWebSocketServerContainer.java | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java index 1247b5ec7e7..207fe198b15 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-server/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/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 ecb5827195d..46c0d264bda 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 @@ -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()); From c1b39509e7bf3587796b81c0f3940db4655d3414 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 8 Dec 2023 09:32:32 +1100 Subject: [PATCH 4/5] Issue #11009 - fix checkstyle errors Signed-off-by: Lachlan Roberts --- .../jakarta/server/JakartaWebSocketServerContainer.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 46c0d264bda..041bf6b427a 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 @@ -178,9 +178,9 @@ 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) + 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()); } From a9d2d1346bb21abec01e83d31a0134efeea2bb60 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 11 Dec 2023 16:11:16 +1100 Subject: [PATCH 5/5] add same test and fix for ee9 Signed-off-by: Lachlan Roberts --- .../tests/server/sockets/BadEndpoint.java | 22 ++++++------ .../JakartaWebSocketServerContainer.java | 8 +++++ .../jakarta/tests/server/BadEndpoint.java | 34 +++++++++++++++++++ .../tests/server/DeploymentExceptionTest.java | 27 +++++++++++++++ 4 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/BadEndpoint.java diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java index daa764ab8c6..d551cc41dd4 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee10/websocket/jakarta/tests/server/sockets/BadEndpoint.java @@ -17,18 +17,18 @@ import jakarta.websocket.MessageHandler; public class BadEndpoint +{ + public void onOpen(jakarta.websocket.Session session, EndpointConfig config) { - public void onOpen(jakarta.websocket.Session session, EndpointConfig config) + try { - try - { - session.addMessageHandler((MessageHandler.Whole)System.out::println); - System.out.println("server open"); - session.getBasicRemote().sendText("connected"); - } - catch (Throwable t) - { - t.printStackTrace(System.err); - } + session.addMessageHandler((MessageHandler.Whole)System.out::println); + System.out.println("server open"); + session.getBasicRemote().sendText("connected"); + } + catch (Throwable t) + { + t.printStackTrace(System.err); } } +} diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java index cdfbf517f35..e646f5eb141 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/JakartaWebSocketServerContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/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/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/BadEndpoint.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/BadEndpoint.java new file mode 100644 index 00000000000..e74ffd33e0a --- /dev/null +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-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)System.out::println); + System.out.println("server open"); + session.getBasicRemote().sendText("connected"); + } + catch (Throwable t) + { + t.printStackTrace(System.err); + } + } +} diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/DeploymentExceptionTest.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/DeploymentExceptionTest.java index fb164bac8fc..17740266994 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/java/org/eclipse/jetty/ee9/websocket/jakarta/tests/server/DeploymentExceptionTest.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-tests/src/test/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(); + } + } }