Issue #8170 - fix WebSocket close over HTTP/2

Signed-off-by: Lachlan Roberts <[email protected]>
jetty · Jun 17, 2022 · e4b0db8 · e4b0db8
1 parent 0699bc5
commit e4b0db8
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 2 deletions.
diff --git a/.../http2-server/src/main/java/org/eclipse/jetty/http2/server/ServerHTTP2StreamEndPoint.java b/.../http2-server/src/main/java/org/eclipse/jetty/http2/server/ServerHTTP2StreamEndPoint.java
@@ -53,11 +53,11 @@ public boolean onTimeout(Throwable failure, Consumer<Runnable> consumer)
     {
         if (LOG.isDebugEnabled())
             LOG.debug("idle timeout on {}: {}", this, failure);
-        offerFailure(failure);
         boolean result = true;
         Connection connection = getConnection();
         if (connection != null)
             result = connection.onIdleExpired();
+        offerFailure(failure);
         consumer.accept(() -> close(failure));
         return result;
     }

diff --git a/...t-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java b/...t-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketOverHTTP2Test.java
@@ -18,6 +18,7 @@
 import java.net.ConnectException;
 import java.net.URI;
 import java.nio.channels.ClosedChannelException;
+import java.time.Duration;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -57,6 +58,7 @@
 import org.eclipse.jetty.websocket.api.exceptions.UpgradeException;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
 import org.eclipse.jetty.websocket.core.server.internal.UpgradeHttpServletRequest;
+import org.eclipse.jetty.websocket.server.JettyWebSocketServerContainer;
 import org.eclipse.jetty.websocket.server.JettyWebSocketServlet;
 import org.eclipse.jetty.websocket.server.JettyWebSocketServletFactory;
 import org.eclipse.jetty.websocket.server.config.JettyWebSocketServletContainerInitializer;
@@ -68,6 +70,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsStringIgnoringCase;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -80,6 +83,7 @@ public class WebSocketOverHTTP2Test
     private ServerConnector connector;
     private ServerConnector tlsConnector;
     private WebSocketClient wsClient;
+    private ServletContextHandler context;
 
     private void startServer() throws Exception
     {
@@ -112,7 +116,7 @@ private void startServer(TestJettyWebSocketServlet servlet) throws Exception
         tlsConnector = new ServerConnector(server, 1, 1, ssl, alpn, h1s, h2s);
         server.addConnector(tlsConnector);
 
-        ServletContextHandler context = new ServletContextHandler(server, "/");
+        context = new ServletContextHandler(server, "/");
         context.addServlet(new ServletHolder(servlet), "/ws/*");
         JettyWebSocketServletContainerInitializer.configure(context, null);
 
@@ -337,6 +341,41 @@ public void testServerConnectionClose() throws Exception
         assertThat(cause, instanceOf(ClosedChannelException.class));
     }
 
+    @Test
+    public void testServerTimeout() throws Exception
+    {
+        startServer();
+        JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(context.getServletContext());
+        startClient(clientConnector -> new ClientConnectionFactoryOverHTTP2.HTTP2(new HTTP2Client(clientConnector)));
+        EchoSocket serverEndpoint = new EchoSocket();
+        container.addMapping("/specialEcho", (req, resp) -> serverEndpoint);
+
+        // Set up idle timeouts.
+        long timeout = 1000;
+        container.setIdleTimeout(Duration.ofMillis(timeout));
+        wsClient.setIdleTimeout(Duration.ZERO);
+
+        // Setup a websocket connection.
+        EventSocket clientEndpoint = new EventSocket();
+        URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/specialEcho");
+        Session session = wsClient.connect(clientEndpoint, uri).get(5, TimeUnit.SECONDS);
+        session.getRemote().sendString("hello world");
+        String received = clientEndpoint.textMessages.poll(5, TimeUnit.SECONDS);
+        assertThat(received, equalTo("hello world"));
+
+        // Wait for timeout on server.
+        assertTrue(serverEndpoint.closeLatch.await(timeout * 2, TimeUnit.MILLISECONDS));
+        assertThat(serverEndpoint.closeCode, equalTo(StatusCode.SHUTDOWN));
+        assertThat(serverEndpoint.closeReason, containsStringIgnoringCase("timeout"));
+        assertNotNull(serverEndpoint.error);
+
+        // Wait for timeout on client.
+        assertTrue(clientEndpoint.closeLatch.await(timeout * 2, TimeUnit.MILLISECONDS));
+        assertThat(clientEndpoint.closeCode, equalTo(StatusCode.SHUTDOWN));
+        assertThat(clientEndpoint.closeReason, containsStringIgnoringCase("timeout"));
+        assertNull(clientEndpoint.error);
+    }
+
     private static class TestJettyWebSocketServlet extends JettyWebSocketServlet
     {
         @Override