From 272679b39c988d152ce80de3aefb6fe1082e54c7 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Jun 2017 14:23:09 -0700 Subject: [PATCH] Reviewing more @Ignore'd WebSocket tests --- .../tests/AbstractTrackingEndpoint.java | 2 +- .../tests/client/BadNetworkTest.java | 145 -------------- .../tests/client/ClientCloseTest.java | 57 ++---- ...eTest.java => ClientDisconnectedTest.java} | 178 ++++++++++++++++-- .../tests/client/SlowServerTest.java | 2 - .../DelayedStartClientOnServerTest.java | 2 - .../test/resources/jetty-logging.properties | 2 +- 7 files changed, 179 insertions(+), 209 deletions(-) delete mode 100644 jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java rename jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/{ClientEarlyCloseTest.java => ClientDisconnectedTest.java} (64%) diff --git a/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/AbstractTrackingEndpoint.java b/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/AbstractTrackingEndpoint.java index 601757fd277..f4986e0abaf 100644 --- a/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/AbstractTrackingEndpoint.java +++ b/jetty-websocket/websocket-tests/src/main/java/org/eclipse/jetty/websocket/tests/AbstractTrackingEndpoint.java @@ -72,7 +72,7 @@ public abstract class AbstractTrackingEndpoint public void assertNotClosed(String prefix) { - assertTrue(prefix + " close event should not have occurred", closeLatch.getCount() > 0); + assertTrue(prefix + " close event should not have occurred: got " + closeInfo.get(), closeLatch.getCount() > 0); } public void assertNotOpened(String prefix) diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java deleted file mode 100644 index a8740a87084..00000000000 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/BadNetworkTest.java +++ /dev/null @@ -1,145 +0,0 @@ -// -// ======================================================================== -// 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 static org.hamcrest.CoreMatchers.containsString; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; - -import java.net.URI; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; - -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.tests.Defaults; -import org.eclipse.jetty.websocket.tests.LeakTrackingBufferPoolRule; -import org.eclipse.jetty.websocket.tests.TrackingEndpoint; -import org.eclipse.jetty.websocket.tests.UntrustedWSServer; -import org.eclipse.jetty.websocket.tests.UntrustedWSSession; -import org.junit.After; -import org.junit.Before; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TestName; - -/** - * Tests for conditions due to bad networking. - */ -public class BadNetworkTest -{ - @Rule - public TestName testname = new TestName(); - - @Rule - public LeakTrackingBufferPoolRule bufferPool = new LeakTrackingBufferPoolRule("Test"); - - private UntrustedWSServer server; - private WebSocketClient client; - - @Before - public void startClient() throws Exception - { - client = new WebSocketClient(bufferPool); - client.getPolicy().setIdleTimeout(250); - client.start(); - } - - @Before - public void startServer() throws Exception - { - server = new UntrustedWSServer(); - server.start(); - } - - @After - public void stopClient() throws Exception - { - client.stop(); - } - - @After - public void stopServer() throws Exception - { - server.stop(); - } - - @Test - @Ignore("Not working yet") - public void testAbruptClientClose() throws Exception - { - TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); - - URI wsUri = server.getUntrustedWsUri(this.getClass(), testname); - - Future clientConnectFuture = client.connect(clientSocket, wsUri); - - // Validate that we are connected - assertThat("Client Open Event Received", clientSocket.openLatch.await(30, TimeUnit.SECONDS), is(true)); - - // Have client disconnect abruptly - Session clientSession = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - clientSession.disconnect(); - - // Client Socket should see close - clientSocket.awaitCloseEvent("Client"); - - // Client Socket should see a close event, with status NO_CLOSE - // This event is automatically supplied by the underlying WebSocketClientConnection - // in the situation of a bad network connection. - clientSocket.assertCloseInfo("Client", StatusCode.NO_CLOSE, containsString("disconnect")); - } - - @Test - @Ignore("Not working yet") - public void testAbruptServerClose() throws Exception - { - TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); - - URI wsUri = server.getUntrustedWsUri(this.getClass(), testname); - - CompletableFuture sessionFuture = new CompletableFuture() - { - @Override - public boolean complete(UntrustedWSSession session) - { - // server disconnect - session.disconnect(); - return super.complete(session); - } - }; - server.registerOnOpenFuture(wsUri, sessionFuture); - Future clientConnectFuture = client.connect(clientSocket, wsUri); - Session clientSession = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - - // Validate that we are connected - clientSocket.awaitOpenEvent("Client"); - - // Wait for close (as response to idle timeout) - clientSocket.awaitCloseEvent("Client"); - - // Client Socket should see a close event, with status NO_CLOSE - // This event is automatically supplied by the underlying WebSocketClientConnection - // in the situation of a bad network connection. - clientSocket.assertCloseInfo("Client", StatusCode.PROTOCOL, containsString("EOF")); - } -} diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java index ab4e6b9f026..846ce7b08b6 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientCloseTest.java @@ -51,12 +51,12 @@ import org.eclipse.jetty.util.thread.Scheduler; import org.eclipse.jetty.websocket.api.BatchMode; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.api.WebSocketAdapter; import org.eclipse.jetty.websocket.client.WebSocketClient; import org.eclipse.jetty.websocket.common.OpCode; import org.eclipse.jetty.websocket.common.WebSocketFrame; import org.eclipse.jetty.websocket.tests.Defaults; import org.eclipse.jetty.websocket.tests.TrackingEndpoint; -import org.eclipse.jetty.websocket.tests.UntrustedWSConnection; import org.eclipse.jetty.websocket.tests.UntrustedWSEndpoint; import org.eclipse.jetty.websocket.tests.UntrustedWSServer; import org.eclipse.jetty.websocket.tests.UntrustedWSSession; @@ -166,6 +166,20 @@ public class ClientCloseTest public void startServer() throws Exception { server = new UntrustedWSServer(); + server.registerWebSocket("/noclose", (req, resp) -> new WebSocketAdapter() + { + @Override + public void onWebSocketClose(int statusCode, String reason) + { + try + { + getSession().disconnect(); + } + catch (IOException ignore) + { + } + } + }); server.start(); } @@ -230,47 +244,6 @@ public class ClientCloseTest clientSocket.assertErrorEvent("Client", instanceOf(SocketTimeoutException.class), anything()); } - @Test - @Ignore("Not working yet") - public void testServerNoCloseHandshake() throws Exception - { - // 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); - - // Client connects - TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); - Future clientConnectFuture = client.connect(clientSocket, wsUri); - - // Server accepts connect - UntrustedWSSession serverSession = serverSessionFut.get(10, TimeUnit.SECONDS); - UntrustedWSConnection serverConn = serverSession.getUntrustedConnection(); - - // client confirms connection via echo - confirmConnection(clientSocket, clientConnectFuture, serverSession); - - // client sends close frame - final String origCloseReason = "Normal Close"; - clientSocket.close(StatusCode.NORMAL, origCloseReason); - - // server receives close frame - serverSession.getUntrustedEndpoint().awaitCloseEvent("Server"); - serverSession.getUntrustedEndpoint().assertCloseInfo("Server", StatusCode.NORMAL, is(origCloseReason)); - - // client should not have received close message (yet) - clientSocket.assertNotClosed("Client"); - - // server never sends close frame handshake - // server sits idle - - // client idle timeout triggers close event on client ws-endpoint - clientSocket.assertErrorEvent("Client", instanceOf(SocketTimeoutException.class), containsString("Timeout on Read")); - } - @Test(timeout = 5000L) public void testStopLifecycle() throws Exception { diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientEarlyCloseTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientDisconnectedTest.java similarity index 64% rename from jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientEarlyCloseTest.java rename to jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientDisconnectedTest.java index fcdccd92e52..511972b6f3e 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientEarlyCloseTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/ClientDisconnectedTest.java @@ -34,10 +34,12 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jetty.client.HttpResponseException; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.util.log.StacklessLogging; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketAdapter; import org.eclipse.jetty.websocket.api.WebSocketTimeoutException; +import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect; import org.eclipse.jetty.websocket.api.annotations.WebSocket; import org.eclipse.jetty.websocket.client.ClientUpgradeRequest; @@ -58,9 +60,9 @@ import org.junit.rules.ExpectedException; import org.junit.rules.TestName; /** - * Tests various early drop/close scenarios + * Tests various early disconnected connection situations */ -public class ClientEarlyCloseTest +public class ClientDisconnectedTest { /** * On Open, close socket @@ -123,6 +125,52 @@ public class ClientEarlyCloseTest } } + /** + * On Close, drop connection + */ + @WebSocket + public static class CloseDropSocket + { + private static final Logger LOG = Log.getLogger(CloseDropSocket.class); + + @OnWebSocketClose + public void onClose(Session session) + { + LOG.debug("onClose({})", session); + try + { + session.disconnect(); + } + catch (IOException ignore) + { + } + } + } + + /** + * On Close, no reply + */ + @WebSocket + public static class CloseNoReplySocket + { + private static final Logger LOG = Log.getLogger(CloseDropSocket.class); + + @OnWebSocketClose + public void onClose(Session session) + { + LOG.debug("onClose({})", session); + try + { + // Take too long to reply + // The client should see an idle timeout (no reply from server) + TimeUnit.SECONDS.sleep(5); + } + catch (InterruptedException ignore) + { + } + } + } + public static class EarlyCloseServlet extends WebSocketServlet implements WebSocketCreator { @Override @@ -152,6 +200,18 @@ public class ClientEarlyCloseTest return new MessageDropSocket(); } + if (req.hasSubProtocol("closedrop")) + { + resp.setAcceptedSubProtocol("closedrop"); + return new CloseDropSocket(); + } + + if (req.hasSubProtocol("closenoreply")) + { + resp.setAcceptedSubProtocol("closenoreply"); + return new CloseDropSocket(); + } + return null; } } @@ -227,24 +287,28 @@ public class ClientEarlyCloseTest TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); URI wsUri = server.getServerUri().resolve("/"); - Future clientConnectFuture = client.connect(clientSocket, wsUri, upgradeRequest); - Session session = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - - try + try(StacklessLogging ignore = new StacklessLogging(OpenFailSocket.class)) { - clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + Future clientConnectFuture = client.connect(clientSocket, wsUri, upgradeRequest); - assertThat("OnOpen.UpgradeRequest", clientSocket.openUpgradeRequest, notNullValue()); - assertThat("OnOpen.UpgradeResponse", clientSocket.openUpgradeResponse, notNullValue()); - assertThat("Negotiated SubProtocol", clientSocket.openUpgradeResponse.getAcceptedSubProtocol(), is("openfail")); + Session session = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); - clientSocket.awaitCloseEvent("Client"); - clientSocket.assertCloseInfo("Client", StatusCode.SERVER_ERROR, anything()); - } - finally - { - session.close(); + try + { + clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + assertThat("OnOpen.UpgradeRequest", clientSocket.openUpgradeRequest, notNullValue()); + assertThat("OnOpen.UpgradeResponse", clientSocket.openUpgradeResponse, notNullValue()); + assertThat("Negotiated SubProtocol", clientSocket.openUpgradeResponse.getAcceptedSubProtocol(), is("openfail")); + + clientSocket.awaitCloseEvent("Client"); + clientSocket.assertCloseInfo("Client", StatusCode.SERVER_ERROR, anything()); + } + finally + { + session.close(); + } } } @@ -288,4 +352,86 @@ public class ClientEarlyCloseTest session.close(); } } + + /** + * The connection has performed handshake successfully. + *

+ * Client sends close handshake, remote drops connection with no reply + *

+ * + * @throws Exception on test failure + */ + @Test + public void closeDrop() throws Exception + { + ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); + upgradeRequest.setSubProtocols("closedrop"); + + TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); + + URI wsUri = server.getServerUri().resolve("/"); + client.setMaxIdleTimeout(3000); + Future clientConnectFuture = client.connect(clientSocket, wsUri, upgradeRequest); + + Session session = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + try + { + clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + assertThat("OnOpen.UpgradeRequest", clientSocket.openUpgradeRequest, notNullValue()); + assertThat("OnOpen.UpgradeResponse", clientSocket.openUpgradeResponse, notNullValue()); + assertThat("Negotiated SubProtocol", clientSocket.openUpgradeResponse.getAcceptedSubProtocol(), is("closedrop")); + + clientSocket.close(StatusCode.NORMAL, "All Done"); + + clientSocket.awaitErrorEvent("Client"); + clientSocket.assertErrorEvent("Client", instanceOf(WebSocketTimeoutException.class), containsString("Connection Idle Timeout")); + } + finally + { + session.close(); + } + } + + /** + * The connection has performed handshake successfully. + *

+ * Client sends close handshake, remote never replies (but leaves connection open) + *

+ * + * @throws Exception on test failure + */ + @Test + public void closeNoReply() throws Exception + { + ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(); + upgradeRequest.setSubProtocols("closenoreply"); + + TrackingEndpoint clientSocket = new TrackingEndpoint(testname.getMethodName()); + + URI wsUri = server.getServerUri().resolve("/"); + client.setMaxIdleTimeout(3000); + Future clientConnectFuture = client.connect(clientSocket, wsUri, upgradeRequest); + + Session session = clientConnectFuture.get(Defaults.CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + try + { + clientSocket.openLatch.await(Defaults.OPEN_EVENT_TIMEOUT_MS, TimeUnit.MILLISECONDS); + + assertThat("OnOpen.UpgradeRequest", clientSocket.openUpgradeRequest, notNullValue()); + assertThat("OnOpen.UpgradeResponse", clientSocket.openUpgradeResponse, notNullValue()); + assertThat("Negotiated SubProtocol", clientSocket.openUpgradeResponse.getAcceptedSubProtocol(), is("closenoreply")); + + clientSocket.close(StatusCode.NORMAL, "All Done"); + + clientSocket.awaitErrorEvent("Client"); + clientSocket.assertErrorEvent("Client", instanceOf(WebSocketTimeoutException.class), containsString("Connection Idle Timeout")); + } + finally + { + session.close(); + } + } } diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowServerTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowServerTest.java index abb548de6b3..c7463f15894 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowServerTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/SlowServerTest.java @@ -77,7 +77,6 @@ public class SlowServerTest } @Test - @Ignore("Not working yet") public void testServerSlowToRead() throws Exception { TrackingEndpoint clientEndpoint = new TrackingEndpoint(testname.getMethodName()); @@ -125,7 +124,6 @@ public class SlowServerTest } @Test - @Ignore("Not working yet") public void testServerSlowToSend() throws Exception { TrackingEndpoint clientEndpoint = new TrackingEndpoint(testname.getMethodName()); diff --git a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/jsr356/DelayedStartClientOnServerTest.java b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/jsr356/DelayedStartClientOnServerTest.java index 0a84ff17e55..07d4c4966d5 100644 --- a/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/jsr356/DelayedStartClientOnServerTest.java +++ b/jetty-websocket/websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/jsr356/DelayedStartClientOnServerTest.java @@ -61,7 +61,6 @@ import org.eclipse.jetty.websocket.api.util.WSURI; import org.eclipse.jetty.websocket.jsr356.ClientContainer; import org.eclipse.jetty.websocket.jsr356.server.ServerContainer; import org.eclipse.jetty.websocket.jsr356.server.deploy.WebSocketServerContainerInitializer; -import org.junit.Ignore; import org.junit.Test; public class DelayedStartClientOnServerTest @@ -234,7 +233,6 @@ public class DelayedStartClientOnServerTest } @Test - @Ignore("Not working yet") public void testHttpClientThreads_AfterClientConnectTo() throws Exception { Server server = new Server(0); 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 1c7cf791ed8..314719e45f2 100644 --- a/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/websocket-tests/src/test/resources/jetty-logging.properties @@ -47,6 +47,6 @@ org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.websocket.common.message.LEVEL=DEBUG ### Showing any unintended (ignored) errors from CompletionCallback -org.eclipse.jetty.websocket.common.CompletionCallback.LEVEL=ALL +# org.eclipse.jetty.websocket.common.CompletionCallback.LEVEL=ALL ### Disabling intentional error out of RFCSocket org.eclipse.jetty.websocket.tests.server.RFCSocket.LEVEL=OFF