[WebSocket] Incoming close frame should not overtake data frames

net/websockets/websocket_channel.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/websockets/websocket_channel.h"
 
 #include <stddef.h>
 #include <limits.h>  // for INT_MAX
 
 #include <algorithm>
@@ skipping 408 matching lines @@
   // TODO(ricea): If current_send_quota_ has dropped below
   // send_quota_low_water_mark_, it might be good to increase the "low
   // water mark" and "high water mark", but only if the link to the WebSocket
   // server is not saturated.
   scoped_refptr<IOBuffer> buffer(new IOBuffer(data.size()));
   std::copy(data.begin(), data.end(), buffer->data());
   return SendFrameFromIOBuffer(fin, op_code, buffer, data.size());
   // |this| may have been deleted.
 }
 
-void WebSocketChannel::SendFlowControl(int64_t quota) {
+ChannelState WebSocketChannel::SendFlowControl(int64_t quota) {
   DCHECK(state_ == CONNECTING || state_ == CONNECTED || state_ == SEND_CLOSED ||
          state_ == CLOSE_WAIT);
   // TODO(ricea): Kill the renderer if it tries to send us a negative quota
   // value or > INT_MAX.
   DCHECK_GE(quota, 0);
   DCHECK_LE(quota, INT_MAX);
   if (!pending_received_frames_.empty()) {
     DCHECK_EQ(0u, current_receive_quota_);
   }
   while (!pending_received_frames_.empty() && quota > 0) {
     PendingReceivedFrame& front = pending_received_frames_.front();
     const uint64_t data_size = front.size() - front.offset();
     const uint64_t bytes_to_send =
         std::min(base::checked_cast<uint64_t>(quota), data_size);
     const bool final = front.final() && data_size == bytes_to_send;
     const char* data =
         front.data().get() ? front.data()->data() + front.offset() : NULL;
     DCHECK(!bytes_to_send || data) << "Non empty data should not be null.";
     const std::vector<char> data_vector(data, data + bytes_to_send);
     DVLOG(3) << "Sending frame previously split due to quota to the "
              << "renderer: quota=" << quota << " data_size=" << data_size
              << " bytes_to_send=" << bytes_to_send;
     if (event_interface_->OnDataFrame(final, front.opcode(), data_vector) ==
         CHANNEL_DELETED)
-      return;
+      return CHANNEL_DELETED;
     if (bytes_to_send < data_size) {
       front.DidConsume(bytes_to_send);
       front.ResetOpcode();
-      return;
+      return CHANNEL_ALIVE;
     }
     quota -= bytes_to_send;
 
     pending_received_frames_.pop();
   }
+  if (pending_received_frames_.empty() && has_received_close_frame_) {
+    // We've been waiting for the client to consume the frames before
+    // responding to the closing handshake initiated by the server.
+    return RespondToClosingHandshake();
+  }
+
   // If current_receive_quota_ == 0 then there is no pending ReadFrames()
   // operation.
   const bool start_read =
       current_receive_quota_ == 0 && quota > 0 &&
       (state_ == CONNECTED || state_ == SEND_CLOSED || state_ == CLOSE_WAIT);
   current_receive_quota_ += quota;
   if (start_read)
-    ignore_result(ReadFrames());
-  // |this| may have been deleted.
+    return ReadFrames();
+  return CHANNEL_ALIVE;
 }
 
-void WebSocketChannel::StartClosingHandshake(uint16_t code,
-                                             const std::string& reason) {
+ChannelState WebSocketChannel::StartClosingHandshake(
+    uint16_t code,
+    const std::string& reason) {
   if (InClosingState()) {
     // When the associated renderer process is killed while the channel is in
     // CLOSING state we reach here.
     DVLOG(1) << "StartClosingHandshake called in state " << state_
              << ". This may be a bug, or a harmless race.";
-    return;
+    return CHANNEL_ALIVE;
+  }
+  if (has_received_close_frame_) {
+    // We reach here if the client wants to start a closing handshake while
+    // the browser is waiting for the client to consume incoming data frames
+    // before responding to a closing handshake initiated by the server.
+    // As the client doesn't want the data frames any more, we can respond to
+    // the closing handshake initiated by the server.
+    return RespondToClosingHandshake();
   }
   if (state_ == CONNECTING) {
     // Abort the in-progress handshake and drop the connection immediately.
     stream_request_.reset();
     SetState(CLOSED);
-    DoDropChannel(false, kWebSocketErrorAbnormalClosure, "");
-    return;
+    return DoDropChannel(false, kWebSocketErrorAbnormalClosure, "");
   }
   if (state_ != CONNECTED) {
     NOTREACHED() << "StartClosingHandshake() called in state " << state_;
-    return;
+    return CHANNEL_ALIVE;
   }
 
   DCHECK(!close_timer_.IsRunning());
   // This use of base::Unretained() is safe because we stop the timer in the
   // destructor.
   close_timer_.Start(
       FROM_HERE,
       closing_handshake_timeout_,
       base::Bind(&WebSocketChannel::CloseTimeout, base::Unretained(this)));
 
   // Javascript actually only permits 1000 and 3000-4999, but the implementation
   // itself may produce different codes. The length of |reason| is also checked
   // by Javascript.
   if (!IsStrictlyValidCloseStatusCode(code) ||
       reason.size() > kMaximumCloseReasonLength) {
     // "InternalServerError" is actually used for errors from any endpoint, per
     // errata 3227 to RFC6455. If the renderer is sending us an invalid code or
     // reason it must be malfunctioning in some way, and based on that we
     // interpret this as an internal error.
-    if (SendClose(kWebSocketErrorInternalServerError, "") != CHANNEL_DELETED) {
-      DCHECK_EQ(CONNECTED, state_);
-      SetState(SEND_CLOSED);
-    }
-    return;
+    if (SendClose(kWebSocketErrorInternalServerError, "") == CHANNEL_DELETED)
+      return CHANNEL_DELETED;
+    DCHECK_EQ(CONNECTED, state_);
+    SetState(SEND_CLOSED);
+    return CHANNEL_ALIVE;
   }
   if (SendClose(
           code,
           StreamingUtf8Validator::Validate(reason) ? reason : std::string()) ==
       CHANNEL_DELETED)
-    return;
+    return CHANNEL_DELETED;
   DCHECK_EQ(CONNECTED, state_);
   SetState(SEND_CLOSED);
+  return CHANNEL_ALIVE;
 }
 
 void WebSocketChannel::SendAddChannelRequestForTesting(
     const GURL& socket_url,
     const std::vector<std::string>& requested_subprotocols,
     const url::Origin& origin,
     const WebSocketStreamCreator& creator) {
   SendAddChannelRequestWithSuppliedCreator(
       socket_url, requested_subprotocols, origin, creator);
 }
@@ skipping 302 matching lines @@
             true, WebSocketFrameHeader::kOpCodePong, data_buffer, size);
       DVLOG(3) << "Ignored ping in state " << state_;
       return CHANNEL_ALIVE;
 
     case WebSocketFrameHeader::kOpCodePong:
       DVLOG(1) << "Got Pong of size " << size;
       // There is no need to do anything with pong messages.
       return CHANNEL_ALIVE;
 
     case WebSocketFrameHeader::kOpCodeClose: {
-      // TODO(ricea): If there is a message which is queued for transmission to
-      // the renderer, then the renderer should not receive an
-      // OnClosingHandshake or OnDropChannel IPC until the queued message has
-      // been completedly transmitted.
       uint16_t code = kWebSocketNormalClosure;
       std::string reason;
       std::string message;
       if (!ParseClose(data_buffer, size, &code, &reason, &message)) {
         return FailChannel(message, code, reason);
       }
       // TODO(ricea): Find a way to safely log the message from the close
       // message (escape control codes and so on).
       DVLOG(1) << "Got Close with code " << code;
-      switch (state_) {
-        case CONNECTED:
-          SetState(RECV_CLOSED);
-
-          if (SendClose(code, reason) == CHANNEL_DELETED)
-            return CHANNEL_DELETED;
-          DCHECK_EQ(RECV_CLOSED, state_);
-
-          SetState(CLOSE_WAIT);
-          DCHECK(!close_timer_.IsRunning());
-          // This use of base::Unretained() is safe because we stop the timer
-          // in the destructor.
-          close_timer_.Start(
-              FROM_HERE,
-              underlying_connection_close_timeout_,
-              base::Bind(
-                  &WebSocketChannel::CloseTimeout, base::Unretained(this)));
-
-          if (event_interface_->OnClosingHandshake() == CHANNEL_DELETED)
-            return CHANNEL_DELETED;
-          has_received_close_frame_  = true;
-          received_close_code_ = code;
-          received_close_reason_ = reason;
-          break;
-
-        case SEND_CLOSED:
-          SetState(CLOSE_WAIT);
-          DCHECK(close_timer_.IsRunning());
-          close_timer_.Stop();
-          // This use of base::Unretained() is safe because we stop the timer
-          // in the destructor.
-          close_timer_.Start(
-              FROM_HERE,
-              underlying_connection_close_timeout_,
-              base::Bind(
-                  &WebSocketChannel::CloseTimeout, base::Unretained(this)));
-
-          // From RFC6455 section 7.1.5: "Each endpoint
-          // will see the status code sent by the other end as _The WebSocket
-          // Connection Close Code_."
-          has_received_close_frame_  = true;
-          received_close_code_ = code;
-          received_close_reason_ = reason;
-          break;
-
-        default:
-          LOG(DFATAL) << "Got Close in unexpected state " << state_;
-          break;
-      }
-      return CHANNEL_ALIVE;
+      return HandleCloseFrame(code, reason);
     }
 
     default:
       return FailChannel(
           base::StringPrintf("Unrecognized frame opcode: %d", opcode),
           kWebSocketErrorProtocolError,
           "Unknown opcode");
   }
 }
 
 ChannelState WebSocketChannel::HandleDataFrame(
     WebSocketFrameHeader::OpCode opcode,
     bool final,
     const scoped_refptr<IOBuffer>& data_buffer,
     uint64_t size) {
   if (state_ != CONNECTED) {
     DVLOG(3) << "Ignored data packet received in state " << state_;
     return CHANNEL_ALIVE;
   }
+  if (has_received_close_frame_) {
+    DVLOG(3) << "Ignored data packet as we've received a close frame.";
+    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
@@ skipping 47 matching lines @@
   // 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);
   current_receive_quota_ -= size;
 
   // Sends the received frame to the renderer process.
   return event_interface_->OnDataFrame(final, opcode_to_send, data);
 }
 
+ChannelState WebSocketChannel::HandleCloseFrame(uint16_t code,
+                                                const std::string& reason) {
+  DVLOG(1) << "Got Close with code " << code;
+  switch (state_) {
+    case CONNECTED:
+      has_received_close_frame_ = true;
+      received_close_code_ = code;
+      received_close_reason_ = reason;
+      if (!pending_received_frames_.empty()) {
+        // We have some data to be sent to the renderer before sending this
+        // frame.
+        return CHANNEL_ALIVE;
+      }
+      return RespondToClosingHandshake();
+
+    case SEND_CLOSED:
+      SetState(CLOSE_WAIT);
+      DCHECK(close_timer_.IsRunning());
+      close_timer_.Stop();
+      // This use of base::Unretained() is safe because we stop the timer
+      // in the destructor.
+      close_timer_.Start(
+          FROM_HERE, underlying_connection_close_timeout_,
+          base::Bind(&WebSocketChannel::CloseTimeout, base::Unretained(this)));
+
+      // From RFC6455 section 7.1.5: "Each endpoint
+      // will see the status code sent by the other end as _The WebSocket
+      // Connection Close Code_."
+      has_received_close_frame_ = true;
+      received_close_code_ = code;
+      received_close_reason_ = reason;
+      break;
+
+    default:
+      LOG(DFATAL) << "Got Close in unexpected state " << state_;
+      break;
+  }
+  return CHANNEL_ALIVE;
+}
+
+ChannelState WebSocketChannel::RespondToClosingHandshake() {
+  DCHECK(has_received_close_frame_);
+  DCHECK_EQ(CONNECTED, state_);
+  SetState(RECV_CLOSED);
+  if (SendClose(received_close_code_, received_close_reason_) ==
+      CHANNEL_DELETED)
+    return CHANNEL_DELETED;
+  DCHECK_EQ(RECV_CLOSED, state_);
+
+  SetState(CLOSE_WAIT);
+  DCHECK(!close_timer_.IsRunning());
+  // This use of base::Unretained() is safe because we stop the timer
+  // in the destructor.
+  close_timer_.Start(
+      FROM_HERE, underlying_connection_close_timeout_,
+      base::Bind(&WebSocketChannel::CloseTimeout, base::Unretained(this)));
+
+  return event_interface_->OnClosingHandshake();
+}
+
 ChannelState WebSocketChannel::SendFrameFromIOBuffer(
     bool fin,
     WebSocketFrameHeader::OpCode op_code,
     const scoped_refptr<IOBuffer>& buffer,
     uint64_t size) {
   DCHECK(state_ == CONNECTED || state_ == RECV_CLOSED);
   DCHECK(stream_);
 
   scoped_ptr<WebSocketFrame> frame(new WebSocketFrame(op_code));
   WebSocketFrameHeader& header = frame->header;
@@ skipping 135 matching lines @@
 }
 
 void WebSocketChannel::CloseTimeout() {
   stream_->Close();
   SetState(CLOSED);
   DoDropChannel(false, kWebSocketErrorAbnormalClosure, "");
   // |this| has been deleted.
 }
 
 }  // namespace net