Support for gzip compressed response

alltherooms / cached-request

Node.js module to perform HTTP requests with caching support

MIT License

61 stars 23 forks source link

Support for gzip compressed response #1

Closed slashdotdash closed 9 years ago

slashdotdash commented 9 years ago

Do not compress the cached response if it was already gzip compressed.

Applicable when the gzip: true request option is used and the server has gzip support.

Use temp module for managing the temporary test cache dir.

slashdotdash commented 9 years ago

Oops, didn't realise that deleting the branch on my forked copy also closed the PR.

@danypype please do take a look and merge if you are happy with the change.

danypype commented 9 years ago

Thank you, @slashdotdash for your contribution! I just have a comment before doing the merge:

If we are not going to gzip the response when it already comes gzipped, then we also have to make sure not to ungzip it (while responding from the cache) when the client is accepting a gzip compressed response.

Would you like to make this change?

slashdotdash commented 9 years ago

@danypype The cached response still needs to be run through zlib.createGunzip() as this is the behaviour of the request module. See https://github.com/request/request/blob/master/request.js#L1135 for details.

So regardless of whether the response from the server is compressed or not, it should be returned to the consumer uncompressed. At least this is what I am encountering when running request on its own.

danypype commented 9 years ago

I didn't know that at all! Thank you for the clarification @slashdotdash you're great. PR merged and landed on ae5322a