CORS pre-flight breaks socket.io behind load balancer

[dmkc]

I ran into an issue on our servers. We are running socket.io v1.0.6 on multiple server instances behind a load balancer. For polling, the requests go through the ELB with sticky sessions turned on. Our real-time service is on a subdomain, and thanks to CORS pre-flight requests, socket.io fucks up. Here is what happens on the client when the polling transport is used:

1. A socket.io handshake POST request occurs. The response comes back valid with an sid, and the headers include the AWS ELB cookie.
2. Next, a pre-flight OPTIONS request is made by the browser. The ELB cookie is not included by the browser here. As a result, the OPTIONS request is routed to a potentially different server which will not recognize the sid in the query string.
3. When the request is routed to the wrong server, socket.io responds with a 400 HTTP status code and an Session ID unknown error.
4. Since the pre-flight request fails, the browser also fails the actual GET polling request, and tries to re-do the handshake from the beginning
5. Possibly due to the headers being sent, the browser sends the OPTIONS pre-flight request fairly regularly as opposed to doing it only once, so this cycle repeats over and over.

The fix on our end currently is to respond to all OPTIONS requests with a 200 and all the usual Access-Control-Allow-… headers the browser knows and loves. We do this before they even get to socket.io in our nginx config.

Now, engine.io appears to already handle this case here: https://github.com/Automattic/engine.io/blob/master/lib/transports/polling-xhr.js#L40

However, that check is only reached if the sid is valid here: https://github.com/Automattic/engine.io/blob/master/lib/server.js#L180

which it isn't, of course. I can submit a PR but I'd like to know how you guys think it'd be best to handle this. AFAIK, if a request method is OPTIONS, we can make the assumption that we are polling. But, since we don't have a valid sid to look up a client by, this might mean moving fairly transport-specific logic into server.js which sounds less than ideal.

Thoughts?

[rauchg]

This is one of the best issue descriptions I've read in a while. Thanks for taking the time.

Can you send me the complete headers of the request that yields OPTIONS? Ideally we wouldn't need the pre-flight. I'm suspecting you're sending binary data which is resulting in a Content-Type switch?

Requests that do not need pre-flight according to MDN are:

A simple cross-site request is one that:

Only uses GET, HEAD or POST. If POST is used to send data to the server, the Content-Type of the data sent to the server with the HTTP POST request is one of application/x-www-form-urlencoded, multipart/form-data, or text/plain.
Does not set custom headers with the HTTP Request (such as X-Modified, etc.)

Before proceeding with the OPTIONS fix, I want to make sure it's happening for a good reason.

[mokesmokes]

Plus the performance hit and additional server load.... do what you can to avoid them as part of your connection.

[rauchg]

+1 :datfeel:

[defunctzombie]

There are two issues here. We should be supporting OPTIONS unconditionally so that even if the browser makes a pre-flight it doesn't break the existing session stickiness. And we should also see if we can make engine.io not do the pre-flight request at all.

[mokesmokes]

I think the default behavior should stay as today (break) since in 90% of cases it's unintended behavior and we don't want this happening silently. We can add a flag that if explicitly set the server sends OK on every OPTIONS request.

[rauchg]

Agreed with @mokesmokes

—
Guillermo Rauch – @rauchg https://twitter.com/rauchg

On Tue, Sep 2, 2014 at 9:14 PM, mokesmokes notifications@github.com wrote:

I think the default behavior should stay as today (break) since in 90% of
cases it's unintended behavior and we don't want this happening silently.
We can add a flag that if explicitly set the server sends OK on every
OPTIONS request.

—
Reply to this email directly or view it on GitHub
#279 (comment)
.

[dmkc]

@guille Thanks for looking into this so quickly! The Content-Type for the requests is indeed application/octet-stream. According to MDN that makes it a preflighted request. Hrm. Here are the complete headers:

Accept:*/*
Accept-Encoding:gzip,deflate
Accept-Language:en-US,en;q=0.8
Connection:keep-alive
Content-Length:65
Content-type:application/octet-stream
Cookie:io=Cu4ixu0L9h01xctVABFa; AWSELB=D92B29591C6D9DF48A890DD33BA2DB332633D5E9B0B9494FF3E6BBC16DECE6A0CB3F6B058AEAE907E103688331ECDF2318C3C96EE3035ABD6F0AAE7F5EE38DAA554DC79045
Host:subdomainof.awesome.internal.site.com
Origin:http://awesome.internal.site.com
Referer:http://awesome.internal.site.com
User-Agent:Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/37.0.2062.94 Safari/537.36

[defunctzombie]

Why don't we want this happening silently? I would think we want this to just work without messing about with settings.

[mokesmokes]

@defunctzombie because in many cases preflighted requests occur due to careless coding, and can be avoided. I think if they occur the dev should be fully aware that they are happening, not make it a silent default decision. Just my $0.02 :)

[dmkc]

@mokesmokes Here the preflight requests are occurring because of the request's content-type. Is choosing this content-type a decision made by socket.io? I am not configuring it anywhere explicitly. If that's the case then the default treatment of the OPTIONS req makes sense.

[defunctzombie]

We should fix this. OPTIONS preflights can happen when doing CORS stuff and this behavior is really annoying to an end user to deal with. It basically renders this module unusable behind the amazon ELB even when sticky sessions are on. I do not think this has anything to do with careless coding and is happening under the simplest uses of this module.

[defunctzombie]

For anyone that runs into this before we fix it. You can work around it by using the manual request handling (on request from the http server) and responding to OPTIONS yourself.

[mokesmokes]

Yup, it's not the user's fault, it's engine.io: https://github.com/Automattic/engine.io-client/blob/master/lib/transports/polling-xhr.js#L166
Binary transfers are the culprit. In this case wouldn't it just be better to encode/parse the text in engine.io rather than doing xhr.setRequestHeader('Content-type', 'application/octet-stream');? We really should avoid OPTIONS at all costs, it's a total performance killer and resource hog on the server.

[mokesmokes]

So basically what I think we need is:

1. OPTIONS turned off by default on engine.io server side
2. By default forceBase64 for XHR if client detects CORS scenario (WS is of course fine)
3. Add a forceBinaryXHRCors flag (default=false) which will disable (2) above, thus the browser will emit OPTIONS requests, and the developer will need to turn on the OPTIONS flag server-side.

So basically, by default all works smoothly and no OPTIONS as most people would probably prefer. If people insist on using octet-stream then they should manually configure this, but it should be supported. I really don't think we should just send binary data blindly in CORS scenarios.