Bug 32700 – WebSocket: Invalid url should raise SYNTAX_ERR exception.

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

[Fumitoshi Ukai]

If it failed to parse the url argument, thow a SYNTAX_ERR exception, as spec says.

[Fumitoshi Ukai]

Created attachment 45125 [details]
WebCore: Invalid url should raise SYNTAX_ERR exception.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45125 [details] without any errors.

[Alexey Proskuryakov]

Comment on attachment 45125 [details]
WebCore: Invalid url should raise SYNTAX_ERR exception.

r=me

I wish we could get JavaScript line number for the error messages.

[Fumitoshi Ukai]

Committed r52399: <http://trac.webkit.org/changeset/52399>

[Adam Barth]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/websocket/tests/bad-sub-protocol-expected.txt
	M	LayoutTests/websocket/tests/script-tests/url-parsing.js
	M	LayoutTests/websocket/tests/url-parsing-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/websockets/WebSocket.cpp
Committed r52402
	M	WebCore/ChangeLog
	M	WebCore/websockets/WebSocket.cpp
	M	LayoutTests/websocket/tests/url-parsing-expected.txt
	M	LayoutTests/websocket/tests/bad-sub-protocol-expected.txt
	M	LayoutTests/websocket/tests/script-tests/url-parsing.js
	M	LayoutTests/ChangeLog
r52402 = e9ae8c0dad923306815a6290589fea519962d3b4 (refs/remotes/trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk

[Adam Barth]

Comment on attachment 45125 [details]
WebCore: Invalid url should raise SYNTAX_ERR exception.

websocket/tests/bad-sub-protocol.html -> failed

[Fumitoshi Ukai]

Created attachment 45258 [details]
Invalid url should raise SYNTAX_ERR exception.

[Fumitoshi Ukai]

(In reply to comment #6)
> (From update of attachment 45125 [details])
> websocket/tests/bad-sub-protocol.html -> failed

Seems it was failed because console message containing a non-printable character differs on windows.
http://build.webkit.org/results/Windows%20Release%20(Tests)/r52399%20(7249)/websocket/tests/

protocol string should be ASCII string, so report bad protocol string with escaping.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 45258 [details] without any errors.

[Eric Seidel (no email)]

What's the status of this patch?  Was it rolled out?

[Fumitoshi Ukai]

(In reply to comment #10)
> What's the status of this patch?  Was it rolled out?

Yes. patch#1 was landed at r52399, but it caused layout test failure on win (because of difference in console message of non-printable chars), so rolled out at r52402 by Adam Barth.
patch#2 addressed the issue of the layout test failures by escaping non-printable characters in protocol string.

[Alexey Proskuryakov]

Comment on attachment 45258 [details]
Invalid url should raise SYNTAX_ERR exception.

> +static WebCore::String encodeProtocolString(const WebCore::String& protocol)

This is inside namespace WebCore, and doesn't need namespace prefixes. Please fix before landing.

> +    const UChar* characters = protocol.characters();
> +    StringBuilder builder;
> +    for (size_t i = 0; i < protocol.length(); i++) {
> +        if (characters[i] < 0x20 || characters[i] > 0x7E)

Why not just use protocol[i]?

r=me

[Fumitoshi Ukai]

Committed r52478: <http://trac.webkit.org/changeset/52478>