From 646707b85ca265cf70a9642fa89b57b4eef849b5 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 12 Aug 2021 15:50:25 +1000 Subject: [PATCH] Issue #6602 - do not invoke SessionListener onOpen if session has been closed in OnOpen Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/CoreSession.java | 2 +- .../common/JavaxWebSocketFrameHandler.java | 7 +- .../javax/tests/CloseInOnOpenTest.java | 97 +++++++++++++++++++ .../common/JettyWebSocketFrameHandler.java | 5 +- .../websocket/tests/CloseInOnOpenTest.java | 95 ++++++++++++++++++ 5 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/CloseInOnOpenTest.java create mode 100644 jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseInOnOpenTest.java diff --git a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java index 7c6bd9bb4ec0..bbaf05c4e5a7 100644 --- a/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java +++ b/jetty-websocket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java @@ -256,7 +256,7 @@ public SocketAddress getRemoteAddress() @Override public boolean isOutputOpen() { - return false; + return true; } @Override diff --git a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java index 99cfd8bb2e6f..030312178243 100644 --- a/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java +++ b/jetty-websocket/websocket-javax-common/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java @@ -133,6 +133,9 @@ public void onOpen(CoreSession coreSession, Callback callback) // Rewire EndpointConfig to call CoreSession setters if Jetty specific properties are set. endpointConfig = getWrappedEndpointConfig(); session = new JavaxWebSocketSession(container, coreSession, this, endpointConfig); + if (!session.isOpen()) + throw new IllegalStateException("Session is not open"); + openHandle = InvokerUtils.bindTo(openHandle, session, endpointConfig); closeHandle = InvokerUtils.bindTo(closeHandle, session); errorHandle = InvokerUtils.bindTo(errorHandle, session); @@ -171,7 +174,9 @@ public void onOpen(CoreSession coreSession, Callback callback) if (openHandle != null) openHandle.invoke(); - container.notifySessionListeners((listener) -> listener.onJavaxWebSocketSessionOpened(session)); + if (session.isOpen()) + container.notifySessionListeners((listener) -> listener.onJavaxWebSocketSessionOpened(session)); + callback.succeeded(); } catch (Throwable cause) diff --git a/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/CloseInOnOpenTest.java b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/CloseInOnOpenTest.java new file mode 100644 index 000000000000..859a8568e5e6 --- /dev/null +++ b/jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/CloseInOnOpenTest.java @@ -0,0 +1,97 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.websocket.javax.tests; + +import java.net.URI; +import java.util.concurrent.TimeUnit; +import javax.websocket.CloseReason; +import javax.websocket.OnOpen; +import javax.websocket.Session; +import javax.websocket.server.ServerEndpoint; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer; +import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer; +import org.eclipse.jetty.websocket.javax.server.internal.JavaxWebSocketServerContainer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class CloseInOnOpenTest +{ + private Server server; + private ServerConnector connector; + private JavaxWebSocketServerContainer serverContainer; + private JavaxWebSocketClientContainer client; + + @BeforeEach + public void beforeEach() throws Exception + { + server = new Server(); + + connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + server.setHandler(context); + + JavaxWebSocketServletContainerInitializer.configure(context, (servletContext, wsContainer) -> + wsContainer.addEndpoint(ClosingListener.class)); + server.start(); + + serverContainer = JavaxWebSocketServerContainer.getContainer(context.getServletContext()); + assertNotNull(serverContainer); + + client = new JavaxWebSocketClientContainer(); + client.start(); + } + + @AfterEach + public void afterEach() throws Exception + { + client.stop(); + server.stop(); + } + + @Test + public void testCloseInOnWebSocketConnect() throws Exception + { + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/ws"); + EventSocket clientEndpoint = new EventSocket(); + + client.connectToServer(clientEndpoint, uri); + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeReason.getCloseCode(), is(CloseReason.CloseCodes.VIOLATED_POLICY)); + + assertThat(serverContainer.getOpenSessions().size(), is(0)); + } + + @ServerEndpoint("/ws") + public static class ClosingListener + { + @OnOpen + public void onWebSocketConnect(Session session) throws Exception + { + session.close(new CloseReason(CloseReason.CloseCodes.VIOLATED_POLICY, "I am a WS that closes immediately")); + } + } +} diff --git a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java index d3235ad0ed73..992c9042dd2a 100644 --- a/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java +++ b/jetty-websocket/websocket-jetty-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java @@ -151,6 +151,8 @@ public void onOpen(CoreSession coreSession, Callback callback) { customizer.customize(coreSession); session = new WebSocketSession(container, coreSession, this); + if (!session.isOpen()) + throw new IllegalStateException("Session is not open"); frameHandle = InvokerUtils.bindTo(frameHandle, session); openHandle = InvokerUtils.bindTo(openHandle, session); @@ -172,7 +174,8 @@ public void onOpen(CoreSession coreSession, Callback callback) if (openHandle != null) openHandle.invoke(); - container.notifySessionListeners((listener) -> listener.onWebSocketSessionOpened(session)); + if (session.isOpen()) + container.notifySessionListeners((listener) -> listener.onWebSocketSessionOpened(session)); callback.succeeded(); demand(); diff --git a/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseInOnOpenTest.java b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseInOnOpenTest.java new file mode 100644 index 000000000000..903c51c5ed6d --- /dev/null +++ b/jetty-websocket/websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseInOnOpenTest.java @@ -0,0 +1,95 @@ +// +// ======================================================================== +// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.websocket.tests; + +import java.net.URI; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.websocket.api.Session; +import org.eclipse.jetty.websocket.api.StatusCode; +import org.eclipse.jetty.websocket.api.WebSocketConnectionListener; +import org.eclipse.jetty.websocket.client.WebSocketClient; +import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer; +import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class CloseInOnOpenTest +{ + private Server server; + private ServerConnector connector; + private JettyWebSocketServerContainer serverContainer; + private WebSocketClient client; + + @BeforeEach + public void beforeEach() throws Exception + { + server = new Server(); + + connector = new ServerConnector(server); + server.addConnector(connector); + + ServletContextHandler context = new ServletContextHandler(); + context.setContextPath("/"); + server.setHandler(context); + + JettyWebSocketServletContainerInitializer.configure(context, (servletContext, wsContainer) -> + wsContainer.addMapping("/ws", (req, resp) -> new ClosingListener())); + server.start(); + + serverContainer = JettyWebSocketServerContainer.getContainer(context.getServletContext()); + assertNotNull(serverContainer); + + client = new WebSocketClient(); + client.start(); + } + + @AfterEach + public void afterEach() throws Exception + { + client.stop(); + server.stop(); + } + + @Test + public void testCloseInOnWebSocketConnect() throws Exception + { + URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/ws"); + EventSocket clientEndpoint = new EventSocket(); + + client.connect(clientEndpoint, uri).get(5, TimeUnit.SECONDS); + assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS)); + assertThat(clientEndpoint.closeCode, is(StatusCode.POLICY_VIOLATION)); + + assertThat(serverContainer.getOpenSessions().size(), is(0)); + } + + public static class ClosingListener implements WebSocketConnectionListener + { + @Override + public void onWebSocketConnect(Session session) + { + session.close(StatusCode.POLICY_VIOLATION, "I am a WS that closes immediately"); + } + } +}