Issue #4903 - Better errors for non public endpoints

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-05-25 12:27:49 +10:00
parent 74f9b464a6
commit 28a588b8bd
3 changed files with 178 additions and 27 deletions

View File

@ -159,8 +159,7 @@ public class JsrCreator implements WebSocketCreator
}
catch (InstantiationException e)
{
if (LOG.isDebugEnabled())
LOG.debug("Unable to create websocket: " + config.getEndpointClass().getName(), e);
LOG.warn("Unable to create websocket: " + config.getEndpointClass().getName(), e);
return null;
}
}

View File

@ -18,6 +18,7 @@
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;
@ -137,6 +138,9 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
private void addEndpoint(ServerEndpointMetadata metadata) throws DeploymentException
{
if (!Modifier.isPublic(metadata.getEndpointClass().getModifiers()))
throw new DeploymentException("Class modifier must be public");
JsrCreator creator = new JsrCreator(this, metadata, this.configuration.getFactory().getExtensionFactory());
this.configuration.addMapping("uri-template|" + metadata.getPath(), creator);
}
@ -191,35 +195,36 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
public ServerEndpointMetadata getServerEndpointMetadata(final Class<?> endpoint, final ServerEndpointConfig config) throws DeploymentException
{
ServerEndpointMetadata metadata = null;
try
{
ServerEndpointMetadata metadata;
ServerEndpoint anno = endpoint.getAnnotation(ServerEndpoint.class);
if (anno != null)
{
// Annotated takes precedence here
AnnotatedServerEndpointMetadata ametadata = new AnnotatedServerEndpointMetadata(this, endpoint, config);
AnnotatedEndpointScanner<ServerEndpoint, ServerEndpointConfig> scanner = new AnnotatedEndpointScanner<>(ametadata);
metadata = ametadata;
scanner.scan();
}
else if (Endpoint.class.isAssignableFrom(endpoint))
{
// extends Endpoint
@SuppressWarnings("unchecked")
Class<? extends Endpoint> eendpoint = (Class<? extends Endpoint>)endpoint;
metadata = new SimpleServerEndpointMetadata(eendpoint, config);
}
else
{
throw new DeploymentException("Unable to identify as valid Endpoint: " + endpoint);
}
ServerEndpoint anno = endpoint.getAnnotation(ServerEndpoint.class);
if (anno != null)
{
// Annotated takes precedence here
AnnotatedServerEndpointMetadata ametadata = new AnnotatedServerEndpointMetadata(this, endpoint, config);
AnnotatedEndpointScanner<ServerEndpoint, ServerEndpointConfig> scanner = new AnnotatedEndpointScanner<>(ametadata);
metadata = ametadata;
scanner.scan();
return metadata;
}
else if (Endpoint.class.isAssignableFrom(endpoint))
catch (RuntimeException e)
{
// extends Endpoint
@SuppressWarnings("unchecked")
Class<? extends Endpoint> eendpoint = (Class<? extends Endpoint>)endpoint;
metadata = new SimpleServerEndpointMetadata(eendpoint, config);
throw new DeploymentException(e.getMessage(), e);
}
else
{
StringBuilder err = new StringBuilder();
err.append("Not a recognized websocket [");
err.append(endpoint.getName());
err.append("] does not extend @").append(ServerEndpoint.class.getName());
err.append(" or extend from ").append(Endpoint.class.getName());
throw new DeploymentException("Unable to identify as valid Endpoint: " + endpoint);
}
return metadata;
}
@Override

View File

@ -0,0 +1,147 @@
//
// ========================================================================
// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
// ------------------------------------------------------------------------
// All rights reserved. This program and the accompanying materials
// are made available under the terms of the Eclipse Public License v1.0
// and Apache License v2.0 which accompanies this distribution.
//
// The Eclipse Public License is available at
// http://www.eclipse.org/legal/epl-v10.html
//
// The Apache License v2.0 is available at
// http://www.opensource.org/licenses/apache2.0.php
//
// You may elect to redistribute this code under either of these licenses.
// ========================================================================
//
package org.eclipse.jetty.websocket.jsr356.server;
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.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.jsr356.server.deploy.WebSocketServerContainerInitializer;
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
{
WebSocketServerContainerInitializer.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)
{
}
}
@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("Class modifier must be public"));
}
@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("Class modifier must be public"));
}
@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.getMessage(), containsString("Method modifier must be public"));
}
}