Issue #6602 - do not invoke SessionListener onOpen if session has bee…

…n closed in OnOpen Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · Aug 12, 2021 · 646707b · 646707b
1 parent 10ef07b
commit 646707b
Show file tree

Hide file tree

Showing 5 changed files with 203 additions and 3 deletions.
diff --git a/...ket/websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/CoreSession.java b/...ket/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/...on/src/main/java/org/eclipse/jetty/websocket/javax/common/JavaxWebSocketFrameHandler.java b/...on/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/...-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/CloseInOnOpenTest.java b/...-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/...y-common/src/main/java/org/eclipse/jetty/websocket/common/JettyWebSocketFrameHandler.java b/...y-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/...socket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseInOnOpenTest.java b/...socket-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");
+        }
+    }
+}