Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(208)

Issue 8370035: Upgrade DevTools WebSocket server implementation from Hybi10 to Hybi17. (Closed)

Created:
9 years, 2 months ago by loislo
Modified:
9 years, 2 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Upgrade DevTools WebSocket server implementation from Hybi10 to Hybi17. Hybi-10 (corresponds to Sec-WebSocket-Version: 8) -- "Frames sent from the server to the client are not masked." Hybi-17 (corresponds to Sec-WebSocket-Version: 13) -- "A server MUST NOT mask any frames that it sends to the client. A client MUST close a connection if it detects a masked frame." BUG=101340 TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106918

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -23 lines) Patch
M net/server/web_socket.cc View 9 chunks +14 lines, -23 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
loislo
9 years, 2 months ago (2011-10-24 10:30:14 UTC) #1
yurys
lgtm
9 years, 2 months ago (2011-10-24 12:55:35 UTC) #2
willchan no longer on Chromium
9 years, 2 months ago (2011-10-24 15:38:04 UTC) #3
Rubberstamp lgtm

Sent from my iNexus.
On Oct 24, 2011 3:30 AM, <loislo@chromium.org> wrote:

> Reviewers: pfeldman, Yury Semikhatsky, willchan,
>
> Description:
> Upgrade DevTools WebSocket server implementation from Hybi10 to Hybi17.
>
> Hybi-10 (corresponds to Sec-WebSocket-Version: 8) -- "Frames sent from
> the server to the client are not masked."
>
> Hybi-17 (corresponds to Sec-WebSocket-Version: 13) --
>  "A server MUST NOT mask any frames that it sends to
>  the client.  A client MUST close a connection if it detects a masked
>  frame."
>
> BUG=101340
> TEST=none
>
>
> Please review this at
http://codereview.chromium.**org/8370035/<http://codereview.chromium.org/8370...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M 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 2f5ad5fa9d37d32e80dee12b09a857**3eaecf20a5..**
> c2b6098b490173f870b5c4d5b4efbe**7f02cb86b4 100644
> --- a/net/server/web_socket.cc
> +++ b/net/server/web_socket.cc
> @@ -169,7 +169,7 @@ const size_t kTwoBytePayloadLengthField = 126;
>  const size_t kEightBytePayloadLengthField = 127;
>  const size_t kMaskingKeyWidthInBytes = 4;
>
> -class WebSocketHybi10 : public WebSocket {
> +class WebSocketHybi17 : public WebSocket {
>  public:
>   static WebSocket* Create(HttpConnection* connection,
>                            const HttpServerRequestInfo& request,
> @@ -184,7 +184,7 @@ class WebSocketHybi10 : public WebSocket {
>               "Sec-WebSocket-Key is empty or isn't specified.");
>       return NULL;
>     }
> -    return new WebSocketHybi10(connection, request, pos);
> +    return new WebSocketHybi17(connection, request, pos);
>   }
>
>   virtual void Accept(const HttpServerRequestInfo& request) {
> @@ -237,6 +237,9 @@ class WebSocketHybi10 : public WebSocket {
>       return FRAME_ERROR;
>     }
>
> +    if (!masked_) // According to Hybi-17 spec client MUST mask his frame.
> +      return FRAME_ERROR;
> +
>     uint64 payload_length64 = second_byte & kPayloadLengthMask;
>     if (payload_length64 > kMaxSingleBytePayloadLength) {
>       int extended_payload_length_size;
> @@ -256,16 +259,15 @@ class WebSocketHybi10 : public WebSocket {
>     }
>
>     static const uint64 max_payload_length = 0x7FFFFFFFFFFFFFFFull;
> -    size_t masking_key_length = masked_ ? kMaskingKeyWidthInBytes : 0;
>     static size_t max_length = std::numeric_limits<size_t>::**max();
>     if (payload_length64 > max_payload_length ||
> -        payload_length64 + masking_key_length > max_length) {
> +        payload_length64 + kMaskingKeyWidthInBytes > max_length) {
>       // WebSocket frame length too large.
>       return FRAME_ERROR;
>     }
>     payload_length_ = static_cast<size_t>(payload_**length64);
>
> -    size_t total_length = masking_key_length + payload_length_;
> +    size_t total_length = kMaskingKeyWidthInBytes + payload_length_;
>     if (static_cast<size_t>(buffer_**end - p) < total_length)
>       return FRAME_INCOMPLETE;
>
> @@ -280,7 +282,7 @@ class WebSocketHybi10 : public WebSocket {
>       message->swap(buffer);
>     }
>
> -    size_t pos = p + masking_key_length + payload_length_ -
> +    size_t pos = p + kMaskingKeyWidthInBytes + payload_length_ -
>         connection_->recv_data().c_**str();
>     connection_->Shift(pos);
>
> @@ -297,13 +299,13 @@ class WebSocketHybi10 : public WebSocket {
>
>     frame.push_back(kFinalBit | op_code);
>     if (data_length <= kMaxSingleBytePayloadLength)
> -      frame.push_back(kMaskBit | data_length);
> +      frame.push_back(data_length);
>     else if (data_length <= 0xFFFF) {
> -      frame.push_back(kMaskBit | kTwoBytePayloadLengthField);
> +      frame.push_back(**kTwoBytePayloadLengthField);
>       frame.push_back((data_length & 0xFF00) >> 8);
>       frame.push_back(data_length & 0xFF);
>     } else {
> -      frame.push_back(kMaskBit | kEightBytePayloadLengthField);
> +      frame.push_back(**kEightBytePayloadLengthField);
>       char extended_payload_length[8];
>       size_t remaining = data_length;
>       // Fill the length into extended_payload_length in the network byte
> order.
> @@ -317,25 +319,14 @@ class WebSocketHybi10 : public WebSocket {
>       DCHECK(!remaining);
>     }
>
> -    // Mask the frame.
> -    size_t masking_key_start = frame.size();
> -    // Add placeholder for masking key. Will be overwritten.
> -    frame.resize(frame.size() + kMaskingKeyWidthInBytes);
> -    size_t payload_start = frame.size();
>     const char* data = message.c_str();
>     frame.insert(frame.end(), data, data + data_length);
>
> -    base::RandBytes(&frame[0] + masking_key_start,
> -                    kMaskingKeyWidthInBytes);
> -    for (size_t i = 0; i < data_length; ++i) {
> -      frame[payload_start + i] ^=
> -          frame[masking_key_start + i % kMaskingKeyWidthInBytes];
> -    }
>     connection_->Send(&frame[0], frame.size());
>   }
>
>  private:
> -  WebSocketHybi10(**HttpConnection* connection,
> +  WebSocketHybi17(**HttpConnection* connection,
>                   const HttpServerRequestInfo& request,
>                   size_t* pos)
>     : WebSocket(connection),
> @@ -362,7 +353,7 @@ class WebSocketHybi10 : public WebSocket {
>   const char* frame_end_;
>   bool closed_;
>
> -  DISALLOW_COPY_AND_ASSIGN(**WebSocketHybi10);
> +  DISALLOW_COPY_AND_ASSIGN(**WebSocketHybi17);
>  };
>
>  }  // anonymous namespace
> @@ -370,7 +361,7 @@ class WebSocketHybi10 : public WebSocket {
>  WebSocket* WebSocket::CreateWebSocket(**HttpConnection* connection,
>                                       const HttpServerRequestInfo& request,
>                                       size_t* pos) {
> -  WebSocket* socket = WebSocketHybi10::Create(**connection, request,
> pos);
> +  WebSocket* socket = WebSocketHybi17::Create(**connection, request,
> pos);
>   if (socket)
>     return socket;
>
>
>
>

Powered by Google App Engine
This is Rietveld 408576698