From 796da084b4bc40c855b9ed6b18ad12865aec0c38 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 16 Jun 2017 14:23:29 -0700 Subject: [PATCH] Issue #1625 - Support new IANA declared websocket close status codes --- .../jetty/websocket/api/StatusCode.java | 33 ++++++++++- .../websocket/client/SlowServerTest.java | 4 +- .../jetty/websocket/common/CloseInfo.java | 37 ++++++++---- .../jetty/websocket/common/CloseInfoTest.java | 19 ++----- .../websocket/server/ab/TestABCase3.java | 5 -- .../server/ab/TestABCase7_BadStatusCodes.java | 16 +++--- .../ab/TestABCase7_GoodStatusCodes.java | 5 +- .../test/resources/jetty-logging.properties | 56 +++++++++---------- 8 files changed, 106 insertions(+), 69 deletions(-) diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java index 4dfe06c906b..3b4d8df23ff 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/StatusCode.java @@ -22,7 +22,7 @@ package org.eclipse.jetty.websocket.api; * The RFC 6455 specified status codes and IANA: WebSocket Close Code Number Registry */ -public class StatusCode +public final class StatusCode { /** * 1000 indicates a normal closure, meaning that the purpose for which the connection was established has been fulfilled. @@ -137,6 +137,13 @@ public class StatusCode */ public final static int TRY_AGAIN_LATER = 1013; + /** + * 1014 indicates that a gateway or proxy received and invalid upstream response. + *

+ * See [hybi] WebSocket Subprotocol Close Code: Bad Gateway + */ + public final static int INVALID_UPSTREAM_RESPONSE = 1014; + /** * 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). @@ -144,4 +151,28 @@ public class StatusCode * See RFC 6455, Section 7.4.1 Defined Status Codes. */ public final static int FAILED_TLS_HANDSHAKE = 1015; + + /** + * Test if provided status code can be sent/received on a WebSocket close. + *

+ * This honors the RFC6455 rules and IANA rules. + *

+ * @param statusCode the statusCode to test + * @return true if transmittable + */ + public static boolean isTransmittable(int statusCode) + { + return (statusCode == NORMAL) || + (statusCode == SHUTDOWN) || + (statusCode == PROTOCOL) || + (statusCode == BAD_DATA) || + (statusCode == BAD_PAYLOAD) || + (statusCode == POLICY_VIOLATION) || + (statusCode == MESSAGE_TOO_LARGE) || + (statusCode == REQUIRED_EXTENSION) || + (statusCode == SERVER_ERROR) || + (statusCode == SERVICE_RESTART) || + (statusCode == TRY_AGAIN_LATER) || + (statusCode == INVALID_UPSTREAM_RESPONSE); + } } diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SlowServerTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SlowServerTest.java index f2d76a2de35..3fae9a28704 100644 --- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SlowServerTest.java +++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/SlowServerTest.java @@ -18,7 +18,7 @@ package org.eclipse.jetty.websocket.client; -import static org.hamcrest.Matchers.*; +import static org.hamcrest.Matchers.is; import java.net.URI; import java.util.concurrent.Future; @@ -34,9 +34,11 @@ import org.eclipse.jetty.websocket.common.test.IBlockheadServerConnection; import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +@Ignore("TODO: Flappy Test") public class SlowServerTest { @Rule diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/CloseInfo.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/CloseInfo.java index a411716c38d..0e1b3fc0b77 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/CloseInfo.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/CloseInfo.java @@ -33,7 +33,7 @@ import org.eclipse.jetty.websocket.common.frames.CloseFrame; public class CloseInfo { - private int statusCode; + private int statusCode = 0; private byte[] reasonBytes; public CloseInfo() @@ -71,11 +71,7 @@ public class CloseInfo if (validate) { - if ((statusCode < StatusCode.NORMAL) || (statusCode == StatusCode.UNDEFINED) || (statusCode == StatusCode.NO_CLOSE) - || (statusCode == StatusCode.NO_CODE) || ((statusCode > 1011) && (statusCode <= 2999)) || (statusCode >= 5000)) - { - throw new ProtocolException("Invalid close code: " + statusCode); - } + assertValidStatusCode(statusCode); } if (data.remaining() > 0) @@ -142,6 +138,27 @@ public class CloseInfo } } + private void assertValidStatusCode(int statusCode) + { + // Status Codes outside of RFC6455 defined scope + if ((statusCode <= 999) || (statusCode >= 5000)) + { + throw new ProtocolException("Out of range close status code: " + statusCode); + } + + // Status Codes not allowed to exist in a Close frame (per RFC6455) + if ((statusCode == StatusCode.NO_CLOSE) || (statusCode == StatusCode.NO_CODE) || (statusCode == StatusCode.FAILED_TLS_HANDSHAKE)) + { + throw new ProtocolException("Frame forbidden close status code: " + statusCode); + } + + // Status Code is in defined "reserved space" and is declared (all others are invalid) + if ((statusCode >= 1000) && (statusCode <= 2999) && !StatusCode.isTransmittable(statusCode)) + { + throw new ProtocolException("RFC6455 and IANA Undefined close status code: " + statusCode); + } + } + private ByteBuffer asByteBuffer() { if ((statusCode == StatusCode.NO_CLOSE) || (statusCode == StatusCode.NO_CODE) || (statusCode == (-1))) @@ -175,12 +192,10 @@ public class CloseInfo { CloseFrame frame = new CloseFrame(); frame.setFin(true); - if ((statusCode >= 1000) && (statusCode != StatusCode.NO_CLOSE) && (statusCode != StatusCode.NO_CODE)) + // Frame forbidden codes result in no status code (and no reason string) + if ((statusCode != StatusCode.NO_CLOSE) && (statusCode != StatusCode.NO_CODE) && (statusCode != StatusCode.FAILED_TLS_HANDSHAKE)) { - if (statusCode == StatusCode.FAILED_TLS_HANDSHAKE) - { - throw new ProtocolException("Close Frame with status code " + statusCode + " not allowed (per RFC6455)"); - } + assertValidStatusCode(statusCode); frame.setPayload(asByteBuffer()); } return frame; diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/CloseInfoTest.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/CloseInfoTest.java index 2966271f6a1..67f1d1e3c61 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/CloseInfoTest.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/CloseInfoTest.java @@ -22,17 +22,14 @@ import static org.eclipse.jetty.websocket.api.StatusCode.FAILED_TLS_HANDSHAKE; import static org.eclipse.jetty.websocket.api.StatusCode.NORMAL; import static org.eclipse.jetty.websocket.api.StatusCode.NO_CLOSE; import static org.eclipse.jetty.websocket.api.StatusCode.NO_CODE; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; import java.nio.ByteBuffer; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.websocket.api.ProtocolException; import org.eclipse.jetty.websocket.common.frames.CloseFrame; import org.junit.Test; @@ -99,17 +96,11 @@ public class CloseInfoTest assertThat("close.code",close.getStatusCode(),is(FAILED_TLS_HANDSHAKE)); assertThat("close.reason",close.getReason(),nullValue()); - try - { - @SuppressWarnings("unused") - CloseFrame frame = close.asFrame(); - fail("Expected " + ProtocolException.class.getName()); - } - catch (ProtocolException e) - { - // expected path - assertThat("ProtocolException message",e.getMessage(),containsString("not allowed (per RFC6455)")); - } + CloseFrame frame = close.asFrame(); + assertThat("close frame op code",frame.getOpCode(),is(OpCode.CLOSE)); + // should result in no payload + assertThat("close frame has payload",frame.hasPayload(),is(false)); + assertThat("close frame payload length",frame.getPayloadLength(),is(0)); } /** diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase3.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase3.java index 891088018c1..4172de15563 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase3.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase3.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.util.log.StacklessLogging; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.common.CloseInfo; @@ -32,15 +31,11 @@ import org.eclipse.jetty.websocket.common.frames.BinaryFrame; import org.eclipse.jetty.websocket.common.frames.PingFrame; import org.eclipse.jetty.websocket.common.frames.TextFrame; import org.eclipse.jetty.websocket.common.test.Fuzzer; -import org.junit.Ignore; import org.junit.Test; -import org.junit.runner.RunWith; /** * Test various RSV violations */ -@Ignore -@RunWith(AdvancedRunner.class) public class TestABCase3 extends AbstractABCase { /** diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_BadStatusCodes.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_BadStatusCodes.java index a914953731a..fc02dc63bb2 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_BadStatusCodes.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_BadStatusCodes.java @@ -45,7 +45,7 @@ public class TestABCase7_BadStatusCodes extends AbstractABCase { private static final Logger LOG = Log.getLogger(TestABCase7_GoodStatusCodes.class); - @Parameters + @Parameters(name = "{1} / {0}") public static Collection data() { // The various Good UTF8 sequences as a String (hex form) @@ -54,13 +54,13 @@ public class TestABCase7_BadStatusCodes extends AbstractABCase // @formatter:off data.add(new Object[] { "7.9.1", 0 }); data.add(new Object[] { "7.9.2", 999 }); - data.add(new Object[] { "7.9.3", 1004 }); - data.add(new Object[] { "7.9.4", 1005 }); - data.add(new Object[] { "7.9.5", 1006 }); - data.add(new Object[] { "7.9.6", 1012 }); - data.add(new Object[] { "7.9.7", 1013 }); - data.add(new Object[] { "7.9.8", 1014 }); - data.add(new Object[] { "7.9.9", 1015 }); + data.add(new Object[] { "7.9.3", 1004 }); // RFC6455/UNDEFINED + data.add(new Object[] { "7.9.4", 1005 }); // RFC6455/Cannot Be Transmitted + data.add(new Object[] { "7.9.5", 1006 }); // RFC6455/Cannot Be Transmitted + // data.add(new Object[] { "7.9.6", 1012 }); - IANA Defined + // data.add(new Object[] { "7.9.7", 1013 }); - IANA Defined + // data.add(new Object[] { "7.9.8", 1014 }); - IANA Defined + data.add(new Object[] { "7.9.9", 1015 }); // RFC6455/Cannot Be Transmitted data.add(new Object[] { "7.9.10", 1016 }); data.add(new Object[] { "7.9.11", 1100 }); data.add(new Object[] { "7.9.12", 2000 }); diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_GoodStatusCodes.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_GoodStatusCodes.java index 7cb50f24b32..9dff95e8451 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_GoodStatusCodes.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7_GoodStatusCodes.java @@ -43,7 +43,7 @@ public class TestABCase7_GoodStatusCodes extends AbstractABCase { private static final Logger LOG = Log.getLogger(TestABCase7_GoodStatusCodes.class); - @Parameters + @Parameters(name = "{1} / {0}") public static Collection data() { // The various Good UTF8 sequences as a String (hex form) @@ -59,6 +59,9 @@ public class TestABCase7_GoodStatusCodes extends AbstractABCase data.add(new Object[] { "7.7.7", 1009 }); data.add(new Object[] { "7.7.8", 1010 }); data.add(new Object[] { "7.7.9", 1011 }); + data.add(new Object[] { "IANA Assigned", 1012 }); + data.add(new Object[] { "IANA Assigned", 1013 }); + data.add(new Object[] { "IANA Assigned", 1014 }); data.add(new Object[] { "7.7.10", 3000 }); data.add(new Object[] { "7.7.11", 3999 }); data.add(new Object[] { "7.7.12", 4000 }); diff --git a/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties index 924d0006cbb..2a711f20999 100644 --- a/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties @@ -1,29 +1,29 @@ -org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog -org.eclipse.jetty.LEVEL=WARN - -# org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG -# org.eclipse.jetty.websocket.LEVEL=DEBUG -# org.eclipse.jetty.websocket.LEVEL=INFO -# org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG -# org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG -# org.eclipse.jetty.websocket.common.Parser.LEVEL=DEBUG -# org.eclipse.jetty.websocket.common.Generator.LEVEL=DEBUG -# org.eclipse.jetty.websocket.server.ab.Fuzzer.LEVEL=DEBUG -# org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG -# org.eclipse.jetty.websocket.server.helper.LEVEL=DEBUG - -# org.eclipse.jetty.websocket.client.io.ConnectPromise.LEVEL=DEBUG -# org.eclipse.jetty.websocket.common.WebSocketSession_OPEN.LEVEL=DEBUG -# org.eclipse.jetty.websocket.common.io.IOState.LEVEL=DEBUG -# org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection_OPEN.LEVEL=DEBUG - -### Show state changes on BrowserDebugTool -# -- LEAVE THIS AT DEBUG LEVEL -- -org.eclipse.jetty.websocket.server.browser.LEVEL=DEBUG - -### Disabling intentional error out of RFCSocket -org.eclipse.jetty.websocket.server.helper.RFCSocket.LEVEL=OFF - -### Hiding Stack Traces from various test cases -org.eclipse.jetty.websocket.server.ab.ABSocket.STACKS=OFF +org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog +org.eclipse.jetty.LEVEL=WARN + +# org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG +# org.eclipse.jetty.websocket.LEVEL=DEBUG +# org.eclipse.jetty.websocket.LEVEL=INFO +# org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG +# org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG +# org.eclipse.jetty.websocket.common.Parser.LEVEL=DEBUG +# org.eclipse.jetty.websocket.common.Generator.LEVEL=DEBUG +# org.eclipse.jetty.websocket.server.ab.Fuzzer.LEVEL=DEBUG +# org.eclipse.jetty.websocket.server.blockhead.LEVEL=DEBUG +# org.eclipse.jetty.websocket.server.helper.LEVEL=DEBUG + +# org.eclipse.jetty.websocket.client.io.ConnectPromise.LEVEL=DEBUG +# org.eclipse.jetty.websocket.common.WebSocketSession_OPEN.LEVEL=DEBUG +# org.eclipse.jetty.websocket.common.io.IOState.LEVEL=DEBUG +# org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection_OPEN.LEVEL=DEBUG + +### Show state changes on BrowserDebugTool +# -- LEAVE THIS AT DEBUG LEVEL -- +org.eclipse.jetty.websocket.server.browser.LEVEL=DEBUG + +### Disabling intentional error out of RFCSocket +org.eclipse.jetty.websocket.server.helper.RFCSocket.LEVEL=OFF + +### Hiding Stack Traces from various test cases +org.eclipse.jetty.websocket.server.ab.ABSocket.STACKS=OFF org.eclipse.jetty.websocket.server.WebSocketCloseTest$FastFailSocket.STACKS=OFF \ No newline at end of file