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

Issue 10377124: Add a hash code to client and server web socket connections (Closed)

Created:
8 years, 7 months ago by Søren Gjesse
Modified:
8 years, 7 months ago
Reviewers:
Mads Ager (google)
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Add a hash code to client and server web socket connections The client web socket connection uses part of the nonce generated for the challenge. The server web socket connection uses the hash code from the socket. R=ager@google.com BUG=dart:3017 TEST=tests/standalone/io/web_socket_test.dart Committed: https://code.google.com/p/dart/source/detail?r=7613

Patch Set 1 #

Total comments: 4

Patch Set 2 : Addressed review comments from ager@ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+78 lines, -5 lines) Patch
M runtime/bin/websocket.dart View 3 chunks +13 lines, -2 lines 0 comments Download
M runtime/bin/websocket_impl.dart View 1 7 chunks +10 lines, -3 lines 0 comments Download
M tests/standalone/io/web_socket_test.dart View 1 2 chunks +55 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Søren Gjesse
8 years, 7 months ago (2012-05-14 07:29:40 UTC) #1
Mads Ager (google)
lgtm https://chromiumcodereview.appspot.com/10377124/diff/1/runtime/bin/websocket_impl.dart File runtime/bin/websocket_impl.dart (right): https://chromiumcodereview.appspot.com/10377124/diff/1/runtime/bin/websocket_impl.dart#newcode643 runtime/bin/websocket_impl.dart:643: // Generate the nonce now as it is ...
8 years, 7 months ago (2012-05-14 09:19:39 UTC) #2
Søren Gjesse
8 years, 7 months ago (2012-05-14 15:52:41 UTC) #3
http://codereview.chromium.org/10377124/diff/1/runtime/bin/websocket_impl.dart
File runtime/bin/websocket_impl.dart (right):

http://codereview.chromium.org/10377124/diff/1/runtime/bin/websocket_impl.dar...
runtime/bin/websocket_impl.dart:643: // Generate the nonce now as it is also
used to set the hash code.
On 2012/05/14 09:19:39, Mads Ager wrote:
> I don't actually know if it would be a security issue to use the nonce in the
> hash code. It does sound a little fishy, so maybe we should just generate some
> more random bytes for the hash?

Done.

http://codereview.chromium.org/10377124/diff/1/tests/standalone/io/web_socket...
File tests/standalone/io/web_socket_test.dart (right):

http://codereview.chromium.org/10377124/diff/1/tests/standalone/io/web_socket...
tests/standalone/io/web_socket_test.dart:153: void testHashCode() {
On 2012/05/14 09:19:39, Mads Ager wrote:
> How about extending this with a case where you put them in a hash map and find
> them again?

Done.

Powered by Google App Engine
This is Rietveld 408576698