[Webkit-unassigned] [Bug 82307] [WebSocket]Browser must fail connection if Sec-WebSocket-Protocol mismatched.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=82307


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #134001|review?                     |review-
               Flag|                            |




--- Comment #3 from Benjamin Poulain <benjamin at webkit.org>  2012-03-27 02:39:25 PST ---
(From update of attachment 134001)
View in context: https://bugs.webkit.org/attachment.cgi?id=134001&action=review

As for the previous patches. I do not know enough about the state of WebSocket to r+ so I'll leave the final review to a more knowledgeable reviewer.

> Source/WebCore/ChangeLog:9
> +        From RFC6455, if the WebSocket openhanding respond included the mismatched
> +        Sec-WebSocket-Protocol header field, the client must fail the WebSocket Connection.

I think it is useful when you also add a link to the spec.

> Source/WebCore/Modules/websockets/WebSocketHandshake.cpp:729
> +            size_t i;

The variable name "i" is a bad name in this context. We use "i" when it is stopped an obvious what it is.

Here, you should use something like serverWebSocketProtocolIndex or better.

> LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-mismatch-protocol-header.html:24
> +    if (protocolCase[curTestNumber] === "")
> +        ws = new WebSocket(url);

How does this case happen?

All the tests are run with integers. The strict equality does not seems to mach any of the invocation.

> LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-mismatch-protocol-header_wsh.py:13
> +    raise handshake.AbortedByUserException('Abort the connection') # Prevents pywebsocket from sending its own handshake message.

The comment should probably be on the above line if you want to follow python's style.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.