Bug 135874 – The setter of binaryType attribute in WebSocket should raise the exception.

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

[Jinwoo Jeong]

According to W3C WebSocket Specification,
when any value exclusive of 'blob' or 'arraybuffer' is set on binaryType of WebSocket, a SyntaxError should be raised.

[Jinwoo Jeong]

Created attachment 236498 [details]
Patch

[Build Bot]

Comment on attachment 236498 [details]
Patch

Attachment 236498 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4999644836790272

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236500 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236498 [details]
Patch

Attachment 236498 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5983372736200704

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236501 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Jinwoo Jeong]

Created attachment 236502 [details]
Modify a test related with websocket.

[Build Bot]

Comment on attachment 236502 [details]
Modify a test related with websocket.

Attachment 236502 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5704798808571904

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236504 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236502 [details]
Modify a test related with websocket.

Attachment 236502 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5165196498698240

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236506 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236502 [details]
Modify a test related with websocket.

Attachment 236502 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5273852728836096

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236508 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Jinwoo Jeong]

Created attachment 236578 [details]
Modify typos on a test html file.

[Darin Adler]

Comment on attachment 236578 [details]
Modify typos on a test html file.

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

Code change is OK, test change is not. See comments below.

> LayoutTests/http/tests/websocket/tests/hybi/binary-type.html:12
> +var syntaxErr = "SyntaxError";

This is not needed.

> LayoutTests/http/tests/websocket/tests/hybi/binary-type.html:-21
> -debug("Set invalid values to binaryType. They should be ignored. No exception should be thrown.");

I don’t think this is needed at all. We should just delete it and not add new debug lines.

> LayoutTests/http/tests/websocket/tests/hybi/binary-type.html:-24
> -ws.binaryType = "Blob";
> -shouldBeEqualToString("ws.binaryType", "blob");

The change needed here is much smaller than what's done below. It should just be this.

    shouldThrow("ws.binaryType = 'Blob'", "SyntaxError");
    shouldBeEqualToString("ws.binaryType", "blob");

But also, binary-type-expected.txt has to be updated too since the expected results are changing.

[Jinwoo Jeong]

Created attachment 236908 [details]
Modify an expected file related with a websocket binary-type test

[Build Bot]

Comment on attachment 236908 [details]
Modify an expected file related with a websocket binary-type test

Attachment 236908 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/4535230291509248

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236911 [details]
Archive of layout-test-results from webkit-ews-02 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-02  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Build Bot]

Comment on attachment 236908 [details]
Modify an expected file related with a websocket binary-type test

Attachment 236908 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6689651116998656

New failing tests:
http/tests/websocket/tests/hybi/binary-type.html

[Build Bot]

Created attachment 236912 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Jinwoo Jeong]

Created attachment 236962 [details]
Modify a test file and an expected file.

[Brent Fulgham]

Comment on attachment 236962 [details]
Modify a test file and an expected file.

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

This change seems good, and passed tests. Could you please revise the patch for the current source tree and resubmit for final review?

> Source/WebCore/ChangeLog:9
> +        when invalid values is set on binaryType of WebSocket, a SyntaxError should be raised.

Could you provide a link to the specification here, or at least indicate the specification section that states this?

[Jinwoo Jeong]

I will check this.

[Jinwoo Jeong]

Created attachment 274511 [details]
Patch

[Antonio Gomes]

Comment on attachment 274511 [details]
Patch

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

r=me

Have you check how blink, gecko behave on the same scenario?

> Source/WebCore/ChangeLog:9
> +        when invalid values is set on binaryType of WebSocket, a SyntaxError should be raised.

*when an invalid value is set...

[Jinwoo Jeong]

Created attachment 274570 [details]
Patch

[Jinwoo Jeong]

Created attachment 274571 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 274571 [details]
patch for landing

Clearing flags on attachment: 274571

Committed r198482: <http://trac.webkit.org/changeset/198482>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.