Last Comment Bug 666349 - Update WebSocket API to latest draft
: Update WebSocket API to latest draft
Status: RESOLVED FIXED
: dev-doc-needed
Product: Core
Classification: Components
Component: Networking: WebSockets (show other bugs)
: Trunk
: All All
: -- normal with 4 votes (vote)
: mozilla11
Assigned To: Jason Duell [:jduell] (needinfo me)
:
Mentors:
: 695250 (view as bug list)
Depends on: 674527 674716 674890 674905 676439 710345 710964
Blocks: 695635
  Show dependency treegraph
 
Reported: 2011-06-22 12:18 PDT by Eric Shepherd [:sheppy]
Modified: 2011-12-20 03:45 PST (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Upgrade Version and origin headers (2.29 KB, patch)
2011-12-13 13:08 PST, Jason Duell [:jduell] (needinfo me)
mcmanus: review+
Details | Diff | Splinter Review
update status code usage to latest spec (4.53 KB, patch)
2011-12-16 18:30 PST, Jason Duell [:jduell] (needinfo me)
mcmanus: review+
Details | Diff | Splinter Review
Fail if server replies with non-matching subprotocol (4.74 KB, patch)
2011-12-16 23:16 PST, Jason Duell [:jduell] (needinfo me)
mcmanus: review+
Details | Diff | Splinter Review

Description Eric Shepherd [:sheppy] 2011-06-22 12:18:19 PDT
The current implementation of the WebSocket interface does not match the current specification in several ways:

We don't support constructing a WebSocket with an array of protocols.

We don't support the send() method with ArrayBuffer or Blob as inputs.

We return a boolean from send() instead of void.
Comment 1 Eric Shepherd [:sheppy] 2011-06-22 12:49:54 PDT
We also don't send the CloseEvent per the specification; we send a simple event instead.
Comment 2 Norio Kobota 2011-07-19 18:16:34 PDT
seems not called onmessage event when received WebSocket frame w/ Opcode == %x2 (binary frame)

I checked w/ firefox nightly(8.0a1) and my test server - http://onmessage.ws:8080/
Comment 3 Patrick McManus [:mcmanus] PTO until Sep 6 2011-07-19 18:35:45 PDT
(In reply to comment #2)
> seems not called onmessage event when received WebSocket frame w/ Opcode ==
> %x2 (binary frame)
> 
> I checked w/ firefox nightly(8.0a1) and my test server -
> http://onmessage.ws:8080/

right now we don't yet support any binary message api.
Comment 4 Joel Martin 2011-07-26 11:29:46 PDT
For reference:

The WHATWG version is here: http://www.whatwg.org/specs/web-apps/current-work/complete/network.html#network . It has had the binary data types and CloseEvent since May 27.

The W3C version of the WebSockets API draft is here: http://dev.w3.org/html5/websockets/ . It is now in sync (since July 19th) with the WHATWG version.

The bug tracking the discussion of the new API is here: 
http://www.w3.org/Bugs/Public/show_bug.cgi?id=12102 (resolved since May 27th). 

The version that Firefox (and everybody else) currently have implemented appears to be this one: http://www.w3.org/TR/2009/WD-websockets-20091029/
Comment 5 Joel Martin 2011-08-31 07:07:43 PDT
Any progress to report on this front? It looks like WebKit is about to land full support:

https://bugs.webkit.org/show_bug.cgi?id=65249
https://bugs.webkit.org/show_bug.cgi?id=67180
https://bugs.webkit.org/show_bug.cgi?id=67013
Comment 6 Clemens Eisserer 2011-09-03 03:27:29 PDT
support for binary data using arraybuffer would be really nice. - for now I again have to transmit base64 encoded image data :/
Comment 7 Joel Martin 2011-09-22 14:09:31 PDT
Binary support has landed in Chrome 15. IE10 preview has binary support. Any update on when we might see some movement on this in firefox?
Comment 8 Jason Duell [:jduell] (needinfo me) 2011-09-22 14:27:00 PDT
I'm working feverishly to hopefully get it into FF 9 before the cutoff next week :)
Comment 9 Bronislav Klučka 2011-09-22 14:38:22 PDT
(In reply to Jason Duell (:jduell) from comment #8)
> I'm working feverishly to hopefully get it into FF 9 before the cutoff next
> week :)

FF9? do I understand it correctly that it would be out officially in like 2-3 months?
Comment 10 Joel Martin 2011-09-22 14:43:27 PDT
Jason, okay, glad to hear someone is working on it. Post here if you have a build you would like tested.
Comment 11 Patrick McManus [:mcmanus] PTO until Sep 6 2011-09-22 14:46:54 PDT
(In reply to Bronislav Klučka from comment #9)

> FF9? do I understand it correctly that it would be out officially in like
> 2-3 months?

FF9 will be official 12 weeks after FF9 is promoted to Aurora (which is next week).6 weeks on Aurora, 6 weeks on Beta. That's the way it works now.

Binary websockets support for draft-10 may or may not be on that release train - the cutover is date driven and happens independently of what features are ready for it. If it misses that train, or has to be tossed off it because it went it turns out not to be fully baked, it will presumably then be attached to the release 6 weeks later in time.
Comment 12 Jason Duell [:jduell] (needinfo me) 2011-12-13 13:08:45 PST
Created attachment 581386 [details] [diff] [review]
Upgrade Version and origin headers

- Replaces 'Sec-WebSocket-Origin' header with 'Origin' to match WS spec >= v11

- Upgrades Sec-WebSocket-Version header to version 13.

These are the changes that will make us "speak" the final WS protocol as far as wire protocol detection/compat goes, so I'm putting them here.

Let's keep review to code correctness, and discuss on the dev.tech.network newsgroup what else needs to land (and when) for this to get checked in.
Comment 13 Jason Duell [:jduell] (needinfo me) 2011-12-13 22:58:49 PST
*** Bug 695250 has been marked as a duplicate of this bug. ***
Comment 14 Patrick McManus [:mcmanus] PTO until Sep 6 2011-12-14 08:16:38 PST
Comment on attachment 581386 [details] [diff] [review]
Upgrade Version and origin headers

Review of attachment 581386 [details] [diff] [review]:
-----------------------------------------------------------------

r=mcmanus .. please change the draft-17 reference to be RFC6455 now that it is published as an RFC.
Comment 15 Patrick McManus [:mcmanus] PTO until Sep 6 2011-12-14 08:18:04 PST
protocol changes we also need to be up to date with RFC6455 (will take in another patch, but don't land the first one without them):

our use of TOO_LARGE needs to change from code 1004 to 1009 when rejecting messages that are too large.

When rejecting things because they contain invalid UTF8 we should use code 1007 (a new code).
Comment 16 Jason Duell [:jduell] (needinfo me) 2011-12-16 18:30:29 PST
Created attachment 582456 [details] [diff] [review]
update status code usage to latest spec

Updates nsIMozWebSocket to current list of status codes (including changing TOO_BIG to 1009), and change logic to pass 1007 when invalid UTF-8 received.
Comment 17 Patrick McManus [:mcmanus] PTO until Sep 6 2011-12-16 18:42:42 PST
Comment on attachment 582456 [details] [diff] [review]
update status code usage to latest spec

Review of attachment 582456 [details] [diff] [review]:
-----------------------------------------------------------------

that was easy! r=mcmanus
Comment 18 Jason Duell [:jduell] (needinfo me) 2011-12-16 23:16:26 PST
Created attachment 582496 [details] [diff] [review]
Fail if server replies with non-matching subprotocol

Probably not mission-critical, but a change in the spec:

  If the response includes a "Sec-WebSocket-Protocol" header field, and
  this header field indicates the use of a subprotocol that was not
  present in the client' handshake (the server has indicated a
  subprotocol not requested by the client), the client MUST _Fail the
  WebSocket Connection_.

I find it a little weird that a server reply w/o any Sec-WebSocket-Protocol is ok, but that's the specified behavior.  I tried to write a test for that case, too, but pywebsocket has quite a few checks to enforce that you don't reply with a blank protocol, and I don't feel like maintaining a patch fork just for it.

I think this is the last patch needed for unprefixing.  There's one more change in the spec I know of--section 7.2.3 says we should add delays if clients try to reconnect after being abnormally terminated.  I think we can punt on that for now?  If that's ok I'm going to write the un-prefix patch and land all this, then look into the message size issue.

BTW:  I think the use of mProtocol after we've called nsCRT:strtok on it may not be kosher:  nsCRT.h notes "WARNING - STRTOK WHACKS str THE FIRST TIME IT IS CALLED: MAKE A COPY OF str IF YOU NEED TO USE IT AFTER strtok()".  Not a big deal--should just mean we only log the 1st protocol in a list, IIUC.
Comment 19 Jonas Sicking (:sicking) No longer reading bugmail consistently 2011-12-17 11:26:26 PST
Yeah, no need to keep it prefixed for that. That's more Of a security issue than a feature that people would rely on
Comment 20 Patrick McManus [:mcmanus] PTO until Sep 6 2011-12-17 18:18:03 PST
(In reply to Jason Duell (:jduell) from comment #18)
> I think this is the last patch needed for unprefixing.  There's one more
> change in the spec I know of--section 7.2.3 says we should add delays if
> clients try to reconnect after being abnormally terminated.  I think we can
> punt on that for now? 

yes.

 If that's ok I'm going to write the un-prefix patch
> and land all this, then look into the message size issue.

should probably loosen message limitations before unprefixing.. because the most conservative approach may guide implementations. think that is doable? Even just set big limits for now.

> 
> BTW:  I think the use of mProtocol after we've called nsCRT:strtok on it may

yes, thats a bug. I'd appreciate a patch to copy it to local scratch before the strtok.
Comment 21 Jason Duell [:jduell] (needinfo me) 2011-12-18 00:12:12 PST
Comment on attachment 582496 [details] [diff] [review]
Fail if server replies with non-matching subprotocol

For some reason try is failing occasionally with the non-matching subprotocol patch applied.  I'm trying to repro it locally and on a remote fedora machine (the main culprit is fedora: I've seen it once on Linux).  It looks like the connection is not happening, and times out.  Might be a pywebsocket issue, but really not sure.

How do we feel about unprefixing w/o this feature if I can't fix it soon?  I don't think it's a vital feature myself.
Comment 22 Patrick McManus [:mcmanus] PTO until Sep 6 2011-12-18 07:42:15 PST
(In reply to Jason Duell (:jduell) from comment #21)
> Comment on attachment 582496 [details] [diff] [review]
> Fail if server replies with non-matching subprotocol
> 
> For some reason try is failing occasionally with the non-matching
> subprotocol patch applied.  I'm trying to repro it locally and on a remote
[..]
> 
> How do we feel about unprefixing w/o this feature if I can't fix it soon?  I
> don't think it's a vital feature myself.

It would be ok - the patch is just covering our behavior in response to a broken server which should of course never happen.

But its small - I bet you can figure it out :)
Comment 23 Jason Duell [:jduell] (needinfo me) 2011-12-18 16:23:43 PST
Comment on attachment 582496 [details] [diff] [review]
Fail if server replies with non-matching subprotocol

Split the subprotocol fix out into bug 711886, and moved this patch there.
Comment 25 Jason Duell [:jduell] (needinfo me) 2011-12-20 03:45:46 PST
We now support the final IETF spec (RFC 6455) and the W3C Candidate 
Recommendation 08 December 2011

For a good list of WebSocket protocol changes, BTW, see

   http://pywebsocket.googlecode.com/svn/wiki/WebSocketProtocolSpec.wiki

(Hybi draft 17 is the same as RFC 6455).

Note You need to log in before you can comment on or make changes to this bug.