add option to specify available encodings #27
Conversation
|
+1 |
| @@ -157,7 +158,7 @@ function compression(options) { | |||
|
|
|||
| // compression method | |||
| var accept = accepts(req) | |||
| var method = accept.encoding(['gzip', 'deflate', 'identity']) | |||
| var method = accept.encoding(available) | |||
|
|
|||
| // negotiation failed | |||
| if (!method || method === 'identity') { | |||
There was a problem hiding this comment.
Correct me if I'm wrong, but I think it always needs to have the identity fallback.
There was a problem hiding this comment.
right
if you want disable deflate algorithm, you should use ['gzip', 'identity']
There was a problem hiding this comment.
Right. I'm not sure if the interface should make the user include identity
There was a problem hiding this comment.
before force identity, people should check the presence of their algorithms inside this module
worst case? ciao like a new superpowerful algorithm :)
['ciao', 'identity'] as option
ab -v 4 -H "Accept-Encoding: ciao" -c 1 -n 1 127.0.0.1:3000/with_compression
LOG: header received:
HTTP/1.1 200 OK
Content-Type: text/plain
Vary: Accept-Encoding
Content-Encoding: ciao
Date: Mon, 26 Jan 2015 18:59:20 GMT
Connection: closesent with deflate algorithm https://github.com/expressjs/compression/blob/master/index.js#L172
There was a problem hiding this comment.
@hex7c0 , right, that is another case that needs to be fixed and why this PR hasn't been merged just yet :)
dougwilson
force-pushed
the
master
branch
3 times, most recently
from
d7bb81b
to
cd957aa
Compare
|
The way forward for this functionality would be as described in #25 (comment), which would bring breaking changes, so we need to wait for a future major release to launch it. |
closes #25