Issue #4800 - invalid PathParam types should be reported at Deployment
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
This commit is contained in:
parent
870a765464
commit
fb85c71f8d
|
@ -40,11 +40,14 @@ import org.eclipse.jetty.client.HttpClient;
|
|||
import org.eclipse.jetty.util.annotation.ManagedObject;
|
||||
import org.eclipse.jetty.websocket.core.WebSocketComponents;
|
||||
import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
|
||||
import org.eclipse.jetty.websocket.core.exception.UpgradeException;
|
||||
import org.eclipse.jetty.websocket.core.exception.WebSocketTimeoutException;
|
||||
import org.eclipse.jetty.websocket.javax.common.ConfiguredEndpoint;
|
||||
import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketContainer;
|
||||
import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketExtensionConfig;
|
||||
import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandler;
|
||||
import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory;
|
||||
import org.eclipse.jetty.websocket.util.InvalidWebSocketException;
|
||||
|
||||
/**
|
||||
* Container for Client use of the javax.websocket API.
|
||||
|
@ -131,7 +134,8 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple
|
|||
{
|
||||
if (error != null)
|
||||
{
|
||||
futureSession.completeExceptionally(error);
|
||||
|
||||
futureSession.completeExceptionally(convertCause(error));
|
||||
return;
|
||||
}
|
||||
|
||||
|
@ -147,6 +151,18 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple
|
|||
return futureSession;
|
||||
}
|
||||
|
||||
public static Throwable convertCause(Throwable error)
|
||||
{
|
||||
if (error instanceof UpgradeException ||
|
||||
error instanceof WebSocketTimeoutException)
|
||||
return new IOException(error);
|
||||
|
||||
if (error instanceof InvalidWebSocketException)
|
||||
return new DeploymentException(error.getMessage(), error);
|
||||
|
||||
return error;
|
||||
}
|
||||
|
||||
private Session connect(ConfiguredEndpoint configuredEndpoint, URI destURI) throws IOException
|
||||
{
|
||||
Objects.requireNonNull(configuredEndpoint, "WebSocket configured endpoint cannot be null");
|
||||
|
|
|
@ -22,6 +22,7 @@ import java.lang.annotation.Annotation;
|
|||
import java.lang.reflect.Method;
|
||||
import javax.websocket.server.PathParam;
|
||||
|
||||
import org.eclipse.jetty.websocket.util.InvalidSignatureException;
|
||||
import org.eclipse.jetty.websocket.util.InvokerUtils;
|
||||
|
||||
/**
|
||||
|
@ -40,6 +41,7 @@ public class PathParamIdentifier implements InvokerUtils.ParamIdentifier
|
|||
{
|
||||
if (anno.annotationType().equals(PathParam.class))
|
||||
{
|
||||
validateType(paramType);
|
||||
PathParam pathParam = (PathParam)anno;
|
||||
return new InvokerUtils.Arg(paramType, pathParam.value());
|
||||
}
|
||||
|
@ -47,4 +49,22 @@ public class PathParamIdentifier implements InvokerUtils.ParamIdentifier
|
|||
}
|
||||
return new InvokerUtils.Arg(paramType);
|
||||
}
|
||||
|
||||
/**
|
||||
* The JSR356 rules for @PathParam only support
|
||||
* String, Primitive Types (and their Boxed version)
|
||||
*/
|
||||
public static void validateType(Class<?> type)
|
||||
{
|
||||
if (!String.class.isAssignableFrom(type) &&
|
||||
!Integer.TYPE.isAssignableFrom(type) &&
|
||||
!Long.TYPE.isAssignableFrom(type) &&
|
||||
!Short.TYPE.isAssignableFrom(type) &&
|
||||
!Float.TYPE.isAssignableFrom(type) &&
|
||||
!Double.TYPE.isAssignableFrom(type) &&
|
||||
!Boolean.TYPE.isAssignableFrom(type) &&
|
||||
!Character.TYPE.isAssignableFrom(type) &&
|
||||
!Byte.TYPE.isAssignableFrom(type))
|
||||
throw new InvalidSignatureException("Unsupported PathParam Type: " + type);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -18,21 +18,25 @@
|
|||
|
||||
package org.eclipse.jetty.websocket.javax.tests;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.nio.file.Path;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import javax.websocket.ContainerProvider;
|
||||
import javax.websocket.Session;
|
||||
import javax.websocket.WebSocketContainer;
|
||||
|
||||
import org.eclipse.jetty.annotations.ServletContainerInitializersStarter;
|
||||
import org.eclipse.jetty.logging.StacklessLogging;
|
||||
import org.eclipse.jetty.tests.webapp.websocket.bad.BadOnCloseServerEndpoint;
|
||||
import org.eclipse.jetty.tests.webapp.websocket.bad.BadOnOpenServerEndpoint;
|
||||
import org.eclipse.jetty.tests.webapp.websocket.bad.StringSequence;
|
||||
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
|
||||
import org.eclipse.jetty.webapp.WebAppContext;
|
||||
import org.hamcrest.Matchers;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
|
||||
public class DoubleDeployTest
|
||||
{
|
||||
|
@ -60,7 +64,13 @@ public class DoubleDeployTest
|
|||
app2.copyClass(StringSequence.class);
|
||||
app2.deploy();
|
||||
|
||||
server.start();
|
||||
app1.getWebAppContext().setThrowUnavailableOnStartupException(false);
|
||||
app2.getWebAppContext().setThrowUnavailableOnStartupException(false);
|
||||
|
||||
try (StacklessLogging ignore = new StacklessLogging(ServletContainerInitializersStarter.class, WebAppContext.class))
|
||||
{
|
||||
server.start();
|
||||
}
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
|
@ -70,22 +80,19 @@ public class DoubleDeployTest
|
|||
}
|
||||
|
||||
@Test
|
||||
public void test() throws Exception
|
||||
public void test()
|
||||
{
|
||||
// Initially just test deployment. (We should fail at deployment anyway).
|
||||
if (true) return;
|
||||
|
||||
WebSocketContainer client = ContainerProvider.getWebSocketContainer();
|
||||
EventSocket clientSocket = new EventSocket();
|
||||
Session session = client.connectToServer(clientSocket, server.getWsUri().resolve(app1.getContextPath() + "/badonclose/a"));
|
||||
session.getBasicRemote().sendText("test");
|
||||
session.close();
|
||||
assertTrue(clientSocket.closeLatch.await(5, TimeUnit.SECONDS));
|
||||
|
||||
clientSocket = new EventSocket();
|
||||
session = client.connectToServer(clientSocket, server.getWsUri().resolve(app2.getContextPath() + "/badonopen/b"));
|
||||
session.getBasicRemote().sendText("test");
|
||||
session.close();
|
||||
assertTrue(clientSocket.closeLatch.await(5, TimeUnit.SECONDS));
|
||||
Throwable error = assertThrows(Throwable.class, () ->
|
||||
client.connectToServer(clientSocket, server.getWsUri().resolve(app1.getContextPath() + "/badonclose/a")));
|
||||
assertThat(error, Matchers.instanceOf(IOException.class));
|
||||
assertThat(error.getMessage(), Matchers.containsString("503 Service Unavailable"));
|
||||
|
||||
error = assertThrows(Throwable.class, () ->
|
||||
client.connectToServer(clientSocket, server.getWsUri().resolve(app2.getContextPath() + "/badonclose/a")));
|
||||
assertThat(error, Matchers.instanceOf(IOException.class));
|
||||
assertThat(error.getMessage(), Matchers.containsString("503 Service Unavailable"));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -439,19 +439,14 @@ public class DistributionTests extends AbstractDistributionTest
|
|||
try (DistributionTester.Run run2 = distribution.start(args2))
|
||||
{
|
||||
assertTrue(run2.awaitConsoleLogsFor("Started Server@", 10, TimeUnit.SECONDS));
|
||||
// we do not test that anymore because it doesn't work for java14
|
||||
assertFalse(run2.getLogs().stream().anyMatch(s -> s.contains("LinkageError")));
|
||||
|
||||
startHttpClient();
|
||||
ContentResponse response = client.GET("http://localhost:" + port + "/test1/index.jsp");
|
||||
assertEquals(HttpStatus.OK_200, response.getStatus());
|
||||
assertThat(response.getContentAsString(), containsString("Hello"));
|
||||
assertThat(response.getContentAsString(), not(containsString("<%")));
|
||||
assertEquals(HttpStatus.SERVICE_UNAVAILABLE_503, response.getStatus());
|
||||
|
||||
client.GET("http://localhost:" + port + "/test2/index.jsp");
|
||||
assertEquals(HttpStatus.OK_200, response.getStatus());
|
||||
assertThat(response.getContentAsString(), containsString("Hello"));
|
||||
assertThat(response.getContentAsString(), not(containsString("<%")));
|
||||
assertEquals(HttpStatus.SERVICE_UNAVAILABLE_503, response.getStatus());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue