Bug 64344

Summary:

WebSocket: Implement hybi handshake

Product:

WebKit

Reporter:

Yuta Kitamura <yutak>

Component:

WebCore Misc.

Assignee:

Yuta Kitamura <yutak>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, ap, tkent, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

50099

Attachments:

Description	Flags
Patch	none
Patch v2	none
Patch v3	none
Patch v4	none

Yuta Kitamura

Reported 2011-07-11 23:33:56 PDT

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

Attachments
Patch (42.95 KB, patch) 2011-07-12 00:58 PDT, Yuta Kitamura	no flags	Details Formatted Diff Diff
Patch v2 (42.89 KB, patch) 2011-07-12 04:48 PDT, Yuta Kitamura	no flags	Details Formatted Diff Diff
Patch v3 (42.79 KB, patch) 2011-07-12 04:52 PDT, Yuta Kitamura	no flags	Details Formatted Diff Diff
Patch v4 (44.43 KB, patch) 2011-07-13 01:06 PDT, Yuta Kitamura	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Yuta Kitamura

Comment 1 2011-07-12 00:58:36 PDT

Created attachment 100447 [details] Patch

Kent Tamura

Comment 2 2011-07-12 03:43:11 PDT

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

Comment 3 2011-07-12 04:48:55 PDT

Created attachment 100467 [details] Patch v2

Yuta Kitamura

Comment 4 2011-07-12 04:49:38 PDT

Comment on attachment 100467 [details] Patch v2 Oops, I forgot to address the first comment. Ignore this patch.

Yuta Kitamura

Comment 5 2011-07-12 04:52:07 PDT

Created attachment 100468 [details] Patch v3

Yuta Kitamura

Comment 6 2011-07-12 04:54:40 PDT

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

Comment 7 2011-07-13 01:06:46 PDT

Created attachment 100640 [details] Patch v4

Kent Tamura

Comment 8 2011-07-13 21:32:34 PDT

Comment on attachment 100640 [details] Patch v4 Looks ok.

WebKit Review Bot

Comment 9 2011-07-13 22:32:59 PDT

Comment on attachment 100640 [details] Patch v4 Clearing flags on attachment: 100640 Committed r90979: <http://trac.webkit.org/changeset/90979>

WebKit Review Bot

Comment 10 2011-07-13 22:33:04 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.