From afa71a6b4bc1577c0517a07d40537b3e5ab539da Mon Sep 17 00:00:00 2001 From: Benjamin Sergeant Date: Tue, 3 Sep 2019 12:02:29 -0700 Subject: [PATCH] Framentation: data and continuation blocks received out of order (fix autobahn test: 5.9 through 5.20 Fragmentation) --- DOCKER_VERSION | 2 +- docs/CHANGELOG.md | 20 ++++++++++++-------- ixwebsocket/IXWebSocket.cpp | 4 ++-- ixwebsocket/IXWebSocketCloseConstants.cpp | 5 ++++- ixwebsocket/IXWebSocketCloseConstants.h | 5 ++++- ixwebsocket/IXWebSocketTransport.cpp | 15 ++++++++++++++- ixwebsocket/IXWebSocketVersion.h | 2 +- 7 files changed, 38 insertions(+), 15 deletions(-) diff --git a/DOCKER_VERSION b/DOCKER_VERSION index 76e9e619..220d8e0a 100644 --- a/DOCKER_VERSION +++ b/DOCKER_VERSION @@ -1 +1 @@ -5.1.4 +5.1.5 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 288f05c5..324d20a3 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog All notable changes to this project will be documented in this file. +## [5.1.5] - 2019-09-03 + +Framentation: data and continuation blocks received out of order (fix autobahn test: 5.9 through 5.20 Fragmentation) + ## [5.1.4] - 2019-09-03 Sending invalid UTF-8 TEXT message should fail and close the connection (fix **tons** of autobahn test: 6.X UTF-8 Handling) @@ -19,17 +23,17 @@ Close connections when reserved bits are used (fix autobahn test: 3.X Reserved B ## [5.1.0] - 2019-08-31 -ws autobahn / Add code to test websocket client compliance with the autobahn test-suite -add utf-8 validation code, not hooked up properly yet -Ping received with a payload too large (> 125 bytes) trigger a connection closure -cobra / add tracking about published messages -cobra / publish returns a message id, that can be used when -cobra / new message type in the message received handler when publish/ok is received (can be used to implement an ack system). +- ws autobahn / Add code to test websocket client compliance with the autobahn test-suite +- add utf-8 validation code, not hooked up properly yet +- Ping received with a payload too large (> 125 bytes) trigger a connection closure +- cobra / add tracking about published messages +- cobra / publish returns a message id, that can be used when +- cobra / new message type in the message received handler when publish/ok is received (can be used to implement an ack system). ## [5.0.9] - 2019-08-30 -User-Agent header is set when not specified. -New option to cap the max wait between reconnection attempts. Still default to 10s. (setMaxWaitBetweenReconnectionRetries). +- User-Agent header is set when not specified. +- New option to cap the max wait between reconnection attempts. Still default to 10s. (setMaxWaitBetweenReconnectionRetries). ``` ws connect --max_wait 5000 ws://example.com # will only wait 5 seconds max between reconnection attempts diff --git a/ixwebsocket/IXWebSocket.cpp b/ixwebsocket/IXWebSocket.cpp index b94af0b9..0449c260 100644 --- a/ixwebsocket/IXWebSocket.cpp +++ b/ixwebsocket/IXWebSocket.cpp @@ -461,8 +461,8 @@ namespace ix { if (!isValidUtf8(text)) { - close(WebSocketCloseConstants::kNormalClosureCode, - WebSocketCloseConstants::kInvalidUtf8); + close(WebSocketCloseConstants::kInvalidFramePayloadData, + WebSocketCloseConstants::kInvalidFramePayloadDataMessage); return false; } return sendMessage(text, SendMessageKind::Text, onProgressCallback); diff --git a/ixwebsocket/IXWebSocketCloseConstants.cpp b/ixwebsocket/IXWebSocketCloseConstants.cpp index deb0704a..6e9036e3 100644 --- a/ixwebsocket/IXWebSocketCloseConstants.cpp +++ b/ixwebsocket/IXWebSocketCloseConstants.cpp @@ -11,6 +11,7 @@ namespace ix const uint16_t WebSocketCloseConstants::kNormalClosureCode(1000); const uint16_t WebSocketCloseConstants::kInternalErrorCode(1011); const uint16_t WebSocketCloseConstants::kAbnormalCloseCode(1006); + const uint16_t WebSocketCloseConstants::kInvalidFramePayloadData(1007); const uint16_t WebSocketCloseConstants::kProtocolErrorCode(1002); const uint16_t WebSocketCloseConstants::kNoStatusCodeErrorCode(1005); @@ -23,5 +24,7 @@ namespace ix const std::string WebSocketCloseConstants::kProtocolErrorReservedBitUsed("Reserved bit used"); const std::string WebSocketCloseConstants::kProtocolErrorPingPayloadOversized("Ping reason control frame with payload length > 125 octets"); const std::string WebSocketCloseConstants::kProtocolErrorCodeControlMessageFragmented("Control message fragmented"); - const std::string WebSocketCloseConstants::kInvalidUtf8("Invalid UTF-8"); + const std::string WebSocketCloseConstants::kProtocolErrorCodeDataOpcodeOutOfSequence("Fragmentation: data message out of sequence"); + const std::string WebSocketCloseConstants::kProtocolErrorCodeContinuationOpCodeOutOfSequence("Fragmentation: continuation opcode out of sequence"); + const std::string WebSocketCloseConstants::kInvalidFramePayloadDataMessage("Invalid frame payload data"); } diff --git a/ixwebsocket/IXWebSocketCloseConstants.h b/ixwebsocket/IXWebSocketCloseConstants.h index fb13b6a0..7a10405f 100644 --- a/ixwebsocket/IXWebSocketCloseConstants.h +++ b/ixwebsocket/IXWebSocketCloseConstants.h @@ -18,6 +18,7 @@ namespace ix static const uint16_t kAbnormalCloseCode; static const uint16_t kProtocolErrorCode; static const uint16_t kNoStatusCodeErrorCode; + static const uint16_t kInvalidFramePayloadData; static const std::string kNormalClosureMessage; static const std::string kInternalErrorMessage; @@ -28,6 +29,8 @@ namespace ix static const std::string kProtocolErrorReservedBitUsed; static const std::string kProtocolErrorPingPayloadOversized; static const std::string kProtocolErrorCodeControlMessageFragmented; - static const std::string kInvalidUtf8; + static const std::string kProtocolErrorCodeDataOpcodeOutOfSequence; + static const std::string kProtocolErrorCodeContinuationOpCodeOutOfSequence; + static const std::string kInvalidFramePayloadDataMessage; }; } // namespace ix diff --git a/ixwebsocket/IXWebSocketTransport.cpp b/ixwebsocket/IXWebSocketTransport.cpp index 9c0d7d3f..20221cba 100644 --- a/ixwebsocket/IXWebSocketTransport.cpp +++ b/ixwebsocket/IXWebSocketTransport.cpp @@ -551,7 +551,7 @@ namespace ix || ws.opcode == wsheader_type::PONG || ws.opcode == wsheader_type::CLOSE )){ - // Cntrol messages should not be fragmented + // Control messages should not be fragmented close(WebSocketCloseConstants::kProtocolErrorCode, WebSocketCloseConstants::kProtocolErrorCodeControlMessageFragmented); return; @@ -571,6 +571,19 @@ namespace ix (ws.opcode == wsheader_type::TEXT_FRAME) ? MessageKind::MSG_TEXT : MessageKind::MSG_BINARY; + + // Continuation message needs to follow a non-fin TEXT or BINARY message + if (!_chunks.empty()) + { + close(WebSocketCloseConstants::kProtocolErrorCode, + WebSocketCloseConstants::kProtocolErrorCodeDataOpcodeOutOfSequence); + } + } + else if (_chunks.empty()) + { + // Continuation message need to follow a non-fin TEXT or BINARY message + close(WebSocketCloseConstants::kProtocolErrorCode, + WebSocketCloseConstants::kProtocolErrorCodeContinuationOpCodeOutOfSequence); } // diff --git a/ixwebsocket/IXWebSocketVersion.h b/ixwebsocket/IXWebSocketVersion.h index aff84a3a..9656cab3 100644 --- a/ixwebsocket/IXWebSocketVersion.h +++ b/ixwebsocket/IXWebSocketVersion.h @@ -6,4 +6,4 @@ #pragma once -#define IX_WEBSOCKET_VERSION "5.1.4" +#define IX_WEBSOCKET_VERSION "5.1.5"