spdy-http2 / node-spdy

SPDY server on Node.js
2.81k stars 196 forks source link

SPDY removes Accept-Encoding when browsing with Firefox #199

Closed hdf closed 9 years ago

hdf commented 9 years ago

Coming here from: https://github.com/expressjs/compression/issues/38. Seems, that it all works fine with Chrome, but with Firefox, the Accept-Encoding header is dropped. Note that Chrome sends accept-encoding. Could the case difference be a problem?

hdf commented 9 years ago

@dougwilson has figured out the problem: Seems Firefox is not actually sending Accept-Encoding header, but is lying about it in the developer console. The current solution, that Apache and Nginx is using, (and that node-spdy should be as well) is to check if Firefox is connecting trough SPDY and if so, add the header Accept-Encoding: gzip, deflate.

Here is the issue report at Firefox: https://support.mozilla.org/en-US/questions/995095 At nginx: http://trac.nginx.org/nginx/ticket/542 And at Apache: https://issues.apache.org/jira/browse/TS-3026

indutny commented 9 years ago

@hdf what do you suggest?

hdf commented 9 years ago

For now, I'm using this in my project, probably a few more checks should be present in a more general solution(, like check if Firefox).

app.use(function(req, res, next) {
  if (!req.headers['accept-encoding']) {
    req.headers['accept-encoding'] = 'gzip, deflate';
    req.rawHeaders.push('accept-encoding');
    req.rawHeaders.push('gzip, deflate');
  }
  next();
});
dougwilson commented 9 years ago

@hdf that's exactly what Apache and nginx are both doing. They don't bother with the Firefox check, though, since the value is a given over a SPDY connection (which is why Firefox is not including it).

hdf commented 9 years ago

Any browser, that can handle SPDY, can probably do gzip, so I'm not too worried about that check I guess... And from here on, it's all SPDY for me, so... :)

indutny commented 9 years ago

Ok then :) I guess we may close the issue now?

dougwilson commented 9 years ago

I believe the suggestion is for node-spdy to add the header if not present, to follow Apache and nginx's SPDY implementations.

hdf commented 9 years ago

Yes, that's what I said in my second comment. This is just the solution I use in my project, until node-spdy deals with this itself. As I believe it should.

indutny commented 9 years ago

@dougwilson this is a firefox bug, right? If so - I don't see why it should be fixed anywhere except the firefox itself.

hdf commented 9 years ago

Normally, I would agree, but this bug is over a year old now, it looks like it's going to stay... And all other server side SPDY vendors use this fix, so node-spdy should as well, to follow suit I guess.

indutny commented 9 years ago

@hdf : let's wait for https://twitter.com/indutny/status/589859637618434048 first.

dougwilson commented 9 years ago

Further research has turned up why Firefox doesn't send the header: https://bugzilla.mozilla.org/show_bug.cgi?id=779413

Apparently compressing over a SSL/TLS connection makes your traffic vulnerable to the CRIME attack.

Edit: nevermind, that's specific to only request header compression, not body, so it wouldn't explain the lack of the accept-endoing request header.

indutny commented 9 years ago

@hdf @dougwilson I'm a bit afraid to break non-browser code. SPDY might be used for other purposes, and assuming some headers doesn't seem right to me.

Speaking of particular use cases - we do use it at Voxer for mobile-server communication. I doubt that it'll break our code if I'll add this header, but I'd rather not anyway.

dougwilson commented 9 years ago

@indutny according to http://mbelshe.github.io/SPDY-Specification/draft-mbelshe-spdy-00.xml (section 3.2.1):

User-agents MUST support gzip compression. Regardless of the Accept-Encoding sent by the user-agent, the server may always send content encoded with gzip or deflate encoding.

mcmanus commented 9 years ago

hi All - this is not a bug. The server should safely assume that all spdy transactions are gzip capable: http://dev.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3-1: "User-agents MUST support gzip compression. Regardless of the Accept-Encoding sent by the user-agent, the server may always send content encoded with gzip or deflate encoding."

to include the header is a waste of bytes. chrome didn't use to include it either, but I suspect their spdy stack merged with their h2 stack (and h2 requires it to be sent) - the stacks are separate in firefox.

and devtools isn't lying to you - its just giving you an h1 semantic view of things, because that's what people understand.

indutny commented 9 years ago

@mcmanus thank you!

Ok guys, I'm now convinced about it :) Will you be interested in sending a PR for this?