Check for invalid use of data frame opcodes.

Text and Binary opcodes cannot appear in the middle of a message, and
the Continuation opcode cannot appear at the start.

Fail the channel if violations of this rule are found.

BUG=343105
TEST=net_unittests, layout tests

Review URL: https://codereview.chromium.org/157033013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252001 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 112 insertions, 34 deletions

diff --git a/net/websockets/websocket_channel.cc b/net/websockets/websocket_channel.cc
index 1fc6ab3..a960098 100644
--- a/net/websockets/websocket_channel.cc
+++ b/net/websockets/websocket_channel.cc
@@ -261,7 +261,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_handle_continuation_(false) {}
 
 WebSocketChannel::~WebSocketChannel() {
   // The stream may hold a pointer to read_frames_, and so it needs to be
@@ -677,40 +678,9 @@ ChannelState WebSocketChannel::HandleFrameByState(
   }
   switch (opcode) {
     case WebSocketFrameHeader::kOpCodeText:    // fall-thru
-    case WebSocketFrameHeader::kOpCodeBinary:  // fall-thru
+    case WebSocketFrameHeader::kOpCodeBinary:
     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);
-      }
-      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;
@@ -771,6 +741,59 @@ ChannelState WebSocketChannel::HandleFrameByState(
   }
 }
 
+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;
+  }
+  DCHECK(opcode == WebSocketFrameHeader::kOpCodeContinuation ||
+         opcode == WebSocketFrameHeader::kOpCodeText ||
+         opcode == WebSocketFrameHeader::kOpCodeBinary);
+  const bool got_continuation =
+      (opcode == WebSocketFrameHeader::kOpCodeContinuation);
+  if (got_continuation != expecting_to_handle_continuation_) {
+    const std::string console_log = got_continuation
+        ? "Received unexpected continuation frame."
+        : "Received start of new message but previous message is unfinished.";
+    const std::string reason = got_continuation
+        ? "Unexpected continuation"
+        : "Previous data frame unfinished";
+    return FailChannel(console_log, kWebSocketErrorProtocolError, reason);
+  }
+  expecting_to_handle_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,
diff --git a/net/websockets/websocket_channel.h b/net/websockets/websocket_channel.h
index c5c67bf..edc8a89 100644
--- a/net/websockets/websocket_channel.h
+++ b/net/websockets/websocket_channel.h
@@ -206,6 +206,14 @@ class NET_EXPORT WebSocketChannel {
       const scoped_refptr<IOBuffer>& data_buffer,
       size_t size) WARN_UNUSED_RESULT;
 
+  // Forward a received data frame to the renderer, if connected. If
+  // |expecting_continuation| is not equal to |expecting_to_read_continuation_|,
+  // will fail the channel. Also checks the UTF-8 validity of text frames.
+  ChannelState HandleDataFrame(const WebSocketFrameHeader::OpCode opcode,
+                               bool final,
+                               const scoped_refptr<IOBuffer>& data_buffer,
+                               size_t size);
+
   // Low-level method to send a single frame. Used for both data and control
   // frames. Either sends the frame immediately or buffers it to be scheduled
   // when the current write finishes. |fin| and |op_code| are defined as for
@@ -325,6 +333,9 @@ class NET_EXPORT WebSocketChannel {
   base::StreamingUtf8Validator incoming_utf8_validator_;
   bool receiving_text_message_;
 
+  // True if we are in the middle of receiving a message.
+  bool expecting_to_handle_continuation_;
+
   DISALLOW_COPY_AND_ASSIGN(WebSocketChannel);
 };
 
diff --git a/net/websockets/websocket_channel_test.cc b/net/websockets/websocket_channel_test.cc
index dd87b39..81c2f29 100644
--- a/net/websockets/websocket_channel_test.cc
+++ b/net/websockets/websocket_channel_test.cc
@@ -2848,6 +2848,50 @@ TEST_F(WebSocketChannelReceiveUtf8Test, ValidateMultipleReceived) {
   CreateChannelAndConnectSuccessfully();
 }
 
+// A new data message cannot start in the middle of another data message.
+TEST_F(WebSocketChannelEventInterfaceTest, BogusContinuation) {
+  scoped_ptr<ReadableFakeWebSocketStream> stream(
+      new ReadableFakeWebSocketStream);
+  static const InitFrame frames[] = {
+      {NOT_FINAL_FRAME, WebSocketFrameHeader::kOpCodeBinary,
+       NOT_MASKED, "frame1"},
+      {FINAL_FRAME, WebSocketFrameHeader::kOpCodeText,
+       NOT_MASKED, "frame2"}};
+  stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
+  set_stream(stream.Pass());
+
+  EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, _, _));
+  EXPECT_CALL(*event_interface_, OnFlowControl(kDefaultInitialQuota));
+  EXPECT_CALL(
+      *event_interface_,
+      OnDataFrame(
+          false, WebSocketFrameHeader::kOpCodeBinary, AsVector("frame1")));
+  EXPECT_CALL(
+      *event_interface_,
+      OnFailChannel(
+          "Received start of new message but previous message is unfinished."));
+
+  CreateChannelAndConnectSuccessfully();
+}
+
+// A new message cannot start with a Continuation frame.
+TEST_F(WebSocketChannelEventInterfaceTest, MessageStartingWithContinuation) {
+  scoped_ptr<ReadableFakeWebSocketStream> stream(
+      new ReadableFakeWebSocketStream);
+  static const InitFrame frames[] = {
+      {FINAL_FRAME, WebSocketFrameHeader::kOpCodeContinuation,
+       NOT_MASKED, "continuation"}};
+  stream->PrepareReadFrames(ReadableFakeWebSocketStream::SYNC, OK, frames);
+  set_stream(stream.Pass());
+
+  EXPECT_CALL(*event_interface_, OnAddChannelResponse(false, _, _));
+  EXPECT_CALL(*event_interface_, OnFlowControl(kDefaultInitialQuota));
+  EXPECT_CALL(*event_interface_,
+              OnFailChannel("Received unexpected continuation frame."));
+
+  CreateChannelAndConnectSuccessfully();
+}
+
 // If we receive another frame after Close, it is not valid. It is not
 // completely clear what behaviour is required from the standard in this case,
 // but the current implementation fails the connection. Since a Close has