Bug 92729 – code parameter of WebSocket::close() should be unsigned value than signed.

• Component: New Bugs
• Status: RESOLVED CONFIGURATION CHANGED

[Vineet Chaudhary (vineetc)]

This has came up with discussion from Bug92642.

Currently as per the WebSocket API spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/network.html#websocket
The first parameter(code) of the close() api should be unsigned value but currently it is signed value.

Replacing this code value to unsigned and WebSocketChannel::CloseEventCodeNotSpecified to 0 we can tweak idl definition as

void close(in [Clamp, Optional=DefaultIsUndefined] unsigned short code, in [Optional=DefaultIsNullString] DOMString reason) raises(DOMException);

Also below extra defiantions of close() from WebSocket.h can be removed.

void close(ExceptionCode& ec) { close(WebSocketChannel::CloseEventCodeNotSpecified, String(), ec); }
void close(int code, ExceptionCode& ec) { close(code, String(), ec); }

[Vineet Chaudhary (vineetc)]

Created attachment 155473 [details]
proposed patch

This is the initial approach to fix this issue.

[Vineet Chaudhary (vineetc)]

(In reply to comment #1)
> Created an attachment (id=155473) [details]
> proposed patch
> 
> This is the initial approach to fix this issue.

With this patch one known failing chromium test case is LayoutTests/http/tests/websocket/tests/hybi/close.html

Reason is now close() api doesn't have any mechanism to know "0" value is because of MISSING_PARAMETER or its clamped value from Nan,+/-Infinity.

[Kentaro Hara]

toyoshim: Is that value used by chromium's PPAPI bindings only? I mean, if you could fix the PPAPI bindings, will there be no other compatibility concern?

[Takashi Toyoshima]

Hum... We should raise a proper exception for out of range closing code.
So reusing 0 will mistakenly accept invalid closing code 0 which is passed by JavaScript.

By the way, WebKit API modification requires special review by API owners.
tkent must be the person for that.

[Takashi Toyoshima]

How about following idea?

- move WebSocket::close implementation to WebSocket::close(bool, int, const String&, ExceptionCode&). This implementation accept four arguments, and the first bool indicates if code and reason are passed.
- all three close bindings use this four argument version close.
- keep CloseEventCodeNotSpecified being -1, and keep WebSocketChannel::close accepting int value for code argument.
- also keep WebKit API as is.

[Takashi Toyoshima]

(In reply to comment #3)
> toyoshim: Is that value used by chromium's PPAPI bindings only? I mean, if you could fix the PPAPI bindings, will there be no other compatibility concern?

PPAPI bindings will work if WebKit API's value is also changed as attached patched did.
I have no other concern on changing this value, except for the point of comment #4.

[Anne van Kesteren]

This has been resolved at some point in the past.