Skip to content

Commit

Permalink
Issue #4903 - Better errors for non public endpoints
Browse files Browse the repository at this point in the history
Signed-off-by: Lachlan Roberts <[email protected]>
  • Loading branch information
lachlan-roberts committed May 25, 2020
1 parent 74f9b46 commit 28a588b
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse
}
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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -137,6 +138,9 @@ 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");

JsrCreator creator = new JsrCreator(this, metadata, this.configuration.getFactory().getExtensionFactory());
this.configuration.addMapping("uri-template|" + metadata.getPath(), creator);
}
Expand Down Expand Up @@ -191,35 +195,36 @@ protected void doStart() throws Exception

public ServerEndpointMetadata getServerEndpointMetadata(final Class<?> endpoint, final ServerEndpointConfig config) throws DeploymentException
{
ServerEndpointMetadata metadata = null;

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))
try
{
// extends Endpoint
@SuppressWarnings("unchecked")
Class<? extends Endpoint> eendpoint = (Class<? extends Endpoint>)endpoint;
metadata = new SimpleServerEndpointMetadata(eendpoint, config);
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);
}

return metadata;
}
else
catch (RuntimeException e)
{
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);
throw new DeploymentException(e.getMessage(), e);
}

return metadata;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -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"));
}
}

0 comments on commit 28a588b

Please sign in to comment.