This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b4a49009b24c32c2c56af268e2bb480920a8b2c3 Author: opencode <[email protected]> AuthorDate: Sat Oct 17 13:19:33 2026 +0200 Fix NPE handling connections to a POJO endpoint with no annotated methods The method mapping for a POJO @ServerEndpoint class is stored in the user properties of the ServerEndpointConfig only when the class actually has annotated methods. PojoEndpointServer.onOpen() and the onClose()/onError() handling in PojoEndpointBase then dereference that mapping unconditionally, so the first connection to a class annotated with @ServerEndpoint but with no @OnOpen, @OnMessage, @OnClose or @OnError method triggered a NullPointerException. The error handling NPEd a second time and the exception escaped WsHttpUpgradeHandler.init(), failing the upgrade with an opaque error instead of a usable session. The "only if there are methods" condition was introduced for bug 58232 to avoid storing a mapping for programmatic endpoints (Endpoint subclasses) since those never use it. That restriction was never intended to cover POJO endpoints, for which the mapping must always be present. Store it for any endpoint class that is not an Endpoint subclass, retaining the previous behaviour for Endpoint subclasses, and always store the mapping created at upgrade time in UpgradeUtil.doUpgrade() where the surrounding condition already excludes Endpoint subclasses. An empty mapping is harmless: PojoMethodMapping performs no validation when a class has no annotated methods and getMessageHandlers() returns an empty set, so such an endpoint now behaves like the equivalent @ClientEndpoint POJO - the connection opens and simply has no handlers. Add unit tests covering both addEndpoint() paths (stored for a POJO class with no methods, still not stored for an Endpoint subclass, stored by upgradeHttpToWebSocket()) and an end-to-end test that a connection to such an endpoint stays open. --- .../tomcat/websocket/server/UpgradeUtil.java | 7 +--- .../tomcat/websocket/server/WsServerContainer.java | 8 +++- .../websocket/pojo/TestPojoEndpointBase.java | 39 ++++++++++++++++++++ .../websocket/server/TestWsServerContainer.java | 43 ++++++++++++++++++++++ 4 files changed, 91 insertions(+), 6 deletions(-) diff --git a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java index edbfb9117e..c09cf2e896 100644 --- a/java/org/apache/tomcat/websocket/server/UpgradeUtil.java +++ b/java/org/apache/tomcat/websocket/server/UpgradeUtil.java @@ -216,11 +216,8 @@ public class UpgradeUtil { try { PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), sec.getDecoders(), sec.getPath(), sc.getInstanceManager(Thread.currentThread().getContextClassLoader())); - if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null || - methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) { - sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY, - methodMapping); - } + sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY, + methodMapping); } catch (DeploymentException e) { throw new ServletException(sm.getString("upgradeUtil.pojoMapFail", sec.getEndpointClass().getName()), e); diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java b/java/org/apache/tomcat/websocket/server/WsServerContainer.java index 352cddfbf1..c76fb0d6a1 100644 --- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java +++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java @@ -39,6 +39,7 @@ import javax.websocket.CloseReason; import javax.websocket.CloseReason.CloseCodes; import javax.websocket.DeploymentException; import javax.websocket.Encoder; +import javax.websocket.Endpoint; import javax.websocket.server.ServerContainer; import javax.websocket.server.ServerEndpoint; import javax.websocket.server.ServerEndpointConfig; @@ -148,7 +149,12 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon // Add method mapping to user properties PojoMethodMapping methodMapping = new PojoMethodMapping(sec.getEndpointClass(), sec.getDecoders(), path, getInstanceManager(Thread.currentThread().getContextClassLoader())); - if (methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null || + // The mapping is not used for programmatic endpoints (Endpoint subclasses) + // so for those classes it is only stored when it has content (bug 58232). + // For POJO endpoints it must always be stored, even when empty, since + // PojoEndpointServer expects it to be present. + if (!Endpoint.class.isAssignableFrom(sec.getEndpointClass()) || + methodMapping.getOnClose() != null || methodMapping.getOnOpen() != null || methodMapping.getOnError() != null || methodMapping.hasMessageHandlers()) { sec.getUserProperties().put(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY, methodMapping); diff --git a/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java b/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java index fe8328a2fc..a79ffdb397 100644 --- a/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java +++ b/test/org/apache/tomcat/websocket/pojo/TestPojoEndpointBase.java @@ -120,6 +120,45 @@ public class TestPojoEndpointBase extends TomcatBaseTest { } + /* + * A POJO endpoint class with no annotated methods deploys but has no handlers. + * Previously, the method mapping was not made available to the endpoint wrapper + * for such a class, resulting in a NullPointerException when handling the first + * connection and an abnormal close of that connection. + */ + @Test + public void testNoMethodsServerEndpoint() throws Exception { + // Set up utility classes + ServerConfigListener.setPojoClazz(NoMethodsServerEndpoint.class); + + Tomcat tomcat = getTomcatInstance(); + // No file system docBase required + Context ctx = getProgrammaticRootContext(); + ctx.addApplicationListener(ServerConfigListener.class.getName()); + Tomcat.addServlet(ctx, "default", new DefaultServlet()); + ctx.addServletMapping("/", "default"); + + WebSocketContainer wsContainer = ContainerProvider.getWebSocketContainer(); + + tomcat.start(); + + Client client = new Client(); + URI uri = new URI("ws://localhost:" + getPort() + "/"); + + Session session = wsContainer.connectToServer(client, uri); + + // Nothing should happen on the connection. Prior to the fix, the + // connection was closed abnormally as soon as it was opened. + Assert.assertFalse("Connection was closed unexpectedly", client.waitForClose(2)); + Assert.assertTrue(session.isOpen()); + } + + + @ServerEndpoint("/") + public static class NoMethodsServerEndpoint { + } + + @ServerEndpoint("/") public static class Bug54716 { diff --git a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java index 46182d2d1e..4cce50677e 100644 --- a/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java +++ b/test/org/apache/tomcat/websocket/server/TestWsServerContainer.java @@ -33,6 +33,7 @@ import javax.websocket.ContainerProvider; import javax.websocket.Decoder; import javax.websocket.DeploymentException; import javax.websocket.Encoder; +import javax.websocket.Endpoint; import javax.websocket.EndpointConfig; import javax.websocket.HandshakeResponse; import javax.websocket.Session; @@ -440,6 +441,44 @@ public class TestWsServerContainer extends WebSocketBaseTest { public static class PojoTemplate { } + + /* + * The method mapping must be stored in the user properties for POJO endpoints even when the class has no annotated + * methods since PojoEndpointServer expects it to be present. Without it, handling the first connection results in a + * NullPointerException. The mapping must not be stored for programmatic endpoints (Endpoint subclasses) since it is + * not used (see bug 58232). + */ + @Test + public void testMethodMappingStoredForPojoWithNoAnnotatedMethods() throws Exception { + WsServerContainer sc = new WsServerContainer(new TesterServletContext()); + + ServerEndpointConfig pojoConfig = ServerEndpointConfig.Builder.create(PojoNoMethods.class, "/noMethods") + .build(); + sc.addEndpoint(pojoConfig); + Assert.assertNotNull(pojoConfig.getUserProperties() + .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY)); + + ServerEndpointConfig endpointConfig = ServerEndpointConfig.Builder.create(NoMethodsEndpoint.class, "/endpoint") + .build(); + sc.addEndpoint(endpointConfig); + Assert.assertNull(endpointConfig.getUserProperties() + .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY)); + } + + + @ServerEndpoint("/noMethods") + public static class PojoNoMethods { + } + + + public static class NoMethodsEndpoint extends Endpoint { + @Override + public void onOpen(@SuppressWarnings("unused") Session session, + @SuppressWarnings("unused") EndpointConfig config) { + // NO-OP + } + } + @Test public void testUpgradeHttpToWebSocket01() throws Exception { TesterHttpServletRequest request = new TesterHttpServletRequest(); @@ -465,6 +504,10 @@ public class TestWsServerContainer extends WebSocketBaseTest { Assert.assertEquals(Constants.UPGRADE_HEADER_VALUE, response.getHeader(Constants.UPGRADE_HEADER_NAME)); Assert.assertEquals("aJvXj0bbnSSeXm32ngvbBilP0lE=", response.getHeader(HandshakeResponse.SEC_WEBSOCKET_ACCEPT)); + // The endpoint class is not an Endpoint subclass so the method mapping must + // have been created and stored for the POJO endpoint path. + Assert.assertNotNull(sec.getUserProperties() + .get(org.apache.tomcat.websocket.pojo.Constants.POJO_METHOD_MAPPING_KEY)); } @Test(expected = DeploymentException.class) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
