Issue #4903 - fix validation on custom Configurator annotated endpoint

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
Lachlan Roberts 2020-06-11 09:58:49 +10:00
parent b22e306796
commit f41f601e19
4 changed files with 90 additions and 35 deletions

View File

@ -129,7 +129,7 @@ public class AnnotatedServerEndpointConfig implements ServerEndpointConfig
{
try
{
resolvedConfigurator = anno.configurator().getDeclaredConstructor().newInstance();
resolvedConfigurator = anno.configurator().getConstructor().newInstance();
}
catch (Exception e)
{

View File

@ -118,6 +118,34 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
return new EndpointInstance(endpoint, cec, metadata);
}
private void validateEndpointConfig(ServerEndpointConfig config) throws DeploymentException
{
if (config == null)
{
throw new DeploymentException("Unable to deploy null ServerEndpointConfig");
}
ServerEndpointConfig.Configurator configurator = config.getConfigurator();
if (configurator == null)
{
throw new DeploymentException("Unable to deploy with null ServerEndpointConfig.Configurator");
}
Class<?> endpointClass = config.getEndpointClass();
if (endpointClass == null)
{
throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
}
if (configurator.getClass() == ContainerDefaultConfigurator.class)
{
if (!ReflectUtils.isDefaultConstructable(endpointClass))
{
throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
}
}
}
@Override
public void addEndpoint(Class<?> endpointClass) throws DeploymentException
{
@ -128,16 +156,13 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
if (isStarted() || isStarting())
{
if (!ReflectUtils.isDefaultConstructable(endpointClass))
{
throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
}
if (LOG.isDebugEnabled())
{
LOG.debug("addEndpoint({})", endpointClass);
}
ServerEndpointMetadata metadata = getServerEndpointMetadata(endpointClass, null);
validateEndpointConfig(metadata.getConfig());
addEndpoint(metadata);
}
else
@ -159,39 +184,15 @@ public class ServerContainer extends ClientContainer implements javax.websocket.
@Override
public void addEndpoint(ServerEndpointConfig config) throws DeploymentException
{
if (config == null)
{
throw new DeploymentException("Unable to deploy null ServerEndpointConfig");
}
Class<?> endpointClass = config.getEndpointClass();
if (endpointClass == null)
{
throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
}
if (isStarted() || isStarting())
{
ServerEndpointConfig.Configurator configurator = config.getConfigurator();
if (configurator == null)
{
throw new DeploymentException("Unable to deploy with null ServerEndpointConfig.Configurator");
}
// only validate constructor and class modifiers on non-custom configurators
if (configurator.getClass() == ContainerDefaultConfigurator.class)
{
if (!ReflectUtils.isDefaultConstructable(endpointClass))
{
throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
}
}
validateEndpointConfig(config);
if (LOG.isDebugEnabled())
{
LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), config.getEndpointClass());
}
ServerEndpointMetadata metadata = getServerEndpointMetadata(config.getEndpointClass(), config);
addEndpoint(metadata);
}

View File

@ -18,12 +18,15 @@
package org.eclipse.jetty.websocket.jsr356.server;
import java.util.concurrent.TimeUnit;
import javax.websocket.CloseReason;
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.OnOpen;
import javax.websocket.Session;
import javax.websocket.WebSocketContainer;
import javax.websocket.server.ServerEndpoint;
@ -35,6 +38,8 @@ import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.websocket.api.util.WSURI;
import org.eclipse.jetty.websocket.jsr356.server.deploy.WebSocketServerContainerInitializer;
import org.eclipse.jetty.websocket.jsr356.server.samples.BasicOpenCloseSocket;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@ -44,8 +49,9 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class PrivateEndpointTest
public class AddEndpointTest
{
private Server server;
private WebSocketContainer client;
@ -96,6 +102,7 @@ public class PrivateEndpointTest
}
}
@SuppressWarnings("InnerClassMayBeStatic")
private class CustomPrivateEndpoint extends Endpoint
{
@Override
@ -104,6 +111,30 @@ public class PrivateEndpointTest
}
}
@SuppressWarnings("InnerClassMayBeStatic")
@ServerEndpoint(value = "/", configurator = CustomAnnotatedEndpointConfigurator.class)
public static class CustomAnnotatedEndpoint
{
public CustomAnnotatedEndpoint(String id)
{
}
@OnOpen
public void onOpen(Session session, EndpointConfig config)
{
}
}
public static class CustomAnnotatedEndpointConfigurator extends ServerEndpointConfig.Configurator
{
@SuppressWarnings("unchecked")
@Override
public <T> T getEndpointInstance(Class<T> endpointClass)
{
return (T)new CustomAnnotatedEndpoint("server");
}
}
public static class CustomEndpoint extends Endpoint implements MessageHandler.Whole<String>
{
public CustomEndpoint(String id)
@ -186,8 +217,12 @@ public class PrivateEndpointTest
}).build();
start(container -> container.addEndpoint(config));
Session session = client.connectToServer(new CustomEndpoint("client"), WSURI.toWebsocket(server.getURI().resolve("/")));
BasicOpenCloseSocket clientEndpoint = new BasicOpenCloseSocket();
Session session = client.connectToServer(clientEndpoint, WSURI.toWebsocket(server.getURI().resolve("/")));
assertNotNull(session);
session.close();
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
assertThat(clientEndpoint.closeReason.getCloseCode(), Matchers.is(CloseReason.CloseCodes.NORMAL_CLOSURE));
}
@Test
@ -205,8 +240,25 @@ public class PrivateEndpointTest
}).build();
start(container -> container.addEndpoint(config));
Session session = client.connectToServer(new CustomEndpoint("client"), WSURI.toWebsocket(server.getURI().resolve("/")));
BasicOpenCloseSocket clientEndpoint = new BasicOpenCloseSocket();
Session session = client.connectToServer(clientEndpoint, WSURI.toWebsocket(server.getURI().resolve("/")));
assertNotNull(session);
session.close();
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
assertThat(clientEndpoint.closeReason.getCloseCode(), Matchers.is(CloseReason.CloseCodes.NORMAL_CLOSURE));
}
@Test
public void testCustomAnnotatedEndpoint() throws Exception
{
start(container -> container.addEndpoint(CustomAnnotatedEndpoint.class));
BasicOpenCloseSocket clientEndpoint = new BasicOpenCloseSocket();
Session session = client.connectToServer(clientEndpoint, WSURI.toWebsocket(server.getURI().resolve("/")));
assertNotNull(session);
session.close();
assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
assertThat(clientEndpoint.closeReason.getCloseCode(), Matchers.is(CloseReason.CloseCodes.NORMAL_CLOSURE));
}
@Test

View File

@ -18,6 +18,7 @@
package org.eclipse.jetty.websocket.jsr356.server.samples;
import javax.websocket.ClientEndpoint;
import javax.websocket.CloseReason;
import javax.websocket.OnClose;
import javax.websocket.OnOpen;
@ -26,6 +27,7 @@ import javax.websocket.server.ServerEndpoint;
import org.eclipse.jetty.websocket.jsr356.server.TrackingSocket;
@ServerEndpoint(value = "/basic")
@ClientEndpoint
public class BasicOpenCloseSocket extends TrackingSocket
{
@OnOpen