xmppo / node-xmpp-bosh

An XMPP BOSH & WebSocket server (connection manager) written on node.js using Javascript
https://github.com/xmppo/node-xmpp-bosh
263 stars 85 forks source link

Not setting Sec-WebSocket-Protocol header in the handshake response #88

Closed gavllew closed 10 years ago

gavllew commented 10 years ago

The ws dependency is currently fixed to version 0.4.19. The commit einaros/ws@4fc8c81 came after that, with a fix that correctly sets the Sec-WebSocket-Protocol header.

I noticed this because the current Chrome Canary (30.0.1599.0) now rejects websocket connections to NXB with the following error:

WebSocket connection to 'wss://...' failed: Error during WebSocket handshake: Sec-WebSocket-Protocol mismatch 
dhruvbird commented 10 years ago

@gavllew Is the fix on npm? Thanks for noticing!

gavllew commented 10 years ago

Looks like that commit was released in 0.4.21, over a year ago; the latest version is 0.4.27. There is more advanced subprotocol support in the latest version, but may require NXB changes (not entirely sure).

dhruvbird commented 10 years ago

Makes sense. Updating to 0.4.21 might be a good option if chrome canary is broken.

gavllew commented 10 years ago

I was wrong about the version - the commit in question was not merged into the master branch until 0.4.22. I've my NXB instance to this version and Canary (now 31.0.1601.1) is now working fine.

dylanneild commented 10 years ago

The way I fixed this was (assuming node is in /usr/local/node and you don't already have ws installed directly for something else):

$ /usr/local/node/bin/node install ws $ cd /usr/local/node/node_modules/node-xmpp-bosh/node_modules $ rm -rf ws $ ln -s /usr/local/node/node_modules/ws ./ws

Then restarting node-xmpp-bosh.

This brought ws up to 0.4.29 which fixes the Sec-WebSocket-Protocol issue with Chrome canary and seems to otherwise work just fine.

Any reason why node-xmpp-bosh is fixed to ws 0.4.19 dependency wise?

An installable fix for this is more urgent at this point as Chrome 30.x is now the beta variant and Canary has been upped to 31.x. There aren't "a lot" of people running Canary but there are quite a few running Beta, all of whom will be unable to connect to current installs of node-xmpp-bosh over WebSockets.

dhruvbird commented 10 years ago

https://github.com/dhruvbird/node-xmpp-bosh/commit/d4f6a474a421109f20c01180d87110435cf8c605#package.json added that dep. @anup do you remember why? Also, any reason to not change it to 0.4.29?

dylanneild commented 10 years ago

I haven't seen any production issues with 0.4.29 (about 300-400 WebSocket sessions running right now).

dhruvbird commented 10 years ago

Send across a pull. Request. I'll wait a couple of days for @anup to respond in case he has some insights to share.

anup commented 10 years ago

I do not see a reason not to change to 0.4.29 or any higher version. The node-xmpp-bosh was fixed to ws 0.4.19 dependency because it was tested on it at that time. I think it can be upgraded to the latest version after testing.

dhruvbird commented 10 years ago

Thanks @anup

dhruvbird commented 10 years ago

Fixed by c9cf74916498a7ea72c63fde261762af09083a8d