Support for WebSocket version Hixie draft 76

[sonnyp]

Follow up https://code.google.com/p/node-xmpp-bosh/issues/detail?id=37

http://tools.ietf.org/html/draft-hixie-thewebsocketprotocol-76
Hixie draft 76 is old and deprecated, but still in use by Safari and Opera.

[sonnyp]

Last time I've checked, node-xmpp-bosh allowed to specify the websocket library to use so Hixie draft 76 was supported. The problem with this solution is that I had to run 2 node-xmpp-bosh instances.

I'm working on adding support for Hixie draft 76 by using this WebSocket library https://github.com/einaros/ws .
It supports all commonly used WebSocket revision.
For a benchmark you can take a look at http://worlize.github.com/WebSocket-Node/benchmarks/

Would you be interested in such a patch?

[dhruvbird]

@sonnyp yep - sounds good! a patch would be welcome :)

So, let me see if I understood this right - the library https://github.com/einaros/ws will (after your changes) support all revisions from draft 76 through the current RFC?

[dhruvbird]

@sonnyp I just saw the project page, and it seems that https://github.com/einaros/ws already supports draft76?

[sonnyp]

When I said "I'm working on adding support for Hixie draft 76", I meant on node-xmpp-bosh, not on ws, which, as you said, already support it (which is why I would like to use it for node-xmpp-bosh).

[dhruvbird]

Ah! I had misunderstood you earlier! Sounds good!

I think we can modify the default websocket.js to use this new websocket library rather than websocket-node since it supports more versions of the protocol and the benchmarks seem to suggest that it is equally performant.

[dhruvbird]

Do you see any value in supporting both websocket libraries? I did that in the past and it was slightly painful, without much benefit. I assumed that the community would gravitate toward one websocket library eventually, but that hasn't happened yet (it seems).

[sonnyp]

No I don't see any value in supporting both WebSocket libraries.
However I think that in the future we'll probably have to switch the WebSocket library again. But I believe ws is currently the right choice given the fact Hixie Draft 76 is still widely used (Opera, Safari, outdated Firefox and outdated Chrome).

[dhruvbird]

@sonnyp Any reason why you think that websocket-node is the long-term solution? (just curious)

[sonnyp]

Sorry, I shouldn't have used the word "switch", it wasn't appropriate for what I meant.
I meant that we'll probably have to change the WebSocket backend again. whatever the good one will be.

[dhruvbird]

ah! got you (y)

[sonnyp]

https://github.com/sonnyp/node-xmpp-bosh/commit/6d42bf06d3a55c5a8f3115466a8b907c94886d77

It works, but it's probably still a work in progress, I would really appreciate your feedback on this.
There are probably CORS-related issue and the sub-protocol isn't supported anymore. What would be the correct behavior on CORS and subprotocol?

I took the liberty to strip trailing spaces, replace tabs by spaces and re-indent. Please tell me if you prefer me to do that in a separated commit and/or if you don't want those changes.

[dhruvbird]

This is super!

We prefer spaces instead of tabs. Yeah, a separate commit for the
formatting related changes would be great (helps reviewing the
changes).

Do we accept anything in the subprotocol? I mean it's okay as long as
we are more permissive and don't disallow valid connections.
If that is the case, we can add a comment next to the subprotocol: bit
explaining the behaviour and why that is the case.

I don't know what CORS issue could pop up with websockets. I've been
testing from the browser all this while and it has been working okay.
I'll double check and see if NXB is sending any CORS related headers
when the websocket bit is hit.

Thanks!
-Dhruv.

p.s. Also, could you add yourself to the list if contribs. in
package.json and to the header of any file you modify.

On Thu, Apr 5, 2012 at 9:32 AM, Sonny
reply@reply.github.com
wrote:

https://github.com/sonnyp/node-xmpp-bosh/commit/6d42bf06d3a55c5a8f3115466a8b907c94886d77

It works, but it's probably still a work in progress, I would really appreciate your feedback on this.
There are probably CORS-related issue and the sub-protocol isn't supported anymore. What would be the correct behavior on CORS and subprotocol?

I took the liberty to strip trailing spaces, replace tabs by spaces and re-indent. Please tell me if you prefer me to do that in a separated commit and/or if you don't want those changes.

---

Reply to this email directly or view it on GitHub:
#24 (comment)

   -Dhruv Matani.
http://dhruvbird.com/

"What's the simplest thing that could possibly work?"
-- Ward Cunningham

[sonnyp]

For the subprotocol concern, I thought Opera and Safari didn't support the subprotocol mechanism but apparently they do. I think we should refuse connection when the xmpp subprotocol isn't declared in the header, it might help avoid error for the client.
And of course xmpp-bosh should always send it.

Regarding CORS, I'm more worried about security, but I'm note sure why :-)
I need to take a look at xmpp-bosh and ws internals.

I will address your comment on formatting stuff.

I've just tested and I can confirm xmpp-bosh WebSocket transport is now fully functional with Opera, Safari and Safari iOS. \o/

[sonnyp]

https://github.com/sonnyp/node-xmpp-bosh/tree/ws

[dhruvbird]

@sonnyp Yes, if both the client and the ws package support it, we should allow it.

w.r.t CORS, I am not sure if it is applicable here since we connect to a ws:// URL and not an http:// URL. Of course, I am not at all sure about this and would be great if you could confirm that.

Tested with Chrome as well.

[sonnyp]

"Yes, if both the client and the ws package support it, we should allow it."
You mean subprotocol? What I mean is that I think we should refuse connection that doesn't specify the xmpp subprotocol.

"w.r.t CORS, I am not sure if it is applicable here since we connect to a ws:// URL and not an http:// URL. Of course, I am not at all sure about this and would be great if you could confirm that."
Actually the client will first send a switch protocol request to the http:// origin URL.

[dhruvbird]

w.r.t. subprotocol, I agree - only the xmpp subprotocol should be allowed given that all clients support this.

Yes, even I see that it is the case, but the protocol negotiation seems to be currently working w/o the CORS headers. I wonder how?

[dhruvbird]

@sonnyp Send me a pull request once you are reasonably confident of the stability of your branch.

[sonnyp]

I randomly have this error, need investigation:
/home/tmp/xmpp-bosh/node-xmpp-bosh/node_modules/ws/lib/WebSocket.js:199 else throw new Error('not opened'); ^ Error: not opened at WebSocket.send (/home/tmp/xmpp-bosh/node-xmpp-bosh/node_modules/ws/lib/WebSocket.js:199:16) at /home/tmp/xmpp-bosh/node-xmpp-bosh/src/websocket.js:122:17 at WebSocketEventPipe.emit (/home/tmp/xmpp-bosh/node-xmpp-bosh/node_modules/eventpipe/eventpipe.js:63:25) at Object.<anonymous> (/home/tmp/xmpp-bosh/node-xmpp-bosh/src/xmpp-proxy-connector.js:77:20) at XMPPProxy.<anonymous> (native) at XMPPProxy.emit (events.js:70:17) at XMPPProxy._on_stanza (/home/tmp/xmpp-bosh/node-xmpp-bosh/src/xmpp-proxy.js:204:18) at XmppStreamParser.<anonymous> (native) at XmppStreamParser.emit (events.js:67:17) at XmppStreamParser._handle_end_element (/home/tmp/xmpp-bosh/node-xmpp-bosh/src/stream-parser.js:78:18)

[dhruvbird]

@sonnyp Do you still see such crashes or does it seem to have been fixed?

[sonnyp]

Sorry haven't been around lately.
Give me a week to confirm.

[dhruvbird]

sure!

[sonnyp]

I couldn't reproduce the error.

[dhruvbird]

Great news - thanks for confirming!

[sonnyp]

I'm looking at the subprotocol support within ws.

[sonnyp]

The sub protocol handler was borken, I sent a pull request with a fix.
I don't believe we should wait for this to be landed and release. I'm closing this bug an opening a new one about sub protocol so we don't forget about this.

https://github.com/einaros/ws/pull/97/files

[dhruvbird]

+1