Check for invalid use of data frame opcodes.

net/websockets/websocket_channel.cc

Index: net/websockets/websocket_channel.cc
diff --git a/net/websockets/websocket_channel.cc b/net/websockets/websocket_channel.cc
index 9e255ab1f7a0fa5ad918896e8dc9ccd3fd44bb4e..ba77163f96d80044345ab1faeabe51be586c6af0 100644
--- a/net/websockets/websocket_channel.cc
+++ b/net/websockets/websocket_channel.cc
@@ -229,7 +229,8 @@ WebSocketChannel::WebSocketChannel(
       state_(FRESHLY_CONSTRUCTED),
       notification_sender_(new HandshakeNotificationSender(this)),
       sending_text_message_(false),
-      receiving_text_message_(false) {}
+      receiving_text_message_(false),
+      expecting_to_read_continuation_(false) {}
 
 WebSocketChannel::~WebSocketChannel() {
   // The stream may hold a pointer to read_frames_, and so it needs to be
@@ -665,40 +666,22 @@ ChannelState WebSocketChannel::HandleFrame(
   }
   switch (opcode) {
     case WebSocketFrameHeader::kOpCodeText:    // fall-thru
-    case WebSocketFrameHeader::kOpCodeBinary:  // fall-thru
+    case WebSocketFrameHeader::kOpCodeBinary:
+      if (expecting_to_read_continuation_) {

[yhirano, 2014/02/13 05:36:42]

Shouldn't the received message be ignored when the state_ is not CONNECTED?

[Adam Rice, 2014/02/13 07:29:49]

Yes, you are right. Done.

+        return FailChannel(
+            "Received start of new message but previous message is unfinished.",
+            kWebSocketErrorProtocolError,
+            "Previous data frame unfinished");
+      }
+      return HandleDataFrame(opcode, final, data_buffer, size);
+
     case WebSocketFrameHeader::kOpCodeContinuation:
-      if (state_ == CONNECTED) {
-        if (opcode == WebSocketFrameHeader::kOpCodeText ||
-            (opcode == WebSocketFrameHeader::kOpCodeContinuation &&
-             receiving_text_message_)) {
-          // This call is not redundant when size == 0 because it tells us what
-          // the current state is.
-          StreamingUtf8Validator::State state =
-              incoming_utf8_validator_.AddBytes(
-                  size ? data_buffer->data() : NULL, size);
-          if (state == StreamingUtf8Validator::INVALID ||
-              (state == StreamingUtf8Validator::VALID_MIDPOINT && final)) {
-            return FailChannel("Could not decode a text frame as UTF-8.",
-                               kWebSocketErrorProtocolError,
-                               "Invalid UTF-8 in text frame");
-          }
-          receiving_text_message_ = !final;
-          DCHECK(!final || state == StreamingUtf8Validator::VALID_ENDPOINT);
-        }
-        // TODO(ricea): Can this copy be eliminated?
-        const char* const data_begin = size ? data_buffer->data() : NULL;
-        const char* const data_end = data_begin + size;
-        const std::vector<char> data(data_begin, data_end);
-        // TODO(ricea): Handle the case when ReadFrames returns far
-        // more data at once than should be sent in a single IPC. This needs to
-        // be handled carefully, as an overloaded IO thread is one possible
-        // cause of receiving very large chunks.
-
-        // Sends the received frame to the renderer process.
-        return event_interface_->OnDataFrame(final, opcode, data);
+      if (!expecting_to_read_continuation_) {

[yhirano, 2014/02/13 05:36:42]

ditto

[Adam Rice, 2014/02/13 07:29:49]

Done.

+        return FailChannel("Received unexpected continuation frame.",
+                           kWebSocketErrorProtocolError,
+                           "Unexpected continuation");
       }
-      VLOG(3) << "Ignored data packet received in state " << state_;
-      return CHANNEL_ALIVE;
+      return HandleDataFrame(opcode, final, data_buffer, size);
 
     case WebSocketFrameHeader::kOpCodePing:
       VLOG(1) << "Got Ping of size " << size;
@@ -759,6 +742,45 @@ ChannelState WebSocketChannel::HandleFrame(
   }
 }
 
+ChannelState WebSocketChannel::HandleDataFrame(
+    const WebSocketFrameHeader::OpCode opcode,
+    bool final,
+    const scoped_refptr<IOBuffer>& data_buffer,
+    size_t size) {
+  if (state_ != CONNECTED) {
+    DVLOG(3) << "Ignored data packet received in state " << state_;
+    return CHANNEL_ALIVE;
+  }
+  expecting_to_read_continuation_ = !final;
+  if (opcode == WebSocketFrameHeader::kOpCodeText ||
+      (opcode == WebSocketFrameHeader::kOpCodeContinuation &&
+       receiving_text_message_)) {
+    // This call is not redundant when size == 0 because it tells us what
+    // the current state is.
+    StreamingUtf8Validator::State state = incoming_utf8_validator_.AddBytes(
+        size ? data_buffer->data() : NULL, size);
+    if (state == StreamingUtf8Validator::INVALID ||
+        (state == StreamingUtf8Validator::VALID_MIDPOINT && final)) {
+      return FailChannel("Could not decode a text frame as UTF-8.",
+                         kWebSocketErrorProtocolError,
+                         "Invalid UTF-8 in text frame");
+    }
+    receiving_text_message_ = !final;
+    DCHECK(!final || state == StreamingUtf8Validator::VALID_ENDPOINT);
+  }
+  // TODO(ricea): Can this copy be eliminated?
+  const char* const data_begin = size ? data_buffer->data() : NULL;
+  const char* const data_end = data_begin + size;
+  const std::vector<char> data(data_begin, data_end);
+  // TODO(ricea): Handle the case when ReadFrames returns far
+  // more data at once than should be sent in a single IPC. This needs to
+  // be handled carefully, as an overloaded IO thread is one possible
+  // cause of receiving very large chunks.
+
+  // Sends the received frame to the renderer process.
+  return event_interface_->OnDataFrame(final, opcode, data);
+}
+
 ChannelState WebSocketChannel::SendIOBuffer(
     bool fin,
     WebSocketFrameHeader::OpCode op_code,