Issue 16363005: Ensure that only byte values are sent by sockets and web sockets

Issue 16363005: Ensure that only byte values are sent by sockets and web sockets (Closed)

Created:
7 years, 6 months ago by Søren Gjesse

Modified:
7 years, 6 months ago

Reviewers:
Anders Johnsen

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Ensure that only byte values are sent by sockets and web sockets R=ajohnsen@google.com BUG=http://dartbug.com/11007 Committed: https://code.google.com/p/dart/source/detail?r=23896

Patch Set 2 : Addressed review comments #

Patch Set 3 : Fixed a few bugs #

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+466 lines, -12 lines)			Patch
M	runtime/bin/socket_patch.dart	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	sdk/lib/io/common.dart	View	1	2 chunks	+23 lines, -1 line	0 comments	Download
M	sdk/lib/io/file_impl.dart	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	sdk/lib/io/websocket_impl.dart	View	1 2	1 chunk	+21 lines, -3 lines	0 comments	Download
M	tests/standalone/io/raw_socket_test.dart	View		1 chunk	+1 line, -5 lines	0 comments	Download
A	tests/standalone/io/raw_socket_typed_data_test.dart	View	1 2	1 chunk	+208 lines, -0 lines	0 comments	Download
A	tests/standalone/io/web_socket_typed_data_test.dart	View	1	1 chunk	+210 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

Anders Johnsen

LGTM, nice tests. https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/common.dart File sdk/lib/io/common.dart (right): https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/common.dart#newcode134 sdk/lib/io/common.dart:134: _BufferAndStart _ensureFastAndSerializableByteBuffer( It's not a byte ...

7 years, 6 months ago (2013-06-12 06:13:58 UTC) #2

Søren Gjesse

Committed patchset #3 manually as r23896 (presubmit successful).

7 years, 6 months ago (2013-06-12 08:33:27 UTC) #3

Søren Gjesse

7 years, 6 months ago (2013-06-12 08:34:00 UTC) #4

Message was sent while issue was closed.

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/common.dart
File sdk/lib/io/common.dart (right):

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/common.dart#newcod...
sdk/lib/io/common.dart:134: _BufferAndStart
_ensureFastAndSerializableByteBuffer(
On 2013/06/12 06:13:58, Anders Johnsen wrote:
> It's not a byte buffer but a view we returns, but maybe name it
> _ensureFastAndSerializableTypedData?

Done (just Data instead of TypedData).

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/websocket_impl.dart
File sdk/lib/io/websocket_impl.dart (right):

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/websocket_impl.dar...
sdk/lib/io/websocket_impl.dart:528: if (mask) {
On 2013/06/12 06:13:58, Anders Johnsen wrote:
> What if it's not masked?

If it is not masked it will be send directly on the socket and checked there.
The tests should cover this as data send from the server side is not masked.

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/websocket_impl.dar...
sdk/lib/io/websocket_impl.dart:534: var list = new Uint8List(data.length);
On 2013/06/12 06:13:58, Anders Johnsen wrote:
> If we check if it's a string here, we could just assign data to list in that
> case.

Good point. Done.

https://codereview.chromium.org/16363005/diff/1/sdk/lib/io/websocket_impl.dar...
sdk/lib/io/websocket_impl.dart:541: if (data[i] < 0 || 255 < data[0]) {
On 2013/06/12 06:13:58, Anders Johnsen wrote:
> This reads odd, as you have two '<'. Also, second data index should be
data[i].

This reads nicely :-) data[i] is outside the interval [0..255].

Fixed data[0].

Expand Messages | Collapse Messages