http-proxy doesn't proxy the latest websocket specifications

[3rd-Eden]

Howdy,

A few days ago, we added FF6 and Chrome 14 websocket support to Socket.IO. But http-proxy doesn't seem to be able to proxy these upgrade events.

If I run a plain Socket.IO 0.8.2 chat example it works fine in FF6 and latest chrome, but if I add node-http-proxy infront of it only the Draft 76 specification receives a upgrade event.

After a bit digging I found out that https://github.com/nodejitsu/node-http-proxy/blob/master/lib/node-http-proxy.js#L862 doesn't get fired for the new websocket specifications (it does work for older websocket specifications like draft 76)

[indexzero]

@3rd-Eden. The agent object on the line that you've cited is an instance of the http.Agent from node.js core. So if the upgrade event is not firing on it this seems like a node.js core bug.

The new version of the websocket draft is on our list of priorities, but we probably won't implement it until the 0.6.x timeframe.

[3rd-Eden]

@indexzero I don't know if it's a core bug, I don't know enough of the node-http-proxy module to debug it further. I might have missed something obvious.

[indexzero]

@3rd-Eden I'll try to take a stab at it if I get some cycles this week. NodeConf SummerCamp is next week so I'm kinda strapped for time.

[DTrejo]

+1

This issue makes me T_T
Having my no.de work faster will make me :D

Thanks guys!

[nicokaiser]

Any news on this? I'd love to use node-http-proxy, but I need WebSocket (including newer drafts) support...

[indexzero]

No. This is a difficult problem.

[indexzero]

@3rd-Eden I can confirm this. I can also confirm that Firefox 6 is not working either apparently. I did some digging and here's what I'm seeing

Firefox 7

{ host: 'localhost',
  port: 3000,
  method: 'GET',
  path: '/socket.io/1/websocket/1994801303780560091',
  headers: 
   { host: 'localhost:8000',
     'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0.2) Gecko/20100101 Firefox/6.0.2',
     accept: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
     'accept-language': 'en-us,en;q=0.5',
     'accept-encoding': 'gzip, deflate',
     'accept-charset': 'ISO-8859-1,utf-8;q=0.7,*;q=0.7',
     connection: 'keep-alive, Upgrade',
     'sec-websocket-version': '7',
     'sec-websocket-origin': 'http://localhost:8000',
     'sec-websocket-key': 'i6qqGU2E/s16IHmVLxhXEg==',
     pragma: 'no-cache',
     'cache-control': 'no-cache',
     upgrade: 'websocket' } }

Chrome 13

{ host: 'localhost',
  port: 3000,
  method: 'GET',
  path: '/socket.io/1/websocket/13704292091528906386',
  headers: 
   { upgrade: 'WebSocket',
     connection: 'Upgrade',
     host: 'localhost:8000',
     origin: 'http://localhost:8000',
     'sec-websocket-key1': '2 [ 1O09 k^ hr 0u.0123 0',
     'sec-websocket-key2': '31 0g[| 38.5DyvA6 9 4yqG  0U',
     cookie: '__utma=1.484068460.1314426697.1314426697.1314426697.1; __utmz=1.1314426697.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); _chartbeat2=xjfrmfu3529rbg7z' } }

[indexzero]

Clearly this isn't working in Firefox 6 because Firefox 6 implements WebSockets draft-07 (source: http://en.wikipedia.org/wiki/WebSocket)

[indexzero]

@3rd-Eden Did further investigation here. This may be a bug in node.js core (or more specifically the response parsing in http-parser so I'm going to attempt to summon @ry.

I can confirm deep in the tendrils of node (https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1230-1265), the upgrade event is not being fired for outgoing HTTP requests in the draft-07 protocol.

I'm no expert of the HttpParser used by node.js here, but it seems like the request and response parsing may be different code paths. Clearly the parser is in a different state:

"Request" mode (working): https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1020
"Response" mode (no working): https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1203

I'm building v0.5.5 now to see if this is fixed in HEAD.

[3rd-Eden]

But the odd thing is that If we attach socket.io to a normal HTTP server it does work. So it's not a fundamental flaw on the node core, it might just be related to http client

On 10 sep. 2011, at 12:08, Charlie Robbins wrote:

@3rd-Eden Did further investigation here. This may be a bug in node.js core (or more specifically the response parsing in http-parser so I'm going to attempt to summon @ry.

I can confirm deep in the tendrils of node (https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1230-1265), the upgrade event is not being fired for outgoing HTTP requests in the draft-07 protocol.

I'm no expert of the HttpParser used by node.js here, but it seems like the request and response parsing may be different code paths. Clearly the parser is in a different state:

"Request" mode (working): https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1020
"Response" mode (no working): https://github.com/joyent/node/blob/v0.4.11/lib/http.js#L1203

I'm building v0.5.5 now to see if this is fixed in HEAD.

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

[indexzero]

@3rd-Eden @ry I can confirm that this still isn't working in Firefox 6 and Chrome 14 upon upgrading to http2 from @mikeal in v0.5.5.

I would like to make note for @mikeal that http2 is going to simplify the implementation quite a bit because the upgrade event is fired on the ClientRequest object and not the Agent.

However, the upgrade event is still not firing and I'm starting to think that this is a problem with http-parser since it seems to hold the necessary and sufficient state for the event to be fired: https://github.com/joyent/node/blob/v0.5.5/lib/http2.js#L1107

@ry @mikeal Any thoughts here? I'm stumped.

[indexzero]

The http2 compatible of node-http-proxy is here: https://github.com/nodejitsu/node-http-proxy/tree/http2

[indexzero]

@3rd-Eden

But the odd thing is that If we attach socket.io to a normal HTTP server it does work. So it's not a fundamental flaw on
the node core, it might just be related to http client

Exactly. When you attach socket.io to a normal HTTP server the http-parser is in the "Request" state. When node-http-proxy is attempting to make an outgoing WebSocket connection it is in the "Response" state. I noticed that the only current implementation of the later spec(s) in node is actually using the deprecated http.createClient method because they claim "Node's new Agent-based API is buggy."

I'm not sure if these bugs are in the http-parser or the Agent but as I tried this out with both http.js and http2.js only to get the same result I'm suspicious of it being at the http-level. No evidence to support that claim though.

https://github.com/Worlize/WebSocket-Node/blob/master/lib/WebSocketClient.js#L149-150

[3rd-Eden]

So if http-proxy starts using the old interface this would be resolved i guess?

On 10 sep. 2011, at 13:16, Charlie Robbins wrote:

@3rd-Eden

But the odd thing is that If we attach socket.io to a normal HTTP server it does work. So it's not a fundamental flaw on
the node core, it might just be related to http client

Exactly. When you attach socket.io to a normal HTTP server the http-parser is in the "Request" state. When node-http-proxy is attempting to make an outgoing WebSocket connection it is in the "Response" state. I noticed that the only current implementation of the later spec(s) in node is actually using the deprecated http.createClient method because they claim "Node's new Agent-based API is buggy."

https://github.com/Worlize/WebSocket-Node/blob/master/lib/WebSocketClient.js#L149-150

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

[indexzero]

Thats not a viable solution

[indexzero]

@3rd-Eden Wanted to give more info on my last comment. It was like 8am here; so I was pretty spent after pushing out 0.7.0.

From what I can see in http2 using the deprecated http.Client interface will not work because it is using the new ClientRequest object which is not raising the upgrade event. I'll try to do some more digging, but this seems like a very small header parsing issue.

[indexzero]

@3rd-Eden @ry

I spent some time reading the http-parser code with help from @bmeck and it seems that this is indeed a problem with the connection header parsing not detecting the "Upgrade" state because in Firefox 7 it sends

connection: keep-alive, Upgrade

where as in Chrome 13 and older versions of the spec the header is:

connection: Upgrade

Based on our first pass it seems like the parser enters the h_connection_keep_alive greedily (https://github.com/ry/http-parser/blob/master/http_parser.c#L1367-1377) and does not continue to check the contents of the header.

Not sure how simple a fix this is, but @bmeck will be putting together a low-level parser repro next week.

[ry]

please provide an example request that youve seen in the wild with connection: keep-alive, Upgrade and i'll fix the parser

[indexzero]

@ry Firefox 7 Websocket Requests. Seems to be consistent across Firefox 6 (draft7), Firefox 7 (draft10) and (reportedly) Chrome 14 (draft10).

More info in this comment: #97 (comment)

[indexzero]

Best way to repoduce this is in the wild is to open the simple chat example for socket.io that @3rd-Eden suggested in Firefox 6 or 7. I'm using a different socket.io sample app: http://github.com/fat/space-tweet. To run this just

  [Update config.json with valid Twitter credentials]
  npm install
  node server.js

Then you can throw a proxy server in front of it

  require('http-proxy').createServer(3000, 'localhost').listen(8000);

I found an interesting discussion on the ietf mailing list about a bug in the draft11 spec which confirms that comma-separated connection headers are in-fact supported by HTTP and used in builds of Firefox: http://www.ietf.org/mail-archive/web/hybi/current/msg08519.html

"Connection" header allows a list of tokens separated by comma. So the
following is valid (in fact Firefox 8 uses it):

Connection: keep-alive, upgrade

[3rd-Eden]

Well there seems to be 2 seperate issues then. Because Chrome beta and Chromium are sending the following headers:

Connection:Upgrade
Host:observer.no.de
Sec-WebSocket-Key:l18CXsxK2DurU2awtNQwKg==
Sec-WebSocket-Origin:http://observer.no.de
Sec-WebSocket-Version:8
Upgrade:websocket
(Key3):00:00:00:00:00:00:00:00

And they are not answered correctly by the proxy either. So it seems to me that the

connection: keep-alive, Upgrade

Isn't the only reason why node-http-proxy fails to proxy requests.

[indexzero]

Interesting. Again, in this case as you pointed out @3rd-Eden it is just that the upgrade event is not being emitted. This leads me to believe that this is not a bug in node-http-proxy, but perhaps a combination of small issues elsewhere in node.js core or it's dependencies.

[narup]

I wonder where is (Key3):00:00:00:00:00:00:00:00 coming from, I am trying to implement draft-ietf-hybi-thewebsocketprotocol-15 in jwebsocket.org. But keep getting disconnected. I am using chrome 14.0.835.163

[mmalecki]

I think I might know what causes this bug.

Websockets are messed up a bit in Chrome 14 (I remember that this was a real issue during node knockout), and Firefox's support for them is limited. As you can see, neither browser supports binary frames, while socket.io seems to be using it.

In fact, I was able to run socket.io chat demo with node-http-proxy and Chrome 15.0.874.15 dev (and yes, it was using Websockets transport - verified by socket.socket.transport in inspector and by looking at chrome://net-internals/#events).
Firefox 8.0a works partially. Connection breaks quite randomly, but that happens both with and without proxy, so I guess it isn't our problem.
Also, Chromium 13 works without any problems.

For me, it seems that the source of problem is that browsers are trying to use newest hybi specification, but don't actually implement it fully.

However, please note that I may be terribly wrong and YMMV.

[indexzero]

@mmalecki Thanks for the information, but @3rdeden has confirmed that socket.io@latest works on node stand-alone, but not behind node-http-proxy.

So the binary frames may be a red herring, but I will look into it.

[mmalecki]

After some further investigation with @AvianFlu:

Parser doesn't seem to be a problem. It detects upgrades just fine (and fires this callback). Proxy receives http requests like:

GET /socket.io/1/websocket/8800604461110870103 HTTP/1.1
Upgrade: WebSocket
Connection: Upgrade
Host: localhost:9000
Origin: http://localhost:9000
Sec-WebSocket-Key1: 4 00 q 9g1  94    83 0
Sec-WebSocket-Key2: &H 15453t 16A616

But fails to pass them further. This callback doesn't get executed at all.

Comparison between direct and proxied request - forwarded requests have their headers all in lower case. No forwarded request has an Upgrade header.

I'll investigate it further, but it's 4 AM and it's basically a brain dump.

[mmalecki]

After some further further investigation:

In this line we append a message to a queue and we hope that eventually (when socket is assigned) it'll be send to a server. In fact, socket gets assigned in the very first _cycle call. This calls a _flush method on this message, but flushing fails here, because of empty output and this request gets stuck.

Weirdest thing is: after some time proxyError gets called with 'socket hung up' message. And then this OutgoingMessage, which got stuck is send to a server (to be precise: OutgoingMessage._writeRaw is called - of course, server never receives it because of this socket error).

In fact, I can force this flush by inserting first.end() right after this line or reverseProxy.end() here, but it results in a following error from socket.io:

   debug - websocket writing 1::
connected
   warn  - websocket parser error: reserved fields not empty
   info  - transport end
   debug - set close timeout for client 6597731141049970547
   debug - cleared close timeout for client 6597731141049970547
   debug - cleared heartbeat interval for client 6597731141049970547
   debug - discarding transport
   warn  - websocket parser error: no handler for opcode 15

[AvianFlu]

The request gets stuck, as @mmalecki said, and then, since there's no error status, eventually https://github.com/joyent/node/blob/v0.4.12/lib/http.js#L1284-1293 is reached and the "socket hang up" message is received. The real key to fixing this lies in figuring out how to make it past https://github.com/joyent/node/blob/v0.4.12/lib/http.js#L753 to the socket.write() call at the end of that method.

[ry]

Do you guys have a simple test case yet? Put one together send it to me and I'll just fix it. You guys are thinking too hard.

A simple test case would be: tcpdump the req/res in question, set up a http server, have a tcp client connect to it - send the request as you got it from the tcpdump - assert what you expect to happen. See test/simple/test-http-* in the node tree for examples of test cases.

[indexzero]

@ry There really isn't a fully programmatic test case. Is that what you mean by a "simple test case". No node.js library (node-websocket-client, websocket-node, etc) correctly emulates how browsers have implemented WebSockets.

The easiest way to reproduce this right now is to

1. Write anything that uses socket.io (we have some simple test cases if you'd like to talk offline)
2. Put node-http-proxy in front of them
3. Open it in Firefox 7+ or Chrome 14+

[indexzero]

I can confirm that the fix from @indutny resolves this issue in his pull request. In both Firefox 7 and Chrome 14. I'm merging this into master.

[indexzero]

This should be fixed in 0.7.2. @3rd-Eden can you confirm?

[3rd-Eden]

@indexzero fix confirmed