Skip to content

Commit

Permalink
Merge pull request #4906 from eclipse/jetty-9.4.x-4903-WebSocketPubli…
Browse files Browse the repository at this point in the history
…cModifier

Issue #4903 - give better errors for non public javax.websocket endpoints
  • Loading branch information
lachlan-roberts authored May 26, 2020
2 parents 051e102 + a11c7f5 commit 0b83069
Show file tree
Hide file tree
Showing 4 changed files with 215 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ public <T> T getEndpointInstance(Class<T> endpointClass) throws InstantiationExc
}
catch (Exception e)
{
throw new InstantiationException(String.format("%s: %s", e.getClass().getName(), e.getMessage()));
String errorMsg = String.format("%s: %s", e.getClass().getName(), e.getMessage());
InstantiationException instantiationException = new InstantiationException(errorMsg);
instantiationException.initCause(e);
throw instantiationException;
}
}

Expand Down
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 @@ -34,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;
Expand Down Expand Up @@ -129,14 +130,17 @@ public void addEndpoint(Class<?> endpointClass) throws DeploymentException
{
if (deferredEndpointClasses == null)
{
deferredEndpointClasses = new ArrayList<Class<?>>();
deferredEndpointClasses = new ArrayList<>();
}
deferredEndpointClasses.add(endpointClass);
}
}

private void addEndpoint(ServerEndpointMetadata metadata) throws DeploymentException
{
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);
}
Expand All @@ -157,7 +161,7 @@ public void addEndpoint(ServerEndpointConfig config) throws DeploymentException
{
if (deferredEndpointConfigs == null)
{
deferredEndpointConfigs = new ArrayList<ServerEndpointConfig>();
deferredEndpointConfigs = new ArrayList<>();
}
deferredEndpointConfigs.add(config);
}
Expand Down Expand Up @@ -191,35 +195,43 @@ 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)
try
{
// Annotated takes precedence here
AnnotatedServerEndpointMetadata ametadata = new AnnotatedServerEndpointMetadata(this, endpoint, config);
AnnotatedEndpointScanner<ServerEndpoint, ServerEndpointConfig> scanner = new AnnotatedEndpointScanner<>(ametadata);
metadata = ametadata;
scanner.scan();
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
{
String err = "Not a recognized websocket [" + endpoint.getName() +
"] does not extend @" + ServerEndpoint.class.getName() +
" or extend from " + Endpoint.class.getName();
throw new DeploymentException(err);
}

return metadata;
}
else if (Endpoint.class.isAssignableFrom(endpoint))
catch (DeploymentException e)
{
// extends Endpoint
@SuppressWarnings("unchecked")
Class<? extends Endpoint> eendpoint = (Class<? extends Endpoint>)endpoint;
metadata = new SimpleServerEndpointMetadata(eendpoint, config);
throw e;
}
else
catch (Throwable t)
{
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(t.getMessage(), t);
}

return metadata;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
//
// ========================================================================
// 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)
{
}
}

@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.getMessage(), containsString("Method modifier must be public"));
}
}

0 comments on commit 0b83069

Please sign in to comment.