search

sockjs / sockjs-node

WebSocket emulation - Node.js server
http://sockjs.org
MIT License
2.1k stars 308 forks source link

xhr-polling protocol responses should have "no-transform" or a MIME type other than "application/javascript" #190

Open tombergan opened 9 years ago

tombergan commented 9 years ago

Responses to the "/xhr" and "/xhr_send" URLs have "Content-Type: application/javascript". Middlebox compression proxies may attempt to minimize these responses since they look like Javascript code. One typical optimization is newline removal, but this optimization breaks sockjs since xhr-polling uses "\n" as a frame delimiter. For example, see this bug report: https://code.google.com/p/chromium/issues/detail?id=553828

It is strange to use "Content-Type: application/javascript" since the xhr-polling responses are data, not executable javascript. This confuses compression proxies since the response is not being used as the Content-Type would suggest. The responses aren't even JSON (although I realize JSON can be embedded in the response). I believe sockjs should do one of the following:

1) Set "Cache-Control: no-transform" on the /xhr and /xhr_send responses. Looks like greggman just submitted a change to do exactly this, so you can probably just close this bug "already fixed" :-) See commit https://github.com/sockjs/sockjs-node/commit/7845866b0c1af24795f4a53cbf6a0e8f0468d7b5

2) Use a different Content-Type, such as "application/x-sockjs-xhr-polling" or event "text/plain".

greggman commented 9 years ago

Just for context Google's Data Saver does content aware data compression which is why this issue came up.

Basically like @tombergan said if it's not actually JSON it shouldn't be marked as JSON. Or, if it is JSON it should be treated as JSON and not have the whitespace be significant.

brycekahle commented 9 years ago

I know there was a very specific reason for application/javascript being used. Let me see if I can find references.

greggman commented 9 years ago

There's two seemingly obvious fixes (I haven't looked at the code). Either (a) stop marking it as JSON if it's not JSON or (b) stop looking for insignificant whitespace if it is JSON.

While my patch #189 sets the no-transform header which also solves the issue for the Google Data Saver there's no guarantee this issue won't come up with other content aware proxies who may or many not respect no-transform. So, it seems like either stop marking as JSON or stop checking for insignificant whitespace is a better solution.