From fc15da79d4452b562df6e028c6716e078264fb51 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 14 Jun 2017 16:19:44 -0700 Subject: [PATCH 1/4] Issue #1616 enabling debug output for CI testing --- Jenkinsfile | 2 +- .../websocket/tests/client/ClientCloseHandshakeTest.java | 1 - .../src/test/resources/jetty-logging.properties | 6 +++--- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 0016dc734dd..7cd46bed746 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -53,7 +53,7 @@ node { withEnv(mvnEnv) { timeout(time: 90, unit: 'MINUTES') { // Run test phase / ignore test failures - sh "mvn -B install -Dmaven.test.failure.ignore=true" + sh "mvn -B install -Dmaven.test.failure.ignore=true -Dtest=ClientCloseHandshakeTest" // Report failures in the jenkins UI step([$class: 'JUnitResultArchiver', testResults: '**/target/surefire-reports/TEST-*.xml']) diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java index 6e6534301c1..04846385d9d 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java @@ -590,7 +590,6 @@ public class ClientCloseHandshakeTest * */ @Test - @Ignore("moving to branch to work out test issue on CI") public void testClient_ProtocolViolation_Received() throws Exception { // Set client timeout diff --git a/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties index 314719e45f2..8ef9c7134f3 100644 --- a/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties @@ -26,9 +26,9 @@ org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG # org.eclipse.jetty.io.FillInterest.LEVEL=DEBUG # org.eclipse.jetty.client.LEVEL=DEBUG -# org.eclipse.jetty.io.LEVEL=DEBUG -# org.eclipse.jetty.io.ManagedSelector.LEVEL=INFO -# org.eclipse.jetty.websocket.LEVEL=DEBUG +org.eclipse.jetty.io.LEVEL=DEBUG +org.eclipse.jetty.io.ManagedSelector.LEVEL=INFO +org.eclipse.jetty.websocket.LEVEL=DEBUG # org.eclipse.jetty.websocket.LEVEL=INFO # org.eclipse.jetty.websocket.jsr356.messages.LEVEL=DEBUG # org.eclipse.jetty.websocket.tests.LEVEL=DEBUG From e1ebcdaf185722bf8c4d6b43f3f56a61cb743c01 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 15 Jun 2017 13:34:17 -0700 Subject: [PATCH 2/4] Correcting error message --- .../eclipse/jetty/websocket/server/WebSocketServerFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java index 3f905df5fd7..82889c78e37 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java @@ -275,7 +275,7 @@ public class WebSocketServerFactory extends ContainerLifeCycle implements WebSoc } } - throw new InvalidWebSocketException("Unable to create Session: unrecognized internal EventDriver type: " + websocket.getClass().getName()); + throw new InvalidWebSocketException("Unable to create Session: object unrecognized as websocket: " + websocket.getClass().getName()); } /** From c496dd796122c1a4a7268b8563eb35b3cc6b4099 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 15 Jun 2017 13:35:40 -0700 Subject: [PATCH 3/4] Testing / UntrustedWSServer improvements --- .../websocket/tests/UntrustedWSServer.java | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/UntrustedWSServer.java b/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/UntrustedWSServer.java index e321d55dc45..8d33b0be96f 100644 --- a/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/UntrustedWSServer.java +++ b/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/UntrustedWSServer.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; +import java.util.function.Consumer; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -55,10 +56,11 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS private URI wsUri; private boolean ssl = false; private SslContextFactory sslContextFactory; - + private Consumer serverConsumer; + private Map> onOpenFutures = new ConcurrentHashMap<>(); private final ServletContextHandler context = new ServletContextHandler(); - + @Override protected void doStart() throws Exception { @@ -66,7 +68,7 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS String name = "qtp-untrustedWSServer-" + hashCode(); threadPool.setName(name); threadPool.setDaemon(true); - + // Configure Server server = new Server(threadPool); if (ssl) @@ -80,7 +82,7 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS http_config.setResponseHeaderSize(8192); http_config.setSendServerVersion(true); http_config.setSendDateHeader(false); - + sslContextFactory = new SslContextFactory(); sslContextFactory.setKeyStorePath(MavenTestingUtils.getTestResourceFile("keystore").getAbsolutePath()); sslContextFactory.setKeyStorePassword("storepwd"); @@ -88,11 +90,11 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS sslContextFactory.setExcludeCipherSuites("SSL_RSA_WITH_DES_CBC_SHA", "SSL_DHE_RSA_WITH_DES_CBC_SHA", "SSL_DHE_DSS_WITH_DES_CBC_SHA", "SSL_RSA_EXPORT_WITH_RC4_40_MD5", "SSL_RSA_EXPORT_WITH_DES40_CBC_SHA", "SSL_DHE_RSA_EXPORT_WITH_DES40_CBC_SHA", "SSL_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA"); - + // SSL HTTP Configuration HttpConfiguration https_config = new HttpConfiguration(http_config); https_config.addCustomizer(new SecureRequestCustomizer()); - + // SSL Connector connector = new ServerConnector(server, new SslConnectionFactory(sslContextFactory, HttpVersion.HTTP_1_1.asString()), new HttpConnectionFactory(https_config)); connector.setPort(0); @@ -104,58 +106,74 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS connector.setPort(0); } server.addConnector(connector); - + context.setContextPath("/"); server.setHandler(context); - + // Serve untrusted endpoint context.addServlet(UntrustedWSServlet.class, "/untrusted/*").setInitOrder(1); - + + // Allow for server customization + if (serverConsumer != null) + { + serverConsumer.accept(server); + } + // Start Server addBean(server); - + super.doStart(); - + // Wireup Context related things UntrustedWSSessionFactory sessionFactory = (UntrustedWSSessionFactory) context.getServletContext().getAttribute(UntrustedWSSessionFactory.class.getName()); sessionFactory.addListener(this); - + // Establish the Server URI URI serverUri = server.getURI(); wsUri = WSURI.toWebsocket(serverUri).resolve("/"); - + // Some debugging if (LOG.isDebugEnabled()) { LOG.debug("WebSocket Server URI: " + wsUri.toASCIIString()); server.dump(); } - + super.doStart(); } - + + public void setServerCustomizer(Consumer customizer) + { + this.serverConsumer = customizer; + } + + public void join() throws InterruptedException + { + server.join(); + } + public URI getWsUri() { return wsUri; } - + public URI getUntrustedWsUri(Class clazz, TestName testname) { return wsUri.resolve("/untrusted/" + clazz.getSimpleName() + "/" + testname.getMethodName()); } - + public void registerHttpService(String urlPattern, BiConsumer serviceConsumer) { ServletHolder holder = new ServletHolder(new BiConsumerServiceServlet(serviceConsumer)); context.addServlet(holder, urlPattern); } - + public void registerWebSocket(String urlPattern, WebSocketCreator creator) { ServletHolder holder = new ServletHolder(new UntrustedWSServlet(creator)); context.addServlet(holder, urlPattern); } - + @Override public void onSessionCreate(UntrustedWSSession session, URI requestURI) { @@ -167,7 +185,7 @@ public class UntrustedWSServer extends ContainerLifeCycle implements UntrustedWS this.onOpenFutures.put(requestURI, sessionFuture); } } - + public void registerOnOpenFuture(URI uri, CompletableFuture sessionFuture) { this.onOpenFutures.put(uri, sessionFuture); From d8775c77d933d01b9df06559f110152e67cb4ed3 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 15 Jun 2017 13:36:03 -0700 Subject: [PATCH 4/4] Issue #1616 - Making test more reliable --- .../tests/client/BadCloseSocket.java | 61 ++++++++++++++++ .../client/ClientCloseHandshakeTest.java | 71 +++++++------------ .../test/resources/jetty-logging.properties | 6 +- 3 files changed, 88 insertions(+), 50 deletions(-) create mode 100644 jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadCloseSocket.java diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadCloseSocket.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadCloseSocket.java new file mode 100644 index 00000000000..9f19d036783 --- /dev/null +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadCloseSocket.java @@ -0,0 +1,61 @@ +// +// ======================================================================== +// Copyright (c) 1995-2017 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// 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.tests.client; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Arrays; + +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.common.OpCode; +import org.eclipse.jetty.websocket.tests.RawFrameBuilder; +import org.eclipse.jetty.websocket.tests.UntrustedWSEndpoint; + +public class BadCloseSocket extends UntrustedWSEndpoint +{ + private static final Logger LOG = Log.getLogger(BadCloseSocket.class); + + public BadCloseSocket(String id) + { + super(id); + this.setOnTextFunction((untrustedWSSession, message) -> { + LOG.debug("onTextMessage({})", message); + try + { + byte reason[] = new byte[400]; + Arrays.fill(reason, (byte) 'x'); + ByteBuffer bad = ByteBuffer.allocate(500); + RawFrameBuilder.putOpFin(bad, OpCode.CLOSE, true); + RawFrameBuilder.putLength(bad, reason.length + 2, false); + bad.putShort((short) StatusCode.NORMAL); + bad.put(reason); + BufferUtil.flipToFlush(bad, 0); + untrustedWSSession.getUntrustedConnection().writeRaw(bad); + } + catch (IOException e) + { + throw new RuntimeException("Unable to issue bad control frame", e); + } + return null; + }); + } +} diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java index 04846385d9d..0be3b6bb8d0 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseHandshakeTest.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.websocket.tests.client; -import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -31,7 +30,6 @@ import java.nio.ByteBuffer; import java.nio.channels.SelectableChannel; import java.nio.channels.SelectionKey; import java.nio.channels.SocketChannel; -import java.util.Arrays; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -44,7 +42,6 @@ import org.eclipse.jetty.io.EofException; import org.eclipse.jetty.io.ManagedSelector; import org.eclipse.jetty.io.SelectorManager; import org.eclipse.jetty.io.SocketChannelEndPoint; -import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.log.StacklessLogging; @@ -54,10 +51,7 @@ import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.client.WebSocketClient; -import org.eclipse.jetty.websocket.common.OpCode; -import org.eclipse.jetty.websocket.common.Parser; import org.eclipse.jetty.websocket.tests.Defaults; -import org.eclipse.jetty.websocket.tests.RawFrameBuilder; import org.eclipse.jetty.websocket.tests.TrackingEndpoint; import org.eclipse.jetty.websocket.tests.UntrustedWSServer; import org.eclipse.jetty.websocket.tests.UntrustedWSSession; @@ -128,6 +122,8 @@ public class ClientCloseHandshakeTest public void startServer() throws Exception { server = new UntrustedWSServer(); + + server.registerWebSocket("/badclose", (req, resp) -> new BadCloseSocket("SERVER")); server.start(); } @@ -595,55 +591,36 @@ public class ClientCloseHandshakeTest // Set client timeout final int timeout = 1000; client.setMaxIdleTimeout(timeout); - - URI wsUri = server.getUntrustedWsUri(this.getClass(), testname); - CompletableFuture serverSessionFut = new CompletableFuture<>(); - server.registerOnOpenFuture(wsUri, serverSessionFut); - + + URI wsUri = server.getWsUri().resolve("/badclose"); + // Client connects TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); Future clientConnectFuture = client.connect(clientSocket, wsUri); - - // Server accepts connect - UntrustedWSSession serverSession = serverSessionFut.get(10, TimeUnit.SECONDS); - + // Wait for client connect on via future Session clientSession = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - clientSession.getRemote().setBatchMode(BatchMode.OFF); - - // Wait for client connect via client websocket - assertThat("Client WebSocket is Open", clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS), is(true)); - - // client should not have received close message (yet) - clientSocket.assertNotClosed("Client"); - - // server sends bad close frame (too big of a reason message) - byte msg[] = new byte[400]; - Arrays.fill(msg, (byte) 'x'); - ByteBuffer bad = ByteBuffer.allocate(500); - RawFrameBuilder.putOpFin(bad, OpCode.CLOSE, true); - RawFrameBuilder.putLength(bad, msg.length + 2, false); - bad.putShort((short) StatusCode.NORMAL); - bad.put(msg); - BufferUtil.flipToFlush(bad, 0); - try (StacklessLogging ignored = new StacklessLogging(Parser.class)) + try { - serverSession.getUntrustedConnection().writeRaw(bad); - - // client should have noticed the error + clientSession.getRemote().setBatchMode(BatchMode.OFF); + + // Wait for client connect via client websocket + assertThat("Client WebSocket is Open", clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS), is(true)); + + // client should not have received close message (yet) + clientSocket.assertNotClosed("Client"); + + // Trigger behavior on server side with message + clientSession.getRemote().sendString("fail-now"); + + // client error event on ws-endpoint + clientSocket.awaitErrorEvent("Client"); clientSocket.assertErrorEvent("Client", instanceOf(ProtocolException.class), containsString("Invalid control frame")); - - // client parse invalid frame, notifies server of close (protocol error) - serverSession.getUntrustedEndpoint().awaitCloseEvent("Server"); - serverSession.getUntrustedEndpoint().assertCloseInfo("Server", StatusCode.PROTOCOL, allOf(containsString("Invalid control frame"), containsString("length"))); } - - // server disconnects - serverSession.disconnect(); - - // client error event on ws-endpoint - clientSocket.awaitErrorEvent("Client"); - clientSocket.assertErrorEvent("Client", instanceOf(ProtocolException.class), containsString("Invalid control frame")); + finally + { + clientSession.close(); + } } /** diff --git a/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties index 8ef9c7134f3..314719e45f2 100644 --- a/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties @@ -26,9 +26,9 @@ org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG # org.eclipse.jetty.io.FillInterest.LEVEL=DEBUG # org.eclipse.jetty.client.LEVEL=DEBUG -org.eclipse.jetty.io.LEVEL=DEBUG -org.eclipse.jetty.io.ManagedSelector.LEVEL=INFO -org.eclipse.jetty.websocket.LEVEL=DEBUG +# org.eclipse.jetty.io.LEVEL=DEBUG +# org.eclipse.jetty.io.ManagedSelector.LEVEL=INFO +# org.eclipse.jetty.websocket.LEVEL=DEBUG # org.eclipse.jetty.websocket.LEVEL=INFO # org.eclipse.jetty.websocket.jsr356.messages.LEVEL=DEBUG # org.eclipse.jetty.websocket.tests.LEVEL=DEBUG