From 2d5fa0a3bc8eb17518bcda1eb1e09c2aa69aec4c Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Mon, 2 Jul 2012 16:06:43 -0500 Subject: [PATCH 1/2] close short to int --- jetty-websocket/pom.xml | 2 +- .../jetty/websocket/api/CloseException.java | 10 +++---- .../jetty/websocket/api/StatusCode.java | 26 +++++++++---------- .../jetty/websocket/frames/ControlFrame.java | 5 ++-- .../parser/ClosePayloadParserTest.java | 6 ++--- 5 files changed, 24 insertions(+), 25 deletions(-) diff --git a/jetty-websocket/pom.xml b/jetty-websocket/pom.xml index 4dd8817c856..0501bf303f6 100644 --- a/jetty-websocket/pom.xml +++ b/jetty-websocket/pom.xml @@ -14,7 +14,7 @@ websocket-core - websocket-client + websocket-server diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/CloseException.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/CloseException.java index aa3c41a191a..fdfe9d0e4c5 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/CloseException.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/CloseException.java @@ -3,27 +3,27 @@ package org.eclipse.jetty.websocket.api; @SuppressWarnings("serial") public class CloseException extends WebSocketException { - private short statusCode; + private int statusCode; - public CloseException(short closeCode, String message) + public CloseException(int closeCode, String message) { super(message); this.statusCode = closeCode; } - public CloseException(short closeCode, String message, Throwable cause) + public CloseException(int closeCode, String message, Throwable cause) { super(message,cause); this.statusCode = closeCode; } - public CloseException(short closeCode, Throwable cause) + public CloseException(int closeCode, Throwable cause) { super(cause); this.statusCode = closeCode; } - public short getStatusCode() + public int getStatusCode() { return statusCode; } diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java index 78e3559e326..89906c5cba5 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java @@ -10,53 +10,53 @@ public class StatusCode *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short NORMAL = 1000; + public final static int NORMAL = 1000; /** * 1001 indicates that an endpoint is "going away", such as a server going down or a browser having navigated away from a page. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short SHUTDOWN = 1001; + public final static int SHUTDOWN = 1001; /** * 1002 indicates that an endpoint is terminating the connection due to a protocol error. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short PROTOCOL = 1002; + public final static int PROTOCOL = 1002; /** * 1003 indicates that an endpoint is terminating the connection because it has received a type of data it cannot accept (e.g., an endpoint that understands * only text data MAY send this if it receives a binary message). *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short BAD_DATA = 1003; + public final static int BAD_DATA = 1003; /** * Reserved. The specific meaning might be defined in the future. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short UNDEFINED = 1004; + public final static int UNDEFINED = 1004; /** * 1005 is a reserved value and MUST NOT be set as a status code in a Close control frame by an endpoint. It is designated for use in applications expecting * a status code to indicate that no status code was actually present. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short NO_CODE = 1005; + public final static int NO_CODE = 1005; /** * 1006 is a reserved value and MUST NOT be set as a status code in a Close control frame by an endpoint. It is designated for use in applications expecting * a status code to indicate that the connection was closed abnormally, e.g., without sending or receiving a Close control frame. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short NO_CLOSE = 1006; + public final static int NO_CLOSE = 1006; /** * 1007 indicates that an endpoint is terminating the connection because it has received data within a message that was not consistent with the type of the * message (e.g., non-UTF-8 [RFC3629] data within a text message). *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short BAD_PAYLOAD = 1007; + public final static int BAD_PAYLOAD = 1007; /** * 1008 indicates that an endpoint is terminating the connection because it has received a message that violates its policy. This is a generic status code * that can be returned when there is no other more suitable status code (e.g., 1003 or 1009) or if there is a need to hide specific details about the @@ -64,13 +64,13 @@ public class StatusCode *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short POLICY_VIOLATION = 1008; + public final static int POLICY_VIOLATION = 1008; /** * 1009 indicates that an endpoint is terminating the connection because it has received a message that is too big for it to process. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short MESSAGE_TOO_LARGE = 1009; + public final static int MESSAGE_TOO_LARGE = 1009; /** * 1010 indicates that an endpoint (client) is terminating the connection because it has expected the server to negotiate one or more extension, but the * server didn't return them in the response message of the WebSocket handshake. The list of extensions that are needed SHOULD appear in the /reason/ part @@ -78,18 +78,18 @@ public class StatusCode *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short REQUIRED_EXTENSION = 1010; + public final static int REQUIRED_EXTENSION = 1010; /** * 1011 indicates that a server is terminating the connection because it encountered an unexpected condition that prevented it from fulfilling the request. *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short SERVER_ERROR = 1011; + public final static int SERVER_ERROR = 1011; /** * 1015 is a reserved value and MUST NOT be set as a status code in a Close control frame by an endpoint. It is designated for use in applications expecting * a status code to indicate that the connection was closed due to a failure to perform a TLS handshake (e.g., the server certificate can't be verified). *

* See RFC 6455, Section 7.4.1 Defined Status Codes. */ - public final static short FAILED_TLS_HANDSHAKE = 1015; + public final static int FAILED_TLS_HANDSHAKE = 1015; } diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/frames/ControlFrame.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/frames/ControlFrame.java index 9326f1e4209..96d9141e02b 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/frames/ControlFrame.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/frames/ControlFrame.java @@ -4,7 +4,6 @@ import java.nio.ByteBuffer; import org.eclipse.jetty.websocket.api.OpCode; import org.eclipse.jetty.websocket.api.ProtocolException; -import org.eclipse.jetty.websocket.api.WebSocketException; public abstract class ControlFrame extends BaseFrame { @@ -40,9 +39,9 @@ public abstract class ControlFrame extends BaseFrame @Override public void setPayload(byte[] buf) { - if ( buf.length > 125 ) + if (buf.length > MAX_PAYLOAD) { - throw new WebSocketException("Control Payloads can not exceed 125 bytes in length."); + throw new ProtocolException("Control Payloads can not exceed 125 bytes in length."); } super.setPayload(buf); diff --git a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/parser/ClosePayloadParserTest.java b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/parser/ClosePayloadParserTest.java index 6280a3b4e0c..bee74aadb87 100644 --- a/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/parser/ClosePayloadParserTest.java +++ b/jetty-websocket/websocket-core/src/test/java/org/eclipse/jetty/websocket/parser/ClosePayloadParserTest.java @@ -1,6 +1,6 @@ package org.eclipse.jetty.websocket.parser; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.is; import java.nio.ByteBuffer; @@ -22,7 +22,7 @@ public class ClosePayloadParserTest byte utf[] = expectedReason.getBytes(StringUtil.__UTF8_CHARSET); ByteBuffer payload = ByteBuffer.allocate(utf.length + 2); - payload.putShort(StatusCode.NORMAL); + payload.putChar((char)StatusCode.NORMAL); payload.put(utf,0,utf.length); payload.flip(); @@ -42,7 +42,7 @@ public class ClosePayloadParserTest capture.assertNoErrors(); capture.assertHasFrame(CloseFrame.class,1); CloseFrame close = (CloseFrame)capture.getFrames().get(0); - Assert.assertThat("CloseFrame.statusCode",(short)close.getStatusCode(),is(StatusCode.NORMAL)); + Assert.assertThat("CloseFrame.statusCode",close.getStatusCode(),is(StatusCode.NORMAL)); Assert.assertThat("CloseFrame.data",close.getReason(),is(expectedReason)); } } From 46da5fe5e2647ca3152cf133a0a46c8b38eba01c Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Mon, 2 Jul 2012 17:23:16 -0500 Subject: [PATCH 2/2] couple more test cases and fixed fragmented control frame response --- .../websocket/api/WebSocketEventDriver.java | 2 +- .../jetty/websocket/parser/Parser.java | 5 +- .../server/WebSocketAsyncConnection.java | 3 + .../websocket/server/ab/TestABCase5.java | 244 ++++++++++++++++++ 4 files changed, 251 insertions(+), 3 deletions(-) create mode 100644 jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase5.java diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/WebSocketEventDriver.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/WebSocketEventDriver.java index ab74b8bf7ef..f4a614cad72 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/WebSocketEventDriver.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/api/WebSocketEventDriver.java @@ -201,7 +201,7 @@ public class WebSocketEventDriver implements Parser.Listener reason = reason.substring(0,CloseFrame.MAX_REASON); } } - LOG.debug("terminateConnection({},{})",statusCode,reason); + LOG.debug("terminateConnection({},{})",statusCode,rawreason); connection.close(statusCode,reason); } catch (IOException e) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/parser/Parser.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/parser/Parser.java index 9dac059c610..c85512328b6 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/parser/Parser.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/parser/Parser.java @@ -9,6 +9,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.websocket.api.OpCode; +import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.api.WebSocketException; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.frames.BaseFrame; @@ -125,14 +126,14 @@ public class Parser if (opcode.isControlFrame() && !fin) { - throw new WebSocketException("Fragmented Control Frame [" + opcode.name() + "]"); + throw new ProtocolException("Fragmented Control Frame [" + opcode.name() + "]"); } if (opcode == OpCode.CONTINUATION) { if (parser == null) { - throw new WebSocketException("Fragment continuation frame without prior !FIN"); + throw new ProtocolException("Fragment continuation frame without prior !FIN"); } } diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketAsyncConnection.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketAsyncConnection.java index f3c81e18d89..e64ddbf2b6a 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketAsyncConnection.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketAsyncConnection.java @@ -17,7 +17,9 @@ import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.FutureCallback; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.websocket.api.CloseException; import org.eclipse.jetty.websocket.api.ExtensionConfig; +import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketConnection; import org.eclipse.jetty.websocket.api.WebSocketPolicy; @@ -186,6 +188,7 @@ public class WebSocketAsyncConnection extends AbstractAsyncConnection implements terminateConnection(StatusCode.PROTOCOL,null); break; } + parser.parse(buffer); } } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase5.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase5.java new file mode 100644 index 00000000000..ea54c931d4b --- /dev/null +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase5.java @@ -0,0 +1,244 @@ +package org.eclipse.jetty.websocket.server.ab; + +import static org.hamcrest.Matchers.*; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Queue; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; +import org.eclipse.jetty.websocket.api.OpCode; +import org.eclipse.jetty.websocket.api.WebSocketAdapter; +import org.eclipse.jetty.websocket.frames.BaseFrame; +import org.eclipse.jetty.websocket.frames.CloseFrame; +import org.eclipse.jetty.websocket.frames.PingFrame; +import org.eclipse.jetty.websocket.frames.TextFrame; +import org.eclipse.jetty.websocket.generator.FrameGenerator; +import org.eclipse.jetty.websocket.server.SimpleServletServer; +import org.eclipse.jetty.websocket.server.WebSocketServerFactory; +import org.eclipse.jetty.websocket.server.WebSocketServlet; +import org.eclipse.jetty.websocket.server.blockhead.BlockheadClient; +import org.eclipse.jetty.websocket.server.examples.MyEchoServlet; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameters; + +public class TestABCase5 +{ + @SuppressWarnings("serial") + public static class RFCServlet extends WebSocketServlet + { + @Override + public void registerWebSockets(WebSocketServerFactory factory) + { + factory.register(RFCSocket.class); + } + } + + public static class RFCSocket extends WebSocketAdapter + { + private static Logger LOG = Log.getLogger(RFCSocket.class); + + @Override + public void onWebSocketText(String message) + { + LOG.debug("onWebSocketText({})",message); + // Test the RFC 6455 close code 1011 that should close + // trigger a WebSocket server terminated close. + if (message.equals("CRASH")) + { + System.out.printf("Got OnTextMessage"); + throw new RuntimeException("Something bad happened"); + } + + // echo the message back. + try + { + getConnection().write(message); + } + catch (IOException e) + { + e.printStackTrace(System.err); + } + } + } + + private static SimpleServletServer server; + + @BeforeClass + public static void startServer() throws Exception + { + server = new SimpleServletServer(new MyEchoServlet()); + server.start(); + } + + @AfterClass + public static void stopServer() + { + server.stop(); + } + + @Test + public void testCase5_1PingIn2Packets() throws Exception + { + BlockheadClient client = new BlockheadClient(server.getServerUri()); + try + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + ByteBuffer buf = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf); + + String fragment1 = "fragment1"; + + buf.put((byte)(0x00 | OpCode.PING.getCode())); + + byte b = 0x00; // no masking + b |= fragment1.length() & 0x7F; + buf.put(b); + buf.put(fragment1.getBytes()); + BufferUtil.flipToFlush(buf,0); + + client.writeRaw(buf); + + ByteBuffer buf2 = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf2); + + String fragment2 = "fragment2"; + + buf2.put((byte)(0x80 | OpCode.PING.getCode())); + b = 0x00; // no masking + b |= fragment2.length() & 0x7F; + buf2.put(b); + buf2.put(fragment2.getBytes()); + BufferUtil.flipToFlush(buf2,0); + + client.writeRaw(buf2); + + // Read frame (hopefully text frame) + Queue frames = client.readFrames(1,TimeUnit.MILLISECONDS,500); + CloseFrame closeFrame = (CloseFrame)frames.remove(); + Assert.assertThat("CloseFrame.status code",closeFrame.getStatusCode(),is(1002)); + } + finally + { + client.close(); + } + } + + @Test + public void testCase5_2PongIn2Packets() throws Exception + { + BlockheadClient client = new BlockheadClient(server.getServerUri()); + try + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + ByteBuffer buf = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf); + + String fragment1 = "fragment1"; + + buf.put((byte)(0x00 | OpCode.PONG.getCode())); + + byte b = 0x00; // no masking + b |= fragment1.length() & 0x7F; + buf.put(b); + buf.put(fragment1.getBytes()); + BufferUtil.flipToFlush(buf,0); + + client.writeRaw(buf); + + ByteBuffer buf2 = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf2); + + String fragment2 = "fragment2"; + + buf2.put((byte)(0x80 | OpCode.CONTINUATION.getCode())); + b = 0x00; // no masking + b |= fragment2.length() & 0x7F; + buf2.put(b); + buf2.put(fragment2.getBytes()); + BufferUtil.flipToFlush(buf2,0); + + client.writeRaw(buf2); + + // Read frame (hopefully text frame) + Queue frames = client.readFrames(1,TimeUnit.MILLISECONDS,500); + CloseFrame closeFrame = (CloseFrame)frames.remove(); + Assert.assertThat("CloseFrame.status code",closeFrame.getStatusCode(),is(1002)); + } + finally + { + client.close(); + } + } + + + + @Test + @Ignore ("not re-assembling the strings, something odd with echo socket") + public void testCase5_3TextIn2Packets() throws Exception + { + BlockheadClient client = new BlockheadClient(server.getServerUri()); + try + { + client.connect(); + client.sendStandardRequest(); + client.expectUpgradeResponse(); + + ByteBuffer buf = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf); + + String fragment1 = "fragment1"; + + buf.put((byte)(0x00 | OpCode.TEXT.getCode())); + + byte b = 0x00; // no masking + b |= fragment1.length() & 0x7F; + buf.put(b); + buf.put(fragment1.getBytes()); + BufferUtil.flipToFlush(buf,0); + + client.writeRaw(buf); + + ByteBuffer buf2 = ByteBuffer.allocate(FrameGenerator.OVERHEAD + 2); + BufferUtil.clearToFill(buf2); + + String fragment2 = "fragment2"; + + buf2.put((byte)(0x80 | OpCode.CONTINUATION.getCode())); + b = 0x00; // no masking + b |= fragment2.length() & 0x7F; + buf2.put(b); + buf2.put(fragment2.getBytes()); + BufferUtil.flipToFlush(buf2,0); + + client.writeRaw(buf2); + + // Read frame (hopefully text frame) + Queue frames = client.readFrames(1,TimeUnit.MILLISECONDS,500); + TextFrame textFrame = (TextFrame)frames.remove(); + Assert.assertThat("TextFrame.payload",textFrame.getPayloadUTF8(),is(fragment1 + fragment2)); + } + finally + { + client.close(); + } + } +}