faye/faye-websocket-node

Propagate invalid extensions errors gracefully

Closed this issue · 4 comments

macinjosh32 commented

Ever since upgrading to a version of SockJS that uses Faye v0.9.x, my application is crashing periodically with this stack trace:

  [ 'SyntaxError: Invalid Sec-WebSocket-Extensions header: x-webkit-       -frame',
    '    at Object.Parser.parseHeader (/home/sic/projects/sic/node-server/node_modules/sockjs/node_modules/faye-websocket/node_modules/websocket-driver/node_modules/websocket-extensions/lib/parser.js:17:13)',
    '    at instance.generateResponse (/home/sic/projects/sic/node-server/node_modules/sockjs/node_modules/faye-websocket/node_modules/websocket-driver/node_modules/websocket-extensions/lib/websocket_extensions.js:92:27)',
    '    at instance._handshakeResponse (/home/sic/projects/sic/node-server/node_modules/sockjs/node_modules/faye-websocket/node_modules/websocket-driver/lib/websocket/driver/hybi.js:284:39)',
    '    at instance.start (/home/sic/projects/sic/node-server/node_modules/sockjs/node_modules/faye-websocket/node_modules/websocket-driver/lib/websocket/driver/base.js:80:22)',
    '    at /home/sic/projects/sic/node-server/node_modules/sockjs/node_modules/faye-websocket/lib/faye/websocket.js:27:18',
    '    at process._tickCallback (node.js:442:13)' ] }

Is there a way these errors can be emitted more gracefully so as to allow SockJS or my application to handle them?

macinjosh32 commented

This morning I also saw this--

uncaughtException: Invalid Sec-WebSocket-Extensions header: permessage-       ; client_max_window_bits, x-webkit-       -frame

Seems like the word "deflate" is getting blotted out.

jcoglan commented

This error should not be leaking out of websocket-extensions; if the client's header is invalid, we should probably just ignore it and establish the connection without accepting any proposed extensions. That's implemented in faye/websocket-extensions-node@62ac506.

I don't know why deflate is missing from your headers; nothing in the source code of faye-websocket, websocket-driver or websocket-extensions mentions the string deflate. That header should be passed directly from the server down to websocket-extensions without being modified, and I can't see any code in the above modules that modifies the header.

glasser commented

This sounds like some random proxy in between your clients and server is replacing deflate with whitespace.

jcoglan commented

I made a mistake in my original fix for this issue. The spec says that a server or client in receipt of an invalid Sec-WebSocket-Extensions header must fail the connection. I've implemented this in faye/websocket-extensions-node@c8f31cc and faye/websocket-driver-node@0034f5f. The server-side WebSocket should emit error and close in this scenario now, without sending a handshake back to the client.