Merge pull request #3885 from eclipse/jetty-9.4.x-3884-websocket-frame-listener

Issue #3884 - Pure WebSocketFrameListener should not process Continuation events

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/CloseTrackingEndpoint.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.websocket.tests;
 
 import java.lang.reflect.Field;
+import java.nio.ByteBuffer;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
@@ -51,6 +52,7 @@ public class CloseTrackingEndpoint extends WebSocketAdapter
     public CountDownLatch errorLatch = new CountDownLatch(1);
 
     public LinkedBlockingQueue<String> messageQueue = new LinkedBlockingQueue<>();
+    public LinkedBlockingQueue<ByteBuffer> binaryMessageQueue = new LinkedBlockingQueue<>();
     public AtomicReference<Throwable> error = new AtomicReference<>();
 
     public void assertReceivedCloseEvent(int clientTimeoutMs, Matcher<Integer> statusCodeMatcher)
@@ -110,6 +112,13 @@ public class CloseTrackingEndpoint extends WebSocketAdapter
         messageQueue.offer(message);
     }
 
+    @Override
+    public void onWebSocketBinary(byte[] payload, int offset, int len)
+    {
+        LOG.debug("onWebSocketBinary({},offset,len)", payload, offset, len);
+        binaryMessageQueue.offer(ByteBuffer.wrap(payload, offset, len));
+    }
+
     public EndPoint getEndPoint() throws Exception
     {
         Session session = getSession();

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/EchoSocket.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.websocket.tests;
 
 import java.io.IOException;
+import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
@@ -33,4 +34,10 @@ public class EchoSocket
     {
         session.getRemote().sendString(msg);
     }
+
+    @OnWebSocketMessage
+    public void onBinaryMessage(Session session, byte[] data, int offset, int len) throws IOException
+    {
+        session.getRemote().sendBytes(ByteBuffer.wrap(data, offset, len));
+    }
 }

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/client/WebSocketClientTest.java

@@ -21,6 +21,7 @@ package org.eclipse.jetty.websocket.tests.client;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.URI;
+import java.nio.ByteBuffer;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.EnumSet;
@@ -33,6 +34,7 @@ import javax.servlet.DispatcherType;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.websocket.api.RemoteEndpoint;
 import org.eclipse.jetty.websocket.api.Session;
@@ -171,6 +173,86 @@ public class WebSocketClientTest
         }
     }
 
+    @Test
+    public void testBasicEcho_PartialUsage_FromClient() throws Exception
+    {
+        CloseTrackingEndpoint cliSock = new CloseTrackingEndpoint();
+
+        client.getPolicy().setIdleTimeout(10000);
+
+        URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/echo"));
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        request.setSubProtocols("echo");
+        Future<Session> future = client.connect(cliSock, wsUri, request);
+
+        try (Session sess = future.get(30, TimeUnit.SECONDS))
+        {
+            assertThat("Session", sess, notNullValue());
+            assertThat("Session.open", sess.isOpen(), is(true));
+            assertThat("Session.upgradeRequest", sess.getUpgradeRequest(), notNullValue());
+            assertThat("Session.upgradeResponse", sess.getUpgradeResponse(), notNullValue());
+
+            Collection<WebSocketSession> sessions = client.getOpenSessions();
+            assertThat("client.sessions.size", sessions.size(), is(1));
+
+            RemoteEndpoint remote = cliSock.getSession().getRemote();
+            remote.sendPartialString("Hello", false);
+            remote.sendPartialString(" ", false);
+            remote.sendPartialString("World", true);
+
+            // wait for response from server
+            String received = cliSock.messageQueue.poll(5, TimeUnit.SECONDS);
+            assertThat("Message", received, containsString("Hello World"));
+        }
+    }
+
+    @Test
+    public void testBasicEcho_PartialText_WithPartialBinary_FromClient() throws Exception
+    {
+        CloseTrackingEndpoint cliSock = new CloseTrackingEndpoint();
+
+        client.getPolicy().setIdleTimeout(10000);
+
+        URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/echo"));
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        request.setSubProtocols("echo");
+        Future<Session> future = client.connect(cliSock, wsUri, request);
+
+        try (Session sess = future.get(30, TimeUnit.SECONDS))
+        {
+            assertThat("Session", sess, notNullValue());
+            assertThat("Session.open", sess.isOpen(), is(true));
+            assertThat("Session.upgradeRequest", sess.getUpgradeRequest(), notNullValue());
+            assertThat("Session.upgradeResponse", sess.getUpgradeResponse(), notNullValue());
+
+            Collection<WebSocketSession> sessions = client.getOpenSessions();
+            assertThat("client.sessions.size", sessions.size(), is(1));
+
+            RemoteEndpoint remote = cliSock.getSession().getRemote();
+            remote.sendPartialString("Hello", false);
+            remote.sendPartialString(" ", false);
+            remote.sendPartialString("World", true);
+
+            String[] parts = {
+                "The difference between the right word ",
+                "and the almost right word is the difference ",
+                "between lightning and a lightning bug."
+            };
+
+            remote.sendPartialBytes(BufferUtil.toBuffer(parts[0]), false);
+            remote.sendPartialBytes(BufferUtil.toBuffer(parts[1]), false);
+            remote.sendPartialBytes(BufferUtil.toBuffer(parts[2]), true);
+
+            // wait for response from server
+            String received = cliSock.messageQueue.poll(5, TimeUnit.SECONDS);
+            assertThat("Message", received, containsString("Hello World"));
+
+            ByteBuffer bufReceived = cliSock.binaryMessageQueue.poll(5, TimeUnit.SECONDS);
+            received = BufferUtil.toUTF8String(bufReceived.slice());
+            assertThat("Message", received, containsString(parts[0] + parts[1] + parts[2]));
+        }
+    }
+
     @Test
     public void testBasicEcho_UsingCallback() throws Exception
     {

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/FrameAnnotationTest.java

@@ -0,0 +1,207 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 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.server;
+
+import java.net.URI;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.handler.DefaultHandler;
+import org.eclipse.jetty.server.handler.HandlerList;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.log.StacklessLogging;
+import org.eclipse.jetty.websocket.api.RemoteEndpoint;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketFrame;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.api.util.WSURI;
+import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.eclipse.jetty.websocket.common.OpCode;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
+import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
+import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
+import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
+import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+import org.eclipse.jetty.websocket.tests.CloseTrackingEndpoint;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public class FrameAnnotationTest
+{
+    private Server server;
+    private FrameCreator frameCreator;
+    private WebSocketClient client;
+
+    @BeforeEach
+    public void startServer() throws Exception
+    {
+        server = new Server();
+
+        ServerConnector connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        ServletContextHandler context = new ServletContextHandler();
+        context.setContextPath("/");
+
+        ServletHolder closeEndpoint = new ServletHolder(new WebSocketServlet()
+        {
+            @Override
+            public void configure(WebSocketServletFactory factory)
+            {
+                factory.getPolicy().setIdleTimeout(SECONDS.toMillis(2));
+                frameCreator = new FrameCreator();
+                factory.setCreator(frameCreator);
+            }
+        });
+        context.addServlet(closeEndpoint, "/ws");
+
+        HandlerList handlers = new HandlerList();
+        handlers.addHandler(context);
+        handlers.addHandler(new DefaultHandler());
+
+        server.setHandler(handlers);
+
+        server.start();
+    }
+
+    @AfterEach
+    public void stopServer() throws Exception
+    {
+        server.stop();
+    }
+
+    @BeforeEach
+    public void startClient() throws Exception
+    {
+        client = new WebSocketClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void stopClient() throws Exception
+    {
+        client.stop();
+    }
+
+    private void close(Session session)
+    {
+        if (session != null)
+        {
+            session.close();
+        }
+    }
+
+    @Test
+    public void testPartialText() throws Exception
+    {
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        CloseTrackingEndpoint clientEndpoint = new CloseTrackingEndpoint();
+
+        URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/ws"));
+        Future<Session> futSession = client.connect(clientEndpoint, wsUri, request);
+
+        Session session = null;
+        try (StacklessLogging ignore = new StacklessLogging(WebSocketSession.class))
+        {
+            session = futSession.get(5, SECONDS);
+
+            RemoteEndpoint clientRemote = session.getRemote();
+            clientRemote.sendPartialString("hello", false);
+            clientRemote.sendPartialString(" ", false);
+            clientRemote.sendPartialString("world", true);
+
+            FrameEndpoint serverEndpoint = frameCreator.frameEndpoint;
+
+            String event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[TEXT,fin=false,payload=hello,len=5]"));
+            event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[CONTINUATION,fin=false,payload= ,len=1]"));
+            event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[CONTINUATION,fin=true,payload=world,len=5]"));
+        }
+        finally
+        {
+            close(session);
+        }
+    }
+
+    public static class FrameCreator implements WebSocketCreator
+    {
+        public FrameEndpoint frameEndpoint;
+
+        @Override
+        public Object createWebSocket(ServletUpgradeRequest req, ServletUpgradeResponse resp)
+        {
+            frameEndpoint = new FrameEndpoint();
+            return frameEndpoint;
+        }
+    }
+
+    @WebSocket
+    public static class FrameEndpoint
+    {
+        public Session session;
+        public CountDownLatch closeLatch = new CountDownLatch(1);
+        public LinkedBlockingQueue<String> frameEvents = new LinkedBlockingQueue<>();
+
+        @OnWebSocketClose
+        public void onWebSocketClose(int statusCode, String reason)
+        {
+            closeLatch.countDown();
+        }
+
+        @OnWebSocketConnect
+        public void onWebSocketConnect(Session session)
+        {
+            this.session = session;
+        }
+
+        @OnWebSocketError
+        public void onWebSocketError(Throwable cause)
+        {
+            cause.printStackTrace(System.err);
+        }
+
+        @OnWebSocketFrame
+        public void onWebSocketFrame(Frame frame)
+        {
+            frameEvents.offer(String.format("FRAME[%s,fin=%b,payload=%s,len=%d]",
+                OpCode.name(frame.getOpCode()),
+                frame.isFin(),
+                BufferUtil.toUTF8String(frame.getPayload()),
+                frame.getPayloadLength()));
+        }
+    }
+}

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/server/FrameListenerTest.java

@@ -0,0 +1,185 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 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.server;
+
+import java.net.URI;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.Future;
+import java.util.concurrent.LinkedBlockingQueue;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.server.handler.DefaultHandler;
+import org.eclipse.jetty.server.handler.HandlerList;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.util.log.StacklessLogging;
+import org.eclipse.jetty.websocket.api.RemoteEndpoint;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.WebSocketFrameListener;
+import org.eclipse.jetty.websocket.api.extensions.Frame;
+import org.eclipse.jetty.websocket.api.util.WSURI;
+import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.eclipse.jetty.websocket.common.OpCode;
+import org.eclipse.jetty.websocket.common.WebSocketSession;
+import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+import org.eclipse.jetty.websocket.tests.CloseTrackingEndpoint;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public class FrameListenerTest
+{
+    private Server server;
+    private FrameEndpoint serverEndpoint;
+    private WebSocketClient client;
+
+    @BeforeEach
+    public void startServer() throws Exception
+    {
+        server = new Server();
+
+        ServerConnector connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        ServletContextHandler context = new ServletContextHandler();
+        context.setContextPath("/");
+
+        ServletHolder closeEndpoint = new ServletHolder(new WebSocketServlet()
+        {
+            @Override
+            public void configure(WebSocketServletFactory factory)
+            {
+                factory.getPolicy().setIdleTimeout(SECONDS.toMillis(2));
+                serverEndpoint = new FrameEndpoint();
+                factory.setCreator((req, resp) -> serverEndpoint);
+            }
+        });
+        context.addServlet(closeEndpoint, "/ws");
+
+        HandlerList handlers = new HandlerList();
+        handlers.addHandler(context);
+        handlers.addHandler(new DefaultHandler());
+
+        server.setHandler(handlers);
+
+        server.start();
+    }
+
+    @AfterEach
+    public void stopServer() throws Exception
+    {
+        server.stop();
+    }
+
+    @BeforeEach
+    public void startClient() throws Exception
+    {
+        client = new WebSocketClient();
+        client.start();
+    }
+
+    @AfterEach
+    public void stopClient() throws Exception
+    {
+        client.stop();
+    }
+
+    private void close(Session session)
+    {
+        if (session != null)
+        {
+            session.close();
+        }
+    }
+
+    @Test
+    public void testPartialText() throws Exception
+    {
+        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        CloseTrackingEndpoint clientEndpoint = new CloseTrackingEndpoint();
+
+        URI wsUri = WSURI.toWebsocket(server.getURI().resolve("/ws"));
+        Future<Session> futSession = client.connect(clientEndpoint, wsUri, request);
+
+        Session session = null;
+        try (StacklessLogging ignore = new StacklessLogging(WebSocketSession.class))
+        {
+            session = futSession.get(5, SECONDS);
+
+            RemoteEndpoint clientRemote = session.getRemote();
+            clientRemote.sendPartialString("hello", false);
+            clientRemote.sendPartialString(" ", false);
+            clientRemote.sendPartialString("world", true);
+
+            String event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[TEXT,fin=false,payload=hello,len=5]"));
+            event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[CONTINUATION,fin=false,payload= ,len=1]"));
+            event = serverEndpoint.frameEvents.poll(5, SECONDS);
+            assertThat("Event", event, is("FRAME[CONTINUATION,fin=true,payload=world,len=5]"));
+        }
+        finally
+        {
+            close(session);
+        }
+    }
+
+    public static class FrameEndpoint implements WebSocketFrameListener
+    {
+        public Session session;
+        public CountDownLatch closeLatch = new CountDownLatch(1);
+        public LinkedBlockingQueue<String> frameEvents = new LinkedBlockingQueue<>();
+
+        @Override
+        public void onWebSocketClose(int statusCode, String reason)
+        {
+            closeLatch.countDown();
+        }
+
+        @Override
+        public void onWebSocketConnect(Session session)
+        {
+            this.session = session;
+        }
+
+        @Override
+        public void onWebSocketError(Throwable cause)
+        {
+            cause.printStackTrace(System.err);
+        }
+
+        @Override
+        public void onWebSocketFrame(Frame frame)
+        {
+            frameEvents.offer(String.format("FRAME[%s,fin=%b,payload=%s,len=%d]",
+                OpCode.name(frame.getOpCode()),
+                frame.isFin(),
+                BufferUtil.toUTF8String(frame.getPayload()),
+                frame.getPayloadLength()));
+        }
+    }
+}

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyAnnotatedEventDriver.java

@@ -33,6 +33,7 @@ import org.eclipse.jetty.websocket.common.CloseInfo;
 import org.eclipse.jetty.websocket.common.message.MessageAppender;
 import org.eclipse.jetty.websocket.common.message.MessageInputStream;
 import org.eclipse.jetty.websocket.common.message.MessageReader;
+import org.eclipse.jetty.websocket.common.message.NullMessage;
 import org.eclipse.jetty.websocket.common.message.SimpleBinaryMessage;
 import org.eclipse.jetty.websocket.common.message.SimpleTextMessage;
 
@@ -84,6 +85,11 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver
         if (events.onBinary == null)
         {
             // not interested in binary events
+            if (activeMessage == null)
+            {
+                activeMessage = NullMessage.INSTANCE;
+            }
+
             return;
         }
 
@@ -198,6 +204,10 @@ public class JettyAnnotatedEventDriver extends AbstractEventDriver
         if (events.onText == null)
         {
             // not interested in text events
+            if (activeMessage == null)
+            {
+                activeMessage = NullMessage.INSTANCE;
+            }
             return;
         }
 

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/JettyListenerEventDriver.java

@@ -250,7 +250,10 @@ public class JettyListenerEventDriver extends AbstractEventDriver
             return;
         }
 
-        super.onContinuationFrame(buffer, fin);
+        if (listener instanceof WebSocketListener)
+        {
+            super.onContinuationFrame(buffer, fin);
+        }
     }
 
     @Override

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/message/NullMessage.java

@@ -0,0 +1,38 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 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.common.message;
+
+import java.nio.ByteBuffer;
+
+public class NullMessage implements MessageAppender
+{
+    public static final MessageAppender INSTANCE = new NullMessage();
+
+    @Override
+    public void appendFrame(ByteBuffer framePayload, boolean isLast)
+    {
+        // consume payload
+        framePayload.position(framePayload.limit());
+    }
+
+    @Override
+    public void messageComplete()
+    {
+    }
+}