Bug 64344 – WebSocket: Implement hybi handshake

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Yuta Kitamura]

Implement hybi WebSocket handshake.

http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-10#section-5.1

[Yuta Kitamura]

Created attachment 100447 [details]
Patch

[Kent Tamura]

Comment on attachment 100447 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100447&action=review

> LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-no-accept-header_wsh.py:5
> +    # message += 'Sec-WebSocket-Accept: %s\r\n' % compute_accept(request.headers_in['Sec-WebSocket-Key'])[0]

Is this line needed? If not, please remove instead of comment-out.

> Source/WebCore/websockets/WebSocketHandshake.cpp:169
> +    unsigned char key[16];
> +    cryptographicallyRandomValues(key, 16);
> +    return base64Encode(reinterpret_cast<char*>(key), 16);

Please define const variable for the magic number '16'.

> Source/WebCore/websockets/WebSocketHandshake.cpp:180
> +    Vector<uint8_t, 20> hash;
> +    sha1.computeHash(hash);
> +    return base64Encode(reinterpret_cast<const char*>(hash.data()), 20);

We had better have
   const size_t Sha1HashSize = 20;
here or SHA1.h.

[Yuta Kitamura]

Created attachment 100467 [details]
Patch v2

[Yuta Kitamura]

Comment on attachment 100467 [details]
Patch v2

Oops, I forgot to address the first comment. Ignore this patch.

[Yuta Kitamura]

Created attachment 100468 [details]
Patch v3

[Yuta Kitamura]

Comment on attachment 100447 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=100447&action=review

>> LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-no-accept-header_wsh.py:5
>> +    # message += 'Sec-WebSocket-Accept: %s\r\n' % compute_accept(request.headers_in['Sec-WebSocket-Key'])[0]
> 
> Is this line needed? If not, please remove instead of comment-out.

Removed.

>> Source/WebCore/websockets/WebSocketHandshake.cpp:169
>> +    return base64Encode(reinterpret_cast<char*>(key), 16);
> 
> Please define const variable for the magic number '16'.

Defined "nonceSize".

>> Source/WebCore/websockets/WebSocketHandshake.cpp:180
>> +    return base64Encode(reinterpret_cast<const char*>(hash.data()), 20);
> 
> We had better have
>    const size_t Sha1HashSize = 20;
> here or SHA1.h.

Defined a constant inside this function for now. Added a FIXME comment to tell it really should be moved to SHA1.h.

[Yuta Kitamura]

Created attachment 100640 [details]
Patch v4

[Kent Tamura]

Comment on attachment 100640 [details]
Patch v4

Looks ok.

[WebKit Review Bot]

Comment on attachment 100640 [details]
Patch v4

Clearing flags on attachment: 100640

Committed r90979: <http://trac.webkit.org/changeset/90979>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.