Eric Shepherd [:sheppy] Reporter
	Description • 13 years ago

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.

	Eric Shepherd [:sheppy] Reporter
	Comment 1 • 13 years ago

We also don't send the CloseEvent per the specification; we send a simple event instead.

	Norio Kobota
	Comment 2 • 13 years ago

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/

	Patrick McManus [:mcmanus]
	Comment 3 • 13 years ago

(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.

	Joel Martin
	Comment 4 • 13 years ago

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/

	Joel Martin
	Comment 5 • 13 years ago

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

	Clemens Eisserer
	Comment 6 • 13 years ago

support for binary data using arraybuffer would be really nice. - for now I again have to transmit base64 encoded image data :/

	Joel Martin
	Comment 7 • 13 years ago

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?

	Jason Duell Assignee
	Comment 8 • 13 years ago

I'm working feverishly to hopefully get it into FF 9 before the cutoff next week :)

	Bronislav Klučka
	Comment 9 • 13 years ago

(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?

	Joel Martin
	Comment 10 • 13 years ago

Jason, okay, glad to hear someone is working on it. Post here if you have a build you would like tested.

	Patrick McManus [:mcmanus]
	Comment 11 • 13 years ago

(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.

	Jason Duell Assignee
	Comment 12 • 13 years ago

- 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.

	Patrick McManus [:mcmanus]
	Comment 14 • 13 years ago

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.

	Patrick McManus [:mcmanus]
	Comment 15 • 13 years ago

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).

	Jason Duell Assignee
	Comment 16 • 13 years ago

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.

	Patrick McManus [:mcmanus]
	Comment 17 • 13 years ago

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

	Jason Duell Assignee
	Comment 18 • 13 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 19 • 13 years ago

Yeah, no need to keep it prefixed for that. That's more Of a security issue than a feature that people would rely on

	Patrick McManus [:mcmanus]
	Comment 20 • 13 years ago

(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.

	Jason Duell Assignee
	Comment 21 • 13 years ago

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.

	Patrick McManus [:mcmanus]
	Comment 22 • 13 years ago

(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 :)

	Jason Duell Assignee
	Comment 23 • 13 years ago

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.

	Jason Duell Assignee
	Comment 24 • 13 years ago

https://hg.mozilla.org/mozilla-central/rev/7f42341adb99
https://hg.mozilla.org/mozilla-central/rev/90348927d796

	Jason Duell Assignee
	Comment 25 • 13 years ago

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).