HttpServer WebSocket: don't crash on data before server handshake

net/server/web_socket.cc

Index: net/server/web_socket.cc
diff --git a/net/server/web_socket.cc b/net/server/web_socket.cc
index 79ffcecb7ada589c1c3171c27caedea4226931d4..193281ae2d69743e3f2324d3313e876f66c5335d 100644
--- a/net/server/web_socket.cc
+++ b/net/server/web_socket.cc
@@ -98,6 +98,17 @@ WebSocket::ParseResult WebSocket::Read(std::string* message) {
   if (closed_)
     return FRAME_CLOSE;
 
+  if (!encoder_) {
+    // RFC6455, section 4.1 says "Once the client's opening handshake has been
+    // sent, the client MUST wait for a response from the server before sending
+    // any further data". If encoder_ is null here, we haven't even sent a

[mmenke, 2017/01/23 16:17:21]

nit (x2):  |encoder_| is the more common pattern in net/, for class and stack
variable names.

[Maks Orlovich, 2017/01/23 17:28:29]

Done. I incorrectly assumed this was just for API docs...

+    // server handshake in the first place, so the client clearly didn't do
+    // that, so we can reject them guilt-free, especially since we can't

[mmenke, 2017/01/23 16:17:21]

nit (x2):  Avoid we in comments, due to ambiguity.

[Maks Orlovich, 2017/01/23 17:28:29]

Rephrased.

+    // proceed without an encoder_ anyway.
+    Fail();

[mmenke, 2017/01/23 16:17:21]

Hrm...The caller already closes the socket when we return FRAME_CLOSE or
FRAME_ERROR.  There's no documented API contract here, but digging into the
encoder_->DecodeFrame call below, it looks like on errors it does not close the
socket, but instead just returns FRAME_ERROR, so I think it's more consistent
not to call Fail() here.

[Maks Orlovich, 2017/01/23 17:28:29]

Good point, done. This does mean we don't set closed_, but since the connection
is closed no one should be able to call into something like WebSocket::Send()
anyway.

+    return FRAME_CLOSE;

[mmenke, 2017/01/23 16:17:21]

FRAME_ERROR seems more accurate.

[Maks Orlovich, 2017/01/23 17:28:29]

Done.

+  }
+
   HttpConnection::ReadIOBuffer* read_buf = connection_->read_buf();
   base::StringPiece frame(read_buf->StartOfBuffer(), read_buf->GetSize());
   int bytes_consumed = 0;