Add invalid data send check on CONNECTING state

Handshake data should be sent at once. WebSocketJob should reject any
other subsequent data sending until the handshake is done and connection
is opened.

BUG=84422
TEST=net_unittest --gtest_filter=WebSocket\*


Review URL: http://codereview.chromium.org/7075027

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

3 files changed, 130 insertions, 168 deletions

diff --git a/net/websockets/websocket_job.cc b/net/websockets/websocket_job.cc
index 10b351a..fc67f9a 100644
--- a/net/websockets/websocket_job.cc
+++ b/net/websockets/websocket_job.cc
@@ -72,6 +72,7 @@ WebSocketJob::WebSocketJob(SocketStream::Delegate* delegate)
       callback_(NULL),
       handshake_request_(new WebSocketHandshakeRequestHandler),
       handshake_response_(new WebSocketHandshakeResponseHandler),
+      started_to_send_handshake_request_(false),
       handshake_request_sent_(0),
       response_cookies_save_index_(0),
       send_frame_handler_(new WebSocketFrameHandler),
@@ -273,6 +274,8 @@ void WebSocketJob::OnError(const SocketStream* socket, int error) {
 
 bool WebSocketJob::SendHandshakeRequest(const char* data, int len) {
   DCHECK_EQ(state_, CONNECTING);
+  if (started_to_send_handshake_request_)
+    return false;
   if (!handshake_request_->ParseRequest(data, len))
     return false;
 
@@ -281,6 +284,7 @@ bool WebSocketJob::SendHandshakeRequest(const char* data, int len) {
       handshake_request_->protocol_version());
   AddCookieHeaderAndSend();
   // Just buffered in |handshake_request_|.
+  started_to_send_handshake_request_ = true;
   return true;
 }
 
diff --git a/net/websockets/websocket_job.h b/net/websockets/websocket_job.h
index 1584fd6..b554469 100644
--- a/net/websockets/websocket_job.h
+++ b/net/websockets/websocket_job.h
@@ -109,6 +109,7 @@ class NET_API WebSocketJob
   scoped_ptr<WebSocketHandshakeRequestHandler> handshake_request_;
   scoped_ptr<WebSocketHandshakeResponseHandler> handshake_response_;
 
+  bool started_to_send_handshake_request_;
   size_t handshake_request_sent_;
 
   std::vector<std::string> response_cookies_;
diff --git a/net/websockets/websocket_job_unittest.cc b/net/websockets/websocket_job_unittest.cc
index 6643517..8951a4e 100644
--- a/net/websockets/websocket_job_unittest.cc
+++ b/net/websockets/websocket_job_unittest.cc
@@ -210,49 +210,93 @@ class WebSocketJobTest : public PlatformTest {
   scoped_refptr<MockURLRequestContext> context_;
   scoped_refptr<WebSocketJob> websocket_;
   scoped_refptr<MockSocketStream> socket_;
+
+  static const char* kHandshakeRequestWithoutCookie;
+  static const char* kHandshakeRequestWithCookie;
+  static const char* kHandshakeRequestWithFilteredCookie;
+  static const char* kHandshakeResponseWithoutCookie;
+  static const char* kHandshakeResponseWithCookie;
 };
 
+const char* WebSocketJobTest::kHandshakeRequestWithoutCookie =
+    "GET /demo HTTP/1.1\r\n"
+    "Host: example.com\r\n"
+    "Connection: Upgrade\r\n"
+    "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
+    "Sec-WebSocket-Protocol: sample\r\n"
+    "Upgrade: WebSocket\r\n"
+    "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
+    "Origin: http://example.com\r\n"
+    "\r\n"
+    "^n:ds[4U";
+
+const char* WebSocketJobTest::kHandshakeRequestWithCookie =
+    "GET /demo HTTP/1.1\r\n"
+    "Host: example.com\r\n"
+    "Connection: Upgrade\r\n"
+    "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
+    "Sec-WebSocket-Protocol: sample\r\n"
+    "Upgrade: WebSocket\r\n"
+    "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
+    "Origin: http://example.com\r\n"
+    "Cookie: WK-test=1\r\n"
+    "\r\n"
+    "^n:ds[4U";
+
+const char* WebSocketJobTest::kHandshakeRequestWithFilteredCookie =
+    "GET /demo HTTP/1.1\r\n"
+    "Host: example.com\r\n"
+    "Connection: Upgrade\r\n"
+    "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
+    "Sec-WebSocket-Protocol: sample\r\n"
+    "Upgrade: WebSocket\r\n"
+    "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
+    "Origin: http://example.com\r\n"
+    "Cookie: CR-test=1; CR-test-httponly=1\r\n"
+    "\r\n"
+    "^n:ds[4U";
+
+const char* WebSocketJobTest::kHandshakeResponseWithoutCookie =
+    "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
+    "Upgrade: WebSocket\r\n"
+    "Connection: Upgrade\r\n"
+    "Sec-WebSocket-Origin: http://example.com\r\n"
+    "Sec-WebSocket-Location: ws://example.com/demo\r\n"
+    "Sec-WebSocket-Protocol: sample\r\n"
+    "\r\n"
+    "8jKS'y:G*Co,Wxa-";
+
+const char* WebSocketJobTest::kHandshakeResponseWithCookie =
+    "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
+    "Upgrade: WebSocket\r\n"
+    "Connection: Upgrade\r\n"
+    "Sec-WebSocket-Origin: http://example.com\r\n"
+    "Sec-WebSocket-Location: ws://example.com/demo\r\n"
+    "Sec-WebSocket-Protocol: sample\r\n"
+    "Set-Cookie: CR-set-test=1\r\n"
+    "\r\n"
+    "8jKS'y:G*Co,Wxa-";
+
 TEST_F(WebSocketJobTest, SimpleHandshake) {
   GURL url("ws://example.com/demo");
   MockSocketStreamDelegate delegate;
   InitWebSocketJob(url, &delegate);
 
-  static const char* kHandshakeRequestMessage =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  bool sent = websocket_->SendData(kHandshakeRequestMessage,
-                                   strlen(kHandshakeRequestMessage));
+  bool sent = websocket_->SendData(kHandshakeRequestWithoutCookie,
+                                   strlen(kHandshakeRequestWithoutCookie));
   EXPECT_TRUE(sent);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeRequestMessage, socket_->sent_data());
+  EXPECT_EQ(kHandshakeRequestWithoutCookie, socket_->sent_data());
   EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
-  websocket_->OnSentData(socket_.get(), strlen(kHandshakeRequestMessage));
-  EXPECT_EQ(strlen(kHandshakeRequestMessage), delegate.amount_sent());
-
-  const char kHandshakeResponseMessage[] =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
+  websocket_->OnSentData(socket_.get(),
+                         strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_EQ(strlen(kHandshakeRequestWithoutCookie), delegate.amount_sent());
 
   websocket_->OnReceivedData(socket_.get(),
-                             kHandshakeResponseMessage,
-                             strlen(kHandshakeResponseMessage));
+                             kHandshakeResponseWithoutCookie,
+                             strlen(kHandshakeResponseWithoutCookie));
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeResponseMessage, delegate.received_data());
+  EXPECT_EQ(kHandshakeResponseWithoutCookie, delegate.received_data());
   EXPECT_EQ(WebSocketJob::OPEN, GetWebSocketJobState());
   CloseWebSocketJob();
 }
@@ -262,41 +306,20 @@ TEST_F(WebSocketJobTest, SlowHandshake) {
   MockSocketStreamDelegate delegate;
   InitWebSocketJob(url, &delegate);
 
-  static const char* kHandshakeRequestMessage =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  bool sent = websocket_->SendData(kHandshakeRequestMessage,
-                                   strlen(kHandshakeRequestMessage));
+  bool sent = websocket_->SendData(kHandshakeRequestWithoutCookie,
+                                   strlen(kHandshakeRequestWithoutCookie));
   EXPECT_TRUE(sent);
   // We assume request is sent in one data chunk (from WebKit)
   // We don't support streaming request.
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeRequestMessage, socket_->sent_data());
+  EXPECT_EQ(kHandshakeRequestWithoutCookie, socket_->sent_data());
   EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
-  websocket_->OnSentData(socket_.get(), strlen(kHandshakeRequestMessage));
-  EXPECT_EQ(strlen(kHandshakeRequestMessage), delegate.amount_sent());
-
-  const char kHandshakeResponseMessage[] =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
+  websocket_->OnSentData(socket_.get(),
+                         strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_EQ(strlen(kHandshakeRequestWithoutCookie), delegate.amount_sent());
 
   std::vector<std::string> lines;
-  base::SplitString(kHandshakeResponseMessage, '\n', &lines);
+  base::SplitString(kHandshakeResponseWithoutCookie, '\n', &lines);
   for (size_t i = 0; i < lines.size() - 2; i++) {
     std::string line = lines[i] + "\r\n";
     SCOPED_TRACE("Line: " + line);
@@ -312,7 +335,7 @@ TEST_F(WebSocketJobTest, SlowHandshake) {
   EXPECT_TRUE(delegate.received_data().empty());
   EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
   websocket_->OnReceivedData(socket_.get(), "8jKS'y:G*Co,Wxa-", 16);
-  EXPECT_EQ(kHandshakeResponseMessage, delegate.received_data());
+  EXPECT_EQ(kHandshakeResponseWithoutCookie, delegate.received_data());
   EXPECT_EQ(WebSocketJob::OPEN, GetWebSocketJobState());
   CloseWebSocketJob();
 }
@@ -330,67 +353,21 @@ TEST_F(WebSocketJobTest, HandshakeWithCookie) {
   MockSocketStreamDelegate delegate;
   InitWebSocketJob(url, &delegate);
 
-  static const char* kHandshakeRequestMessage =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "Cookie: WK-test=1\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  static const char* kHandshakeRequestExpected =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "Cookie: CR-test=1; CR-test-httponly=1\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  bool sent = websocket_->SendData(kHandshakeRequestMessage,
-                                   strlen(kHandshakeRequestMessage));
+  bool sent = websocket_->SendData(kHandshakeRequestWithCookie,
+                                   strlen(kHandshakeRequestWithCookie));
   EXPECT_TRUE(sent);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeRequestExpected, socket_->sent_data());
+  EXPECT_EQ(kHandshakeRequestWithFilteredCookie, socket_->sent_data());
   EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
-  websocket_->OnSentData(socket_, strlen(kHandshakeRequestExpected));
-  EXPECT_EQ(strlen(kHandshakeRequestMessage), delegate.amount_sent());
-
-  const char kHandshakeResponseMessage[] =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Set-Cookie: CR-set-test=1\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
-
-  static const char* kHandshakeResponseExpected =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
+  websocket_->OnSentData(socket_, strlen(kHandshakeRequestWithFilteredCookie));
+  EXPECT_EQ(strlen(kHandshakeRequestWithCookie),
+            delegate.amount_sent());
 
   websocket_->OnReceivedData(socket_.get(),
-                             kHandshakeResponseMessage,
-                             strlen(kHandshakeResponseMessage));
+                             kHandshakeResponseWithCookie,
+                             strlen(kHandshakeResponseWithCookie));
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeResponseExpected, delegate.received_data());
+  EXPECT_EQ(kHandshakeResponseWithoutCookie, delegate.received_data());
   EXPECT_EQ(WebSocketJob::OPEN, GetWebSocketJobState());
 
   EXPECT_EQ(3U, cookie_store_->entries().size());
@@ -418,66 +395,21 @@ TEST_F(WebSocketJobTest, HandshakeWithCookieButNotAllowed) {
   delegate.set_allow_all_cookies(false);
   InitWebSocketJob(url, &delegate);
 
-  static const char* kHandshakeRequestMessage =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "Cookie: WK-test=1\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  static const char* kHandshakeRequestExpected =
-      "GET /demo HTTP/1.1\r\n"
-      "Host: example.com\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Key2: 12998 5 Y3 1  .P00\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Sec-WebSocket-Key1: 4 @1  46546xW%0l 1 5\r\n"
-      "Origin: http://example.com\r\n"
-      "\r\n"
-      "^n:ds[4U";
-
-  bool sent = websocket_->SendData(kHandshakeRequestMessage,
-                                   strlen(kHandshakeRequestMessage));
+  bool sent = websocket_->SendData(kHandshakeRequestWithCookie,
+                                   strlen(kHandshakeRequestWithCookie));
   EXPECT_TRUE(sent);
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeRequestExpected, socket_->sent_data());
+  EXPECT_EQ(kHandshakeRequestWithoutCookie, socket_->sent_data());
   EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
-  websocket_->OnSentData(socket_, strlen(kHandshakeRequestExpected));
-  EXPECT_EQ(strlen(kHandshakeRequestMessage), delegate.amount_sent());
-
-  const char kHandshakeResponseMessage[] =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "Set-Cookie: CR-set-test=1\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
-
-  static const char* kHandshakeResponseExpected =
-      "HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
-      "Upgrade: WebSocket\r\n"
-      "Connection: Upgrade\r\n"
-      "Sec-WebSocket-Origin: http://example.com\r\n"
-      "Sec-WebSocket-Location: ws://example.com/demo\r\n"
-      "Sec-WebSocket-Protocol: sample\r\n"
-      "\r\n"
-      "8jKS'y:G*Co,Wxa-";
+  websocket_->OnSentData(socket_, strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_EQ(strlen(kHandshakeRequestWithCookie),
+            delegate.amount_sent());
 
   websocket_->OnReceivedData(socket_.get(),
-                             kHandshakeResponseMessage,
-                             strlen(kHandshakeResponseMessage));
+                             kHandshakeResponseWithCookie,
+                             strlen(kHandshakeResponseWithCookie));
   MessageLoop::current()->RunAllPending();
-  EXPECT_EQ(kHandshakeResponseExpected, delegate.received_data());
+  EXPECT_EQ(kHandshakeResponseWithoutCookie, delegate.received_data());
   EXPECT_EQ(WebSocketJob::OPEN, GetWebSocketJobState());
 
   EXPECT_EQ(2U, cookie_store_->entries().size());
@@ -506,4 +438,29 @@ TEST_F(WebSocketJobTest, HSTSUpgrade) {
   job->DetachDelegate();
 }
 
+TEST_F(WebSocketJobTest, InvalidSendData) {
+  GURL url("ws://example.com/demo");
+  MockSocketStreamDelegate delegate;
+  InitWebSocketJob(url, &delegate);
+
+  bool sent = websocket_->SendData(kHandshakeRequestWithoutCookie,
+                                   strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_TRUE(sent);
+  // We assume request is sent in one data chunk (from WebKit)
+  // We don't support streaming request.
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(kHandshakeRequestWithoutCookie, socket_->sent_data());
+  EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
+  websocket_->OnSentData(socket_.get(),
+                         strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_EQ(strlen(kHandshakeRequestWithoutCookie), delegate.amount_sent());
+
+  // We could not send any data until connection is established.
+  sent = websocket_->SendData(kHandshakeRequestWithoutCookie,
+                              strlen(kHandshakeRequestWithoutCookie));
+  EXPECT_FALSE(sent);
+  EXPECT_EQ(WebSocketJob::CONNECTING, GetWebSocketJobState());
+  CloseWebSocketJob();
+}
+
 }  // namespace net