From 4daba061757acd43fa00374cc200502f284d46b5 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 11 Feb 2013 16:50:01 -0700 Subject: [PATCH] 400255 - Using WebSocket.maxMessageSize results in IllegalArgumentException + Adding testcase that replicated failure reported in issue + Fixing AnnotatedEventDriver error with input size + Fixing WebSocketSession detection of isOpen() + Fixing BigEchoSocket test for session.isOpen() --- .../websocket/common/WebSocketSession.java | 23 ++-- .../common/events/AnnotatedEventDriver.java | 2 +- .../server/AnnotatedMaxMessageSizeTest.java | 114 ++++++++++++++++++ .../server/examples/echo/BigEchoSocket.java | 10 +- 4 files changed, 133 insertions(+), 16 deletions(-) create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/AnnotatedMaxMessageSizeTest.java diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java index 07fb134239b..c5296499873 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketSession.java @@ -57,7 +57,6 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc private final EventDriver websocket; private final LogicalConnection connection; private ExtensionFactory extensionFactory; - private boolean active = false; private long maximumMessageSize; private String protocolVersion; private long timeout; @@ -269,7 +268,11 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc @Override public boolean isOpen() { - return active; + if (this.connection == null) + { + return false; + } + return this.connection.isOpen(); } @Override @@ -292,31 +295,24 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc */ public void open() { - if (isOpen()) + if (remote != null) { - throw new WebSocketException("Cannot Open WebSocketSession, Already open"); + // already opened + return; } // Connect remote remote = new WebSocketRemoteEndpoint(connection,outgoingHandler); - // Activate Session - this.active = true; - // Open WebSocket websocket.openSession(this); if (LOG.isDebugEnabled()) { - LOG.debug("{}",dump()); + LOG.debug("open -> {}",dump()); } } - public void setActive(boolean active) - { - this.active = active; - } - public void setExtensionFactory(ExtensionFactory extensionFactory) { this.extensionFactory = extensionFactory; @@ -370,6 +366,7 @@ public class WebSocketSession extends ContainerLifeCycle implements Session, Inc StringBuilder builder = new StringBuilder(); builder.append("WebSocketSession["); builder.append("websocket=").append(websocket); + builder.append(",behavior=").append(policy.getBehavior()); builder.append(",connection=").append(connection); builder.append(",remote=").append(remote); builder.append(",incoming=").append(incomingHandler); diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/AnnotatedEventDriver.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/AnnotatedEventDriver.java index 18d018b6123..083904f6184 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/AnnotatedEventDriver.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/AnnotatedEventDriver.java @@ -53,7 +53,7 @@ public class AnnotatedEventDriver extends EventDriver { this.policy.setMaxMessageSize(anno.maxMessageSize()); } - if (anno.maxMessageSize() > 0) + if (anno.inputBufferSize() > 0) { this.policy.setInputBufferSize(anno.inputBufferSize()); } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/AnnotatedMaxMessageSizeTest.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/AnnotatedMaxMessageSizeTest.java new file mode 100644 index 00000000000..09d06a5a558 --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/AnnotatedMaxMessageSizeTest.java @@ -0,0 +1,114 @@ +// +// ======================================================================== +// Copyright (c) 1995-2013 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.server; + +import static org.hamcrest.Matchers.*; + +import java.io.IOException; +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.websocket.api.UpgradeRequest; +import org.eclipse.jetty.websocket.api.UpgradeResponse; +import org.eclipse.jetty.websocket.common.WebSocketFrame; +import org.eclipse.jetty.websocket.server.blockhead.BlockheadClient; +import org.eclipse.jetty.websocket.server.examples.echo.BigEchoSocket; +import org.eclipse.jetty.websocket.server.helper.IncomingFramesCapture; +import org.eclipse.jetty.websocket.servlet.WebSocketCreator; +import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +public class AnnotatedMaxMessageSizeTest +{ + private static Server server; + private static ServerConnector connector; + private static URI serverUri; + + @BeforeClass + public static void startServer() throws Exception + { + server = new Server(); + connector = new ServerConnector(server); + server.addConnector(connector); + + WebSocketHandler wsHandler = new WebSocketHandler() + { + @Override + public void configure(WebSocketServletFactory factory) + { + factory.setCreator(new WebSocketCreator() + { + @Override + public Object createWebSocket(UpgradeRequest req, UpgradeResponse resp) + { + return new BigEchoSocket(); + } + }); + } + }; + + server.setHandler(wsHandler); + 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() throws Exception + { + server.stop(); + } + + @Test + public void testEcho() throws IOException, Exception + { + BlockheadClient client = new BlockheadClient(serverUri); + try + { + client.setProtocols("echo"); + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + // Generate text frame + String msg = "this is an echo ... cho ... ho ... o"; + client.write(WebSocketFrame.text(msg)); + + // Read frame (hopefully text frame) + IncomingFramesCapture capture = client.readFrames(1,TimeUnit.MILLISECONDS,500); + WebSocketFrame tf = capture.getFrames().get(0); + Assert.assertThat("Text Frame.status code",tf.getPayloadAsUTF8(),is(msg)); + } + finally + { + client.close(); + } + } +} diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/examples/echo/BigEchoSocket.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/examples/echo/BigEchoSocket.java index 7e009543257..da5aa8b4e41 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/examples/echo/BigEchoSocket.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/examples/echo/BigEchoSocket.java @@ -20,6 +20,8 @@ package org.eclipse.jetty.websocket.server.examples.echo; import java.nio.ByteBuffer; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage; import org.eclipse.jetty.websocket.api.annotations.WebSocket; @@ -30,11 +32,14 @@ import org.eclipse.jetty.websocket.api.annotations.WebSocket; @WebSocket(maxMessageSize = 64 * 1024) public class BigEchoSocket { + private static final Logger LOG = Log.getLogger(BigEchoSocket.class); + @OnWebSocketMessage public void onBinary(Session session, byte buf[], int offset, int length) { - if (session.isOpen()) + if (!session.isOpen()) { + LOG.warn("Session is closed"); return; } session.getRemote().sendBytesByFuture(ByteBuffer.wrap(buf,offset,length)); @@ -43,8 +48,9 @@ public class BigEchoSocket @OnWebSocketMessage public void onText(Session session, String message) { - if (session.isOpen()) + if (!session.isOpen()) { + LOG.warn("Session is closed"); return; } session.getRemote().sendStringByFuture(message);