Framentation: data and continuation blocks received out of order (fix autobahn test: 5.9 through 5.20 Fragmentation)

DOCKER_VERSION

@@ -1 +1 @@
-5.1.1
+5.1.5

docs/CHANGELOG.md

@@ -1,27 +1,39 @@
 # 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)
+
+## [5.1.3] - 2019-09-03
+
+Message type (TEXT or BINARY) is invalid for received fragmented messages (fix autobahn test: 5.3 through 5.8 Fragmentation)
+
 ## [5.1.2] - 2019-09-02
 
-Ping and Pong messages cannot be fragmented (autobahn test: 5.1 and 5.2 Fragmentation)
+Ping and Pong messages cannot be fragmented (fix autobahn test: 5.1 and 5.2 Fragmentation)
 
 ## [5.1.1] - 2019-09-01
 
-Close connections when reserved bits are used (autobahn test: 3 Reserved Bits)
+Close connections when reserved bits are used (fix autobahn test: 3.X Reserved Bits)
 
 ## [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

ixwebsocket/IXWebSocket.cpp

@@ -447,9 +447,7 @@ namespace ix
                                       bool binary,
                                       const OnProgressCallback& onProgressCallback)
     {
-        return sendMessage(data,
-                           (binary) ? SendMessageKind::Binary: SendMessageKind::Text,
-                           onProgressCallback);
+        return (binary) ? sendBinary(data, onProgressCallback) : sendText(data, onProgressCallback);
     }
 
     WebSocketSendInfo WebSocket::sendBinary(const std::string& text,
@@ -463,7 +461,8 @@ namespace ix
     {
         if (!isValidUtf8(text))
         {
-            stop();
+            close(WebSocketCloseConstants::kInvalidFramePayloadData,
+                  WebSocketCloseConstants::kInvalidFramePayloadDataMessage);
             return false;
         }
         return sendMessage(text, SendMessageKind::Text, onProgressCallback);

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,4 +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::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");
 }

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,5 +29,8 @@ namespace ix
         static const std::string kProtocolErrorReservedBitUsed;
         static const std::string kProtocolErrorPingPayloadOversized;
         static const std::string kProtocolErrorCodeControlMessageFragmented;
+        static const std::string kProtocolErrorCodeDataOpcodeOutOfSequence;
+        static const std::string kProtocolErrorCodeContinuationOpCodeOutOfSequence;
+        static const std::string kInvalidFramePayloadDataMessage;
     };
 } // namespace ix

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;
@@ -565,17 +565,33 @@ namespace ix
             ) {
                 unmaskReceiveBuffer(ws);
 
-                MessageKind messageKind =
-                    (ws.opcode == wsheader_type::TEXT_FRAME)
-                    ? MessageKind::MSG_TEXT
-                    : MessageKind::MSG_BINARY;
+                if (ws.opcode != wsheader_type::CONTINUATION)
+                {
+                    _fragmentedMessageKind =
+                        (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);
+                }
 
                 //
                 // Usual case. Small unfragmented messages
                 //
                 if (ws.fin && _chunks.empty())
                 {
-                    emitMessage(messageKind,
+                    emitMessage(_fragmentedMessageKind,
                                 std::string(_rxbuf.begin()+ws.header_size,
                                             _rxbuf.begin()+ws.header_size+(size_t) ws.N),
                                 ws,
@@ -595,7 +611,8 @@ namespace ix
                                              _rxbuf.begin()+ws.header_size+(size_t)ws.N));
                     if (ws.fin)
                     {
-                        emitMessage(messageKind, getMergedChunks(), ws, onMessageCallback);
+                        emitMessage(_fragmentedMessageKind, getMergedChunks(),
+                                    ws, onMessageCallback);
                         _chunks.clear();
                     }
                     else

ixwebsocket/IXWebSocketTransport.h

@@ -151,6 +151,11 @@ namespace ix
         // size increased 2 fold, while appending to a list has a fixed cost.
         std::list<std::vector<uint8_t>> _chunks;
 
+        // Record the message kind (will be TEXT or BINARY) for a fragmented
+        // message, present in the first chunk, since the final chunk will be a
+        // CONTINUATION opcode and doesn't tell the full message kind
+        MessageKind _fragmentedMessageKind;
+
         // Fragments are 32K long
         static constexpr size_t kChunkSize = 1 << 15;
 

ixwebsocket/IXWebSocketVersion.h

@@ -6,4 +6,4 @@
 
 #pragma once
 
-#define IX_WEBSOCKET_VERSION "5.1.2"
+#define IX_WEBSOCKET_VERSION "5.1.5"