The default bug view has changed. See this FAQ.

Update WebSocket API to latest draft

RESOLVED FIXED in mozilla11

Status

()

Core
Networking: WebSockets
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: sheppy, Assigned: jduell)

Tracking

({dev-doc-needed})

Trunk
mozilla11
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 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.
(Reporter)

Comment 1

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

Comment 2

6 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/
(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

6 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/
Blocks: 674527
No longer blocks: 674527
Depends on: 674527
Blocks: 674716
No longer blocks: 674716
Depends on: 674716
Blocks: 674890
No longer blocks: 674890
Depends on: 674890
Blocks: 674905
No longer blocks: 674905
Depends on: 674905
Depends on: 676439

Comment 5

6 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

Comment 6

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

Comment 7

6 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?
(Assignee)

Comment 8

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

Comment 9

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

Comment 10

6 years ago
Jason, okay, glad to hear someone is working on it. Post here if you have a build you would like tested.
(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.
(Assignee)

Updated

6 years ago
Blocks: 695635
(Assignee)

Updated

5 years ago
Depends on: 710345
(Assignee)

Comment 12

5 years ago
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.
Attachment #581386 - Flags: review?(mcmanus)
(Assignee)

Updated

5 years ago
Duplicate of this bug: 695250
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.
Attachment #581386 - Flags: review?(mcmanus) → review+
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).
(Assignee)

Updated

5 years ago
Depends on: 710964
(Assignee)

Comment 16

5 years ago
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.
Attachment #582456 - Flags: review?(mcmanus)
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
Attachment #582456 - Flags: review?(mcmanus) → review+
(Assignee)

Comment 18

5 years ago
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.
Assignee: nobody → jduell.mcbugs
Status: NEW → ASSIGNED
Attachment #582496 - Flags: review?(mcmanus)
Yeah, no need to keep it prefixed for that. That's more Of a security issue than a feature that people would rely on
(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.
Attachment #582496 - Flags: review?(mcmanus) → review+
(Assignee)

Comment 21

5 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.
(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 :)
(Assignee)

Comment 23

5 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.
Attachment #582496 - Attachment is obsolete: true
(Assignee)

Comment 24

5 years ago
https://hg.mozilla.org/mozilla-central/rev/7f42341adb99
https://hg.mozilla.org/mozilla-central/rev/90348927d796
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
(Assignee)

Comment 25

5 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).
You need to log in before you can comment on or make changes to this bug.