Never send empty non-final messages to Blink.

The IPC message OnDataFrame() in websocket_messages.h documents the
restriction that frames must be either non-empty or final. Up until now,
net::WebSocketChannel has permitted empty opening frames through. Fix
it.

BUG=343060
TEST=net_unittests --gtest_filter=WebSocket*, fragmented-binary-frames layout test

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

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

3 files changed, 43 insertions, 5 deletions

diff --git a/net/websockets/websocket_channel.cc b/net/websockets/websocket_channel.cc
index a960098..84edecf 100644
--- a/net/websockets/websocket_channel.cc
+++ b/net/websockets/websocket_channel.cc
@@ -262,7 +262,8 @@ WebSocketChannel::WebSocketChannel(
       notification_sender_(new HandshakeNotificationSender(this)),
       sending_text_message_(false),
       receiving_text_message_(false),
-      expecting_to_handle_continuation_(false) {}
+      expecting_to_handle_continuation_(false),
+      initial_frame_forwarded_(false) {}
 
 WebSocketChannel::~WebSocketChannel() {
   // The stream may hold a pointer to read_frames_, and so it needs to be
@@ -742,7 +743,7 @@ ChannelState WebSocketChannel::HandleFrameByState(
 }
 
 ChannelState WebSocketChannel::HandleDataFrame(
-    const WebSocketFrameHeader::OpCode opcode,
+    WebSocketFrameHeader::OpCode opcode,
     bool final,
     const scoped_refptr<IOBuffer>& data_buffer,
     size_t size) {
@@ -765,6 +766,13 @@ ChannelState WebSocketChannel::HandleDataFrame(
     return FailChannel(console_log, kWebSocketErrorProtocolError, reason);
   }
   expecting_to_handle_continuation_ = !final;
+  WebSocketFrameHeader::OpCode opcode_to_send = opcode;
+  if (!initial_frame_forwarded_ &&
+      opcode == WebSocketFrameHeader::kOpCodeContinuation) {
+    opcode_to_send = receiving_text_message_
+                         ? WebSocketFrameHeader::kOpCodeText
+                         : WebSocketFrameHeader::kOpCodeBinary;
+  }
   if (opcode == WebSocketFrameHeader::kOpCodeText ||
       (opcode == WebSocketFrameHeader::kOpCodeContinuation &&
        receiving_text_message_)) {
@@ -781,6 +789,10 @@ ChannelState WebSocketChannel::HandleDataFrame(
     receiving_text_message_ = !final;
     DCHECK(!final || state == StreamingUtf8Validator::VALID_ENDPOINT);
   }
+  if (size == 0U && !final)
+    return CHANNEL_ALIVE;
+
+  initial_frame_forwarded_ = !final;
   // 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;
@@ -791,7 +803,7 @@ ChannelState WebSocketChannel::HandleDataFrame(
   // cause of receiving very large chunks.
 
   // Sends the received frame to the renderer process.
-  return event_interface_->OnDataFrame(final, opcode, data);
+  return event_interface_->OnDataFrame(final, opcode_to_send, data);
 }
 
 ChannelState WebSocketChannel::SendIOBuffer(
diff --git a/net/websockets/websocket_channel.h b/net/websockets/websocket_channel.h
index edc8a89..40b76c9 100644
--- a/net/websockets/websocket_channel.h
+++ b/net/websockets/websocket_channel.h
@@ -209,10 +209,10 @@ class NET_EXPORT WebSocketChannel {
   // 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,
+  ChannelState HandleDataFrame(WebSocketFrameHeader::OpCode opcode,
                                bool final,
                                const scoped_refptr<IOBuffer>& data_buffer,
-                               size_t size);
+                               size_t size) WARN_UNUSED_RESULT;
 
   // 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
@@ -336,6 +336,10 @@ class NET_EXPORT WebSocketChannel {
   // True if we are in the middle of receiving a message.
   bool expecting_to_handle_continuation_;
 
+  // True if we have already sent the type (Text or Binary) of the current
+  // message to the renderer. This can be false if the message is empty so far.
+  bool initial_frame_forwarded_;
+
   DISALLOW_COPY_AND_ASSIGN(WebSocketChannel);
 };
 
diff --git a/net/websockets/websocket_channel_test.cc b/net/websockets/websocket_channel_test.cc
index 81c2f29..02d495a 100644
--- a/net/websockets/websocket_channel_test.cc
+++ b/net/websockets/websocket_channel_test.cc
@@ -2892,6 +2892,28 @@ TEST_F(WebSocketChannelEventInterfaceTest, MessageStartingWithContinuation) {
   CreateChannelAndConnectSuccessfully();
 }
 
+// A frame passed to the renderer must be either non-empty or have the final bit
+// set.
+TEST_F(WebSocketChannelEventInterfaceTest, DataFramesNonEmptyOrFinal) {
+  scoped_ptr<ReadableFakeWebSocketStream> stream(
+      new ReadableFakeWebSocketStream);
+  static const InitFrame frames[] = {
+      {NOT_FINAL_FRAME, WebSocketFrameHeader::kOpCodeText, NOT_MASKED, ""},
+      {NOT_FINAL_FRAME, WebSocketFrameHeader::kOpCodeContinuation,
+       NOT_MASKED, ""},
+      {FINAL_FRAME, WebSocketFrameHeader::kOpCodeContinuation, NOT_MASKED, ""}};
+  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(true, WebSocketFrameHeader::kOpCodeText, AsVector("")));
+
+  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