From 4ec2c3eca2b06c9a43836a31bfc3b5d60b9a408b Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 26 Nov 2020 21:05:06 +1100 Subject: [PATCH] fix websocket proxy for jetty10 websocket api Signed-off-by: Lachlan Roberts --- .../core/internal/WebSocketCoreSession.java | 3 ++- .../websocket/tests/proxy/WebSocketProxy.java | 23 ++++--------------- .../tests/proxy/WebSocketProxyTest.java | 19 +++++++-------- 3 files changed, 16 insertions(+), 29 deletions(-) diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java index 8e87a622654..568ea2b9f91 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketCoreSession.java @@ -475,7 +475,8 @@ public class WebSocketCoreSession implements IncomingFrames, CoreSession, Dumpab }, x -> { - LOG.warn("Error during OPEN", x); + if (LOG.isDebugEnabled()) + LOG.debug("Error during OPEN", x); processHandlerError(new CloseException(CloseStatus.SERVER_ERROR, x), NOOP); }); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxy.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxy.java index abaa272ecb0..5ae50abe229 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxy.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxy.java @@ -23,7 +23,6 @@ import java.nio.ByteBuffer; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; @@ -78,7 +77,6 @@ public class WebSocketProxy { private volatile Session session; private final CountDownLatch closeLatch = new CountDownLatch(1); - private final AtomicInteger pingsReceived = new AtomicInteger(); public Session getSession() { @@ -156,9 +154,7 @@ public class WebSocketProxy try { - // The implementation automatically sends pong response. - pingsReceived.incrementAndGet(); - proxyToServer.getSession().getRemote().sendPing(BufferUtil.copy(payload)); + proxyToServer.getSession().getRemote().sendPing(payload); } catch (Exception e) { @@ -174,11 +170,7 @@ public class WebSocketProxy try { - // If we have sent out a ping then we have already responded with automatic pong. - // If this is an unsolicited pong we still need to forward it to the server. - int valueBeforeUpdate = proxyToServer.pingsReceived.getAndUpdate(i -> i > 0 ? i - 1 : i); - if (valueBeforeUpdate == 0) - proxyToServer.getSession().getRemote().sendPong(BufferUtil.copy(payload)); + proxyToServer.getSession().getRemote().sendPong(payload); } catch (Exception e) { @@ -213,7 +205,6 @@ public class WebSocketProxy { private volatile Session session; private final CountDownLatch closeLatch = new CountDownLatch(1); - private final AtomicInteger pingsReceived = new AtomicInteger(); public Session getSession() { @@ -277,9 +268,7 @@ public class WebSocketProxy try { - // The implementation automatically sends pong response. - pingsReceived.incrementAndGet(); - clientToProxy.getSession().getRemote().sendPing(BufferUtil.copy(payload)); + clientToProxy.getSession().getRemote().sendPing(payload); } catch (Exception e) { @@ -295,11 +284,7 @@ public class WebSocketProxy try { - // If we have sent out a ping then we have already responded with automatic pong. - // If this is an unsolicited pong we still need to forward it to the client. - int valueBeforeUpdate = clientToProxy.pingsReceived.getAndUpdate(i -> i > 0 ? i - 1 : i); - if (valueBeforeUpdate == 0) - clientToProxy.getSession().getRemote().sendPong(BufferUtil.copy(payload)); + clientToProxy.getSession().getRemote().sendPong(payload); } catch (Exception e) { diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxyTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxyTest.java index 59b848c7f4c..c6dffb11f34 100644 --- a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxyTest.java +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/proxy/WebSocketProxyTest.java @@ -161,16 +161,17 @@ public class WebSocketProxyTest client.connect(clientSocket, proxyUri); assertTrue(clientSocket.openLatch.await(5, TimeUnit.SECONDS)); + // TODO: Why is this server error when it is occurring on the client. // Verify expected client close. assertTrue(clientSocket.closeLatch.await(5, TimeUnit.SECONDS)); - assertThat(clientSocket.closeCode, is(StatusCode.NO_CLOSE)); - assertThat(clientSocket.closeReason, is("simulated onOpen error")); + assertThat(clientSocket.closeCode, is(StatusCode.SERVER_ERROR)); + assertThat(clientSocket.closeReason, containsString("simulated onOpen err")); assertNotNull(clientSocket.error); // Verify expected server close. assertTrue(serverSocket.closeLatch.await(5, TimeUnit.SECONDS)); - assertThat(serverSocket.closeCode, is(StatusCode.NO_CLOSE)); - assertThat(serverSocket.closeReason, is("Disconnected")); + assertThat(serverSocket.closeCode, is(StatusCode.SERVER_ERROR)); + assertThat(serverSocket.closeReason, containsString("simulated onOpen err")); assertNull(serverSocket.error); // WebSocketProxy has been completely closed. @@ -189,13 +190,13 @@ public class WebSocketProxyTest // Verify expected client close. assertTrue(clientSocket.closeLatch.await(5, TimeUnit.SECONDS)); assertThat(clientSocket.closeCode, is(StatusCode.SERVER_ERROR)); - assertThat(clientSocket.closeReason, is("simulated onOpen error")); + assertThat(clientSocket.closeReason, containsString("simulated onOpen err")); assertNull(clientSocket.error); // Verify expected server close. assertTrue(serverSocket.closeLatch.await(5, TimeUnit.SECONDS)); assertThat(serverSocket.closeCode, is(StatusCode.SERVER_ERROR)); - assertThat(serverSocket.closeReason, is("simulated onOpen error")); + assertThat(serverSocket.closeReason, containsString("simulated onOpen err")); assertNotNull(serverSocket.error); // WebSocketProxy has been completely closed. @@ -253,12 +254,12 @@ public class WebSocketProxyTest assertTrue(serverSocket.closeLatch.await(clientSessionIdleTimeout * 2, TimeUnit.MILLISECONDS)); // Check errors and close status. - assertThat(clientSocket.error.getMessage(), containsString("Idle timeout expired")); + assertThat(clientSocket.error.getMessage(), containsString("Connection Idle Timeout")); assertThat(clientSocket.closeCode, is(StatusCode.SHUTDOWN)); - assertThat(clientSocket.closeReason, containsString("Idle timeout expired")); + assertThat(clientSocket.closeReason, containsString("Connection Idle Timeout")); assertNull(serverSocket.error); assertThat(serverSocket.closeCode, is(StatusCode.SHUTDOWN)); - assertThat(serverSocket.closeReason, containsString("Idle timeout expired")); + assertThat(serverSocket.closeReason, containsString("Connection Idle Timeout")); } @Test