Issue #4903 - check endpoint class is default constructable

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · May 25, 2020 · add00c9 · add00c9
1 parent 28a588b
commit add00c9
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 6 deletions.
diff --git a/...-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java b/...-server-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/server/ServerContainer.java
@@ -18,7 +18,6 @@
 
 package org.eclipse.jetty.websocket.jsr356.server;
 
-import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Executor;
@@ -35,6 +34,7 @@
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.Logger;
 import org.eclipse.jetty.websocket.common.events.EventDriverFactory;
+import org.eclipse.jetty.websocket.common.util.ReflectUtils;
 import org.eclipse.jetty.websocket.jsr356.ClientContainer;
 import org.eclipse.jetty.websocket.jsr356.JsrSessionFactory;
 import org.eclipse.jetty.websocket.jsr356.annotations.AnnotatedEndpointScanner;
@@ -138,8 +138,8 @@ public void addEndpoint(Class<?> endpointClass) throws DeploymentException
 
     private void addEndpoint(ServerEndpointMetadata metadata) throws DeploymentException
     {
-        if (!Modifier.isPublic(metadata.getEndpointClass().getModifiers()))
-            throw new DeploymentException("Class modifier must be public");
+        if (!ReflectUtils.isDefaultConstructable(metadata.getEndpointClass()))
+            throw new DeploymentException("Cannot access default constructor for the Endpoint class");
 
         JsrCreator creator = new JsrCreator(this, metadata, this.configuration.getFactory().getExtensionFactory());
         this.configuration.addMapping("uri-template|" + metadata.getPath(), creator);

diff --git a/...ver-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/PrivateEndpointTest.java b/...ver-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/server/PrivateEndpointTest.java
@@ -94,6 +94,21 @@ public void onMessage(String message)
         }
     }
 
+    @SuppressWarnings("InnerClassMayBeStatic")
+    public class ServerSocketNonStatic extends Endpoint implements MessageHandler.Whole<String>
+    {
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+            session.addMessageHandler(this);
+        }
+
+        @Override
+        public void onMessage(String message)
+        {
+        }
+    }
+
     @ServerEndpoint("/annotated")
     private static class AnnotatedServerSocket
     {
@@ -120,7 +135,18 @@ public void testEndpoint()
 
         assertThat(error.getCause(), instanceOf(DeploymentException.class));
         DeploymentException deploymentException = (DeploymentException)error.getCause();
-        assertThat(deploymentException.getMessage(), containsString("Class modifier must be public"));
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
+    }
+
+    @Test
+    public void testInnerEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(ServerEndpointConfig.Builder.create(ServerSocketNonStatic.class, "/").build())));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
     }
 
     @Test
@@ -131,7 +157,7 @@ public void testAnnotatedEndpoint()
 
         assertThat(error.getCause(), instanceOf(DeploymentException.class));
         DeploymentException deploymentException = (DeploymentException)error.getCause();
-        assertThat(deploymentException.getMessage(), containsString("Class modifier must be public"));
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
     }
 
     @Test
@@ -144,4 +170,4 @@ public void testAnnotatedMethod()
         DeploymentException deploymentException = (DeploymentException)error.getCause();
         assertThat(deploymentException.getMessage(), containsString("Method modifier must be public"));
     }
-}
+}