From beccc5ee8020e2fa361923b055071e01f271f0d1 Mon Sep 17 00:00:00 2001 From: Pavel Baranchikov Date: Tue, 22 Mar 2016 16:33:38 +0300 Subject: [PATCH 1/4] Fix #437 Avoid NPE on receiving empty message though MessageHandler.Partial Signed-off-by: Pavel Baranchikov --- .../jsr356/messages/BinaryPartialMessage.java | 6 +- .../jsr356/MessageReceivingTest.java | 308 ++++++++++++++++++ 2 files changed, 312 insertions(+), 2 deletions(-) create mode 100644 jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java index 5f94de7966b..7acb6807514 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java @@ -51,14 +51,16 @@ public class BinaryPartialMessage implements MessageAppender // Supported Partial<> Type #1: ByteBuffer if (msgWrapper.isMessageType(ByteBuffer.class)) { - partialHandler.onMessage(payload.slice(),isLast); + partialHandler.onMessage(payload==null?ByteBuffer.allocate(0): + payload.slice(),isLast); return; } // Supported Partial<> Type #2: byte[] if (msgWrapper.isMessageType(byte[].class)) { - partialHandler.onMessage(BufferUtil.toArray(payload),isLast); + partialHandler.onMessage(payload==null?new byte[0]: + BufferUtil.toArray(payload),isLast); return; } diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java new file mode 100644 index 00000000000..92676d2c06a --- /dev/null +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java @@ -0,0 +1,308 @@ +// +// ======================================================================== +// Copyright (c) 1995-2016 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.jsr356; + +import static org.hamcrest.Matchers.instanceOf; + +import java.io.IOException; +import java.net.URI; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; + +import javax.websocket.ContainerProvider; +import javax.websocket.Endpoint; +import javax.websocket.EndpointConfig; +import javax.websocket.MessageHandler; +import javax.websocket.Session; +import javax.websocket.WebSocketContainer; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.handler.ContextHandler; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * This class tests receiving of messages by different types of {@link MessageHandler} + */ +public class MessageReceivingTest { + private static final Logger LOG = Log.getLogger(EndpointEchoTest.class); + private static Server server; + private static EchoHandler handler; + private static URI serverUri; + private WebSocketContainer container; + private final String VERY_LONG_STRING; + + public MessageReceivingTest() { + final StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 1024 * 1024; i++) { + sb.append(i & 1); + } + VERY_LONG_STRING = sb.toString(); + } + + @BeforeClass + public static void startServer() throws Exception { + server = new Server(); + ServerConnector connector = new ServerConnector(server); + server.addConnector(connector); + + handler = new EchoHandler(); + + ContextHandler context = new ContextHandler(); + context.setContextPath("/"); + context.setHandler(handler); + server.setHandler(context); + + // Start Server + server.start(); + + String host = connector.getHost(); + if (host == null) { + host = "localhost"; + } + int port = connector.getLocalPort(); + serverUri = new URI(String.format("ws://%s:%d/", host, port)); + } + + @AfterClass + public static void stopServer() { + try { + server.stop(); + } catch (Exception e) { + e.printStackTrace(System.err); + } + } + + @Before + public void configureTest() { + container = ContainerProvider.getWebSocketContainer(); + } + + /** + * Method tests receiving of text messages at once. + * + * @throws Exception on exception occur + */ + @Test + public void testWholeTextMessage() throws Exception { + final TestEndpoint echoer = new TestEndpoint(new WholeStringCaptureHandler()); + Assert.assertThat(echoer, instanceOf(javax.websocket.Endpoint.class)); + // Issue connect using instance of class that extends Endpoint + final Session session = container.connectToServer(echoer, serverUri); + if (LOG.isDebugEnabled()) + LOG.debug("Client Connected: {}", session); + session.getBasicRemote().sendText(""); + session.getBasicRemote().sendText("Echo"); + session.getBasicRemote().sendText(VERY_LONG_STRING); + session.getBasicRemote().sendText("Echo"); + if (LOG.isDebugEnabled()) + LOG.debug("Client Message Sent"); + echoer.handler.getMessageQueue().awaitMessages(2, 1000, TimeUnit.MILLISECONDS); + } + + /** + * Method tests receiving of text messages by parts. + * + * @throws Exception on exception occur + */ + @Test + public void testPartialTextMessage() throws Exception { + final TestEndpoint echoer = new TestEndpoint(new PartialStringCaptureHandler()); + Assert.assertThat(echoer, instanceOf(javax.websocket.Endpoint.class)); + // Issue connect using instance of class that extends Endpoint + final Session session = container.connectToServer(echoer, serverUri); + if (LOG.isDebugEnabled()) + LOG.debug("Client Connected: {}", session); + session.getBasicRemote().sendText(""); + session.getBasicRemote().sendText("Echo"); + if (LOG.isDebugEnabled()) + LOG.debug("Client Message Sent"); + echoer.handler.getMessageQueue().awaitMessages(2, 1000, TimeUnit.MILLISECONDS); + } + + /** + * Method tests receiving of binary messages at once. + * + * @throws Exception on exception occur + */ + @Test + public void testWholeBinaryMessage() throws Exception { + final TestEndpoint echoer = new TestEndpoint(new WholeByteBufferCaptureHandler()); + Assert.assertThat(echoer, instanceOf(javax.websocket.Endpoint.class)); + // Issue connect using instance of class that extends Endpoint + final Session session = container.connectToServer(echoer, serverUri); + if (LOG.isDebugEnabled()) + LOG.debug("Client Connected: {}", session); + sendBinary(session, ""); + sendBinary(session, "Echo"); + if (LOG.isDebugEnabled()) + LOG.debug("Client Message Sent"); + echoer.handler.getMessageQueue().awaitMessages(2, 1000, TimeUnit.MILLISECONDS); + } + + /** + * Method tests receiving of binary messages by parts. + * + * @throws Exception on exception occur + */ + @Test + public void testPartialBinaryMessage() throws Exception { + final TestEndpoint echoer = new TestEndpoint(new PartialByteBufferCaptureHandler()); + Assert.assertThat(echoer, instanceOf(javax.websocket.Endpoint.class)); + // Issue connect using instance of class that extends Endpoint + final Session session = container.connectToServer(echoer, serverUri); + if (LOG.isDebugEnabled()) + LOG.debug("Client Connected: {}", session); + sendBinary(session, ""); + sendBinary(session, "Echo"); + if (LOG.isDebugEnabled()) + LOG.debug("Client Message Sent"); + echoer.handler.getMessageQueue().awaitMessages(2, 1000, TimeUnit.MILLISECONDS); + } + + private static void sendBinary(Session session, String message) throws IOException { + final ByteBuffer bb = ByteBuffer.wrap(message.getBytes()); + session.getBasicRemote().sendBinary(bb); + } + + private static class TestEndpoint extends Endpoint { + public final AbstractHandler handler; + + public TestEndpoint(AbstractHandler handler) { + this.handler = handler; + } + + @Override + public void onOpen(Session session, EndpointConfig config) { + session.addMessageHandler(handler); + } + + } + + /** + * Abstract message handler implementation, used for tests. + */ + private static abstract class AbstractHandler implements MessageHandler { + /** + * Message queue to put the result messages. + */ + private final MessageQueue messageQueue = new MessageQueue(); + + /** + * Returns message queue to test received messages count. + * + * @return message queue object + */ + public MessageQueue getMessageQueue() { + return messageQueue; + } + } + + /** + * Partial message handler for receiving binary messages. + */ + public static class PartialByteBufferCaptureHandler extends AbstractHandler implements + MessageHandler.Partial { + + /** + * Parts of the current message. This list is appended with every non-last part and is + * cleared after last part of a message has been received. + */ + private final List currentMessage = new ArrayList<>(); + + @Override + public void onMessage(ByteBuffer messagePart, boolean last) { + final ByteBuffer bufferCopy = ByteBuffer.allocate(messagePart.capacity()); + bufferCopy.put(messagePart); + currentMessage.add(bufferCopy); + if (last) { + int totalSize = 0; + for (ByteBuffer bb : currentMessage) { + totalSize += bb.capacity(); + } + final ByteBuffer result = ByteBuffer.allocate(totalSize); + for (ByteBuffer bb : currentMessage) { + result.put(bb); + } + final String stringResult = new String(result.array()); + getMessageQueue().add(stringResult); + currentMessage.clear(); + } + + } + + } + /** + * Whole message handler for receiving binary messages. + */ + public class WholeByteBufferCaptureHandler extends AbstractHandler implements + MessageHandler.Whole { + + @Override + public void onMessage(ByteBuffer message) { + final String stringResult = new String(message.array()); + getMessageQueue().add(stringResult); + + } + } + + /** + * Partial message handler for receiving text messages. + */ + public static class PartialStringCaptureHandler extends AbstractHandler implements + MessageHandler.Partial { + + /** + * Parts of the current message. This list is appended with every non-last part and is + * cleared after last part of a message has been received. + */ + private StringBuilder sb = new StringBuilder(); + + @Override + public void onMessage(String messagePart, boolean last) { + sb.append(messagePart); + if (last) { + getMessageQueue().add(sb.toString()); + sb = new StringBuilder(); + } + } + + } + /** + * Whole message handler for receiving text messages. + */ + public class WholeStringCaptureHandler extends AbstractHandler implements + MessageHandler.Whole { + + @Override + public void onMessage(String message) { + getMessageQueue().add(message); + + } + } + +} From f2216ad108319fd45037c50e383cb6dc229675ba Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 22 Mar 2016 12:51:04 -0700 Subject: [PATCH 2/4] Issue #437 - Avoid NPE on receiving empty message though MessageHandler.Partial Minor updates to use features elsewhere in Jetty. --- .../jsr356/messages/BinaryPartialMessage.java | 2 +- .../websocket/jsr356/MessageReceivingTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java index 7acb6807514..2dd077e5574 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java +++ b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/messages/BinaryPartialMessage.java @@ -51,7 +51,7 @@ public class BinaryPartialMessage implements MessageAppender // Supported Partial<> Type #1: ByteBuffer if (msgWrapper.isMessageType(ByteBuffer.class)) { - partialHandler.onMessage(payload==null?ByteBuffer.allocate(0): + partialHandler.onMessage(payload==null?BufferUtil.EMPTY_BUFFER: payload.slice(),isLast); return; } diff --git a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java index 92676d2c06a..485ef0a4bf5 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java +++ b/jetty-websocket/javax-websocket-client-impl/src/test/java/org/eclipse/jetty/websocket/jsr356/MessageReceivingTest.java @@ -18,12 +18,12 @@ package org.eclipse.jetty.websocket.jsr356; -import static org.hamcrest.Matchers.instanceOf; - import java.io.IOException; import java.net.URI; import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeUnit; @@ -45,6 +45,8 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import static org.hamcrest.Matchers.instanceOf; + /** * This class tests receiving of messages by different types of {@link MessageHandler} */ @@ -57,11 +59,9 @@ public class MessageReceivingTest { private final String VERY_LONG_STRING; public MessageReceivingTest() { - final StringBuilder sb = new StringBuilder(); - for (int i = 0; i < 1024 * 1024; i++) { - sb.append(i & 1); - } - VERY_LONG_STRING = sb.toString(); + byte raw[] = new byte[1024 * 1024]; + Arrays.fill(raw, (byte)'x'); + VERY_LONG_STRING = new String(raw, StandardCharsets.UTF_8); } @BeforeClass From 1c07172635de91499f3853e0046e12dd82a000b1 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 24 Mar 2016 10:03:39 +1100 Subject: [PATCH 3/4] Issue #453 Change logging of setting session maxInactiveInterval to DEBUG from WARN --- .../jetty/server/session/AbstractSession.java | 11 +++++++---- .../jetty/server/session/AbstractSessionManager.java | 12 ++++++++---- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java index d4e6ade5492..8ceb814c023 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java @@ -557,10 +557,13 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI @Override public void setMaxInactiveInterval(int secs) { - if (secs <= 0) - LOG.warn("Session {} is now immortal (maxInactiveInterval={})", _clusterId, secs); - else if (LOG.isDebugEnabled()) - LOG.debug("Session {} maxInactiveInterval={}", _clusterId, secs); + if (LOG.isDebugEnabled()) + { + if (secs <= 0) + LOG.debug("Session {} is now immortal (maxInactiveInterval={})", _clusterId, secs); + else + LOG.debug("Session {} maxInactiveInterval={}", _clusterId, secs); + } _maxIdleMs=(long)secs*1000L; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java index e2194a83299..7a58d09b240 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java @@ -627,10 +627,14 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen public void setMaxInactiveInterval(int seconds) { _dftMaxIdleSecs=seconds; - if (_dftMaxIdleSecs <= 0) - __log.warn("Sessions created by this manager are immortal (default maxInactiveInterval={})"+_dftMaxIdleSecs); - else if (__log.isDebugEnabled()) - __log.debug("SessionManager default maxInactiveInterval={}", _dftMaxIdleSecs); + if (__log.isDebugEnabled()) + { + if (_dftMaxIdleSecs <= 0) + __log.debug("Sessions created by this manager are immortal (default maxInactiveInterval={})"+_dftMaxIdleSecs); + else + __log.debug("SessionManager default maxInactiveInterval={}", _dftMaxIdleSecs); + } + } /* ------------------------------------------------------------ */ From ede4c3a711868ccda92c46fb85db6c9d37feafab Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 24 Mar 2016 10:55:52 +1100 Subject: [PATCH 4/4] Issue #435 adjust debug log message --- .../eclipse/jetty/server/session/AbstractSessionManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java index 7a58d09b240..dd7a2ed4223 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionManager.java @@ -630,7 +630,7 @@ public abstract class AbstractSessionManager extends ContainerLifeCycle implemen if (__log.isDebugEnabled()) { if (_dftMaxIdleSecs <= 0) - __log.debug("Sessions created by this manager are immortal (default maxInactiveInterval={})"+_dftMaxIdleSecs); + __log.debug("Sessions created by this manager are immortal (default maxInactiveInterval={})",_dftMaxIdleSecs); else __log.debug("SessionManager default maxInactiveInterval={}", _dftMaxIdleSecs); }