Give better errors for non public javax.websocket endpoints

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/ContainerDefaultConfigurator.java

@@ -72,7 +72,8 @@ public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationExc
         }
         catch (Exception e)
         {
-            InstantiationException instantiationException = new InstantiationException();
+            String errorMsg = String.format("%s: %s", e.getClass().getName(), e.getMessage());
+            InstantiationException instantiationException = new InstantiationException(errorMsg);
             instantiationException.initCause(e);
             throw instantiationException;
         }

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketCreator.java

@@ -24,6 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import javax.websocket.Extension;
 import javax.websocket.Extension.Parameter;
 import javax.websocket.server.ServerEndpointConfig;
@@ -70,7 +71,7 @@ public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse
         // per upgrade request.
         ServerEndpointConfig config = new ServerEndpointConfigWrapper(baseConfig)
         {
-            Map<String, Object> userProperties = new HashMap<>(baseConfig.getUserProperties());
+            final Map<String, Object> userProperties = new HashMap<>(baseConfig.getUserProperties());
 
             @Override
             public Map<String, Object> getUserProperties()
@@ -183,15 +184,13 @@ public boolean equals(Object o)
             return false;
 
         JavaxWebSocketCreator that = (JavaxWebSocketCreator)o;
-
-        return baseConfig != null ? baseConfig.equals(that.baseConfig) : that.baseConfig == null;
+        return Objects.equals(baseConfig, that.baseConfig);
     }
 
     @Override
     public int hashCode()
     {
-        int result = (baseConfig != null ? baseConfig.hashCode() : 0);
-        return result;
+        return (baseConfig != null ? baseConfig.hashCode() : 0);
     }
 
     @Override

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java

@@ -36,10 +36,10 @@
 import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
-import org.eclipse.jetty.websocket.core.exception.WebSocketException;
 import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents;
 import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer;
 import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer;
+import org.eclipse.jetty.websocket.util.ReflectUtils;
 import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -160,6 +160,7 @@ public JavaxWebSocketServerFrameHandlerFactory getFrameHandlerFactory()
         return frameHandlerFactory;
     }
 
+    @Override
     public void addEndpoint(Class<?> endpointClass) throws DeploymentException
     {
         if (endpointClass == null)
@@ -169,19 +170,12 @@ public void addEndpoint(Class<?> endpointClass) throws DeploymentException
 
         if (isStarted() || isStarting())
         {
-            try
-            {
-                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()));
+            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()));
 
-                ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno);
-                addEndpointMapping(config);
-            }
-            catch (WebSocketException e)
-            {
-                throw new DeploymentException("Unable to deploy: " + endpointClass.getName(), e);
-            }
+            ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno);
+            addEndpointMapping(config);
         }
         else
         {
@@ -199,23 +193,16 @@ public void addEndpoint(ServerEndpointConfig providedConfig) throws DeploymentEx
 
         if (isStarted() || isStarting())
         {
+            // If we have annotations merge the annotated ServerEndpointConfig with the provided one.
             Class<?> endpointClass = providedConfig.getEndpointClass();
-            try
-            {
-                // If we have annotations merge the annotated ServerEndpointConfig with the provided one.
-                ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class);
-                ServerEndpointConfig config = (anno == null) ? providedConfig
-                    : new AnnotatedServerEndpointConfig(this, endpointClass, anno, providedConfig);
+            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);
 
-                addEndpointMapping(config);
-            }
-            catch (WebSocketException e)
-            {
-                throw new DeploymentException("Unable to deploy: " + endpointClass.getName(), e);
-            }
+            addEndpointMapping(config);
         }
         else
         {
@@ -225,14 +212,23 @@ public void addEndpoint(ServerEndpointConfig providedConfig) throws DeploymentEx
         }
     }
 
-    private void addEndpointMapping(ServerEndpointConfig config) throws WebSocketException
+    private void addEndpointMapping(ServerEndpointConfig config) throws DeploymentException
     {
-        frameHandlerFactory.getMetadata(config.getEndpointClass(), config);
+        if (!ReflectUtils.isDefaultConstructable(config.getEndpointClass()))
+            throw new DeploymentException("Cannot access default constructor for the Endpoint class");
 
-        JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, getExtensionRegistry());
+        try
+        {
+            frameHandlerFactory.getMetadata(config.getEndpointClass(), config);
+            JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, getExtensionRegistry());
+            PathSpec pathSpec = new UriTemplatePathSpec(config.getPath());
+            webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer);
+        }
+        catch (Throwable t)
+        {
+            throw new DeploymentException("Unable to deploy: " + config.getEndpointClass().getName(), t);
+        }
 
-        PathSpec pathSpec = new UriTemplatePathSpec(config.getPath());
-        webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer);
     }
 
     @Override

jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/PrivateEndpointTest.java

@@ -0,0 +1,177 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.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.websocket.javax.tests;
+
+import javax.websocket.ContainerProvider;
+import javax.websocket.DeploymentException;
+import javax.websocket.Endpoint;
+import javax.websocket.EndpointConfig;
+import javax.websocket.MessageHandler;
+import javax.websocket.OnMessage;
+import javax.websocket.Session;
+import javax.websocket.WebSocketContainer;
+import javax.websocket.server.ServerContainer;
+import javax.websocket.server.ServerEndpoint;
+import javax.websocket.server.ServerEndpointConfig;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer;
+import org.eclipse.jetty.websocket.util.InvalidSignatureException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+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 PrivateEndpointTest
+{
+    private Server server;
+    private WebSocketContainer client;
+    private ServletContextHandler contextHandler;
+
+    @BeforeEach
+    public void before()
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        contextHandler = new ServletContextHandler();
+        contextHandler.setContextPath("/");
+        server.setHandler(contextHandler);
+        client = ContainerProvider.getWebSocketContainer();
+    }
+
+    @AfterEach
+    public void after() throws Exception
+    {
+        LifeCycle.stop(client);
+        server.stop();
+    }
+
+    public interface CheckedConsumer<T>
+    {
+        void accept(T t) throws DeploymentException;
+    }
+
+    public void start(CheckedConsumer<ServerContainer> containerConsumer) throws Exception
+    {
+        JavaxWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> containerConsumer.accept(container));
+        server.start();
+    }
+
+    private static class ServerSocket extends Endpoint implements MessageHandler.Whole<String>
+    {
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+            session.addMessageHandler(this);
+        }
+
+        @Override
+        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
+    {
+        @OnMessage
+        public void onMessage(String message)
+        {
+        }
+    }
+
+    @ServerEndpoint("/annotatedMethod")
+    public static class AnnotatedServerMethod
+    {
+        @OnMessage
+        private void onMessage(String message)
+        {
+        }
+    }
+
+    @Test
+    public void testEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(ServerEndpointConfig.Builder.create(ServerSocket.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
+    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
+    public void testAnnotatedEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(AnnotatedServerSocket.class)));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor for the Endpoint class"));
+    }
+
+    @Test
+    public void testAnnotatedMethod()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(AnnotatedServerMethod.class)));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getCause(), instanceOf(InvalidSignatureException.class));
+        InvalidSignatureException cause = (InvalidSignatureException)deploymentException.getCause();
+        assertThat(cause.getMessage(), containsString("@OnMessage method must be public"));
+    }
+}