Add a test for Pong frames with NULL data and fix.

Previously, passing a frame with NULL data to
WebSocketBasicStream::WriteFrames() could cause a crash.

This is a particular problem because WebSocketFrameParser creates frames with
NULL data when there is no payload. This is particularly common with Ping
frames. WebSocketChannel copies the original payload from a Ping message to the
Pong message, so if it is NULL it stays NULL.

Stop attempting to access the data for zero-size payload messages in
WebSocketBasicStream.

It appears that the tests for WebSocketDeflateStream already use NULL for empty
payloads and it is not vulnerable to this problem.

BUG=337868
TEST=net_unittests --gtest_filter=WebSocket*

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

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

3 files changed, 30 insertions, 9 deletions

diff --git a/net/websockets/websocket_basic_stream.cc b/net/websockets/websocket_basic_stream.cc
index ea26fd8..8d1fa87 100644
--- a/net/websockets/websocket_basic_stream.cc
+++ b/net/websockets/websocket_basic_stream.cc
@@ -157,13 +157,15 @@ int WebSocketBasicStream::WriteFrames(ScopedVector<WebSocketFrame>* frames,
     dest += result;
     remaining_size -= result;
 
-    const char* const frame_data = frame->data->data();
     const int frame_size = frame->header.payload_length;
-    CHECK_GE(remaining_size, frame_size);
-    std::copy(frame_data, frame_data + frame_size, dest);
-    MaskWebSocketFramePayload(mask, 0, dest, frame_size);
-    dest += frame_size;
-    remaining_size -= frame_size;
+    if (frame_size > 0) {
+      CHECK_GE(remaining_size, frame_size);
+      const char* const frame_data = frame->data->data();
+      std::copy(frame_data, frame_data + frame_size, dest);
+      MaskWebSocketFramePayload(mask, 0, dest, frame_size);
+      dest += frame_size;
+      remaining_size -= frame_size;
+    }
   }
   DCHECK_EQ(0, remaining_size) << "Buffer size calculation was wrong; "
                                << remaining_size << " bytes left over.";
diff --git a/net/websockets/websocket_basic_stream_test.cc b/net/websockets/websocket_basic_stream_test.cc
index cb936a5..85f6156 100644
--- a/net/websockets/websocket_basic_stream_test.cc
+++ b/net/websockets/websocket_basic_stream_test.cc
@@ -57,6 +57,8 @@ WEBSOCKET_BASIC_STREAM_TEST_DEFINE_CONSTANT(CloseFrame,
                                             "\x88\x09\x03\xe8occludo");
 WEBSOCKET_BASIC_STREAM_TEST_DEFINE_CONSTANT(WriteFrame,
                                             "\x81\x85\x00\x00\x00\x00Write");
+WEBSOCKET_BASIC_STREAM_TEST_DEFINE_CONSTANT(MaskedEmptyPong,
+                                            "\x8A\x80\x00\x00\x00\x00");
 const WebSocketMaskingKey kNulMaskingKey = {{'\0', '\0', '\0', '\0'}};
 const WebSocketMaskingKey kNonNulMaskingKey = {
     {'\x0d', '\x1b', '\x06', '\x17'}};
@@ -856,6 +858,23 @@ TEST_F(WebSocketBasicStreamSocketWriteTest, WriteInBits) {
   EXPECT_EQ(OK, cb_.WaitForResult());
 }
 
+// Check that writing a Pong frame with a NULL body works.
+TEST_F(WebSocketBasicStreamSocketWriteTest, WriteNullPong) {
+  MockWrite writes[] = {
+      MockWrite(SYNCHRONOUS, kMaskedEmptyPong, kMaskedEmptyPongSize)};
+  CreateWriteOnly(writes);
+
+  scoped_ptr<WebSocketFrame> frame(
+      new WebSocketFrame(WebSocketFrameHeader::kOpCodePong));
+  WebSocketFrameHeader& header = frame->header;
+  header.final = true;
+  header.masked = true;
+  header.payload_length = 0;
+  ScopedVector<WebSocketFrame> frames;
+  frames.push_back(frame.release());
+  EXPECT_EQ(OK, stream_->WriteFrames(&frames, cb_.callback()));
+}
+
 // Check that writing with a non-NULL mask works correctly.
 TEST_F(WebSocketBasicStreamSocketTest, WriteNonNulMask) {
   std::string masked_frame = std::string("\x81\x88");
diff --git a/net/websockets/websocket_channel_test.cc b/net/websockets/websocket_channel_test.cc
index f470899c6..5b09fe1 100644
--- a/net/websockets/websocket_channel_test.cc
+++ b/net/websockets/websocket_channel_test.cc
@@ -2291,13 +2291,13 @@ TEST_F(WebSocketChannelStreamTest, PingRepliedWithPong) {
   CreateChannelAndConnectSuccessfully();
 }
 
-// A ping with a NULL payload should be responded to with a Pong with an empty
+// A ping with a NULL payload should be responded to with a Pong with a NULL
 // payload.
-TEST_F(WebSocketChannelStreamTest, NullPingRepliedWithEmptyPong) {
+TEST_F(WebSocketChannelStreamTest, NullPingRepliedWithNullPong) {
   static const InitFrame frames[] = {
       {FINAL_FRAME, WebSocketFrameHeader::kOpCodePing, NOT_MASKED, NULL}};
   static const InitFrame expected[] = {
-      {FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, MASKED, ""}};
+      {FINAL_FRAME, WebSocketFrameHeader::kOpCodePong, MASKED, NULL}};
   EXPECT_CALL(*mock_stream_, GetSubProtocol()).Times(AnyNumber());
   EXPECT_CALL(*mock_stream_, ReadFrames(_, _))
       .WillOnce(ReturnFrames(&frames))