Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request does not support 'deflate' encoding. #2171

Closed
czardoz opened this issue Apr 15, 2016 · 5 comments
Closed

Request does not support 'deflate' encoding. #2171

czardoz opened this issue Apr 15, 2016 · 5 comments

Comments

@czardoz
Copy link
Contributor

czardoz commented Apr 15, 2016

This request results in the raw response being dumped to the console:

var req = require('request');

req({
    url: 'http://httpbin.org/deflate',
    method: 'GET',
    headers: {},
    followAllRedirects: false,
    followRedirect: true,
    jar: true,
    timeout: 15000,
    gzip: true,
    rejectUnauthorized: false,
    strictSSL: true
}, function (err, res, body) {
    if (err) {
        console.error(err.stack || err);
    }
    console.log(body);
});

The expected behavior is that body contains the decoded response.

@czardoz
Copy link
Contributor Author

czardoz commented Apr 15, 2016

Changing the gzip handling to

      if (contentEncoding === 'gzip') {
        responseContent = zlib.createGunzip()
        response.pipe(responseContent)
      } else if (contentEncoding === 'deflate') {
        responseContent = zlib.createInflate()
        response.pipe(responseContent)
      }

in request.js ensures that deflate responses work.

I'm willing to submit a PR for this change, but are there any other considerations here?

@simov
Copy link
Member

simov commented Apr 15, 2016

That's weird, I though we support it already. I'm wondering if the gzip option would be appropriate in this case though.

@czardoz
Copy link
Contributor Author

czardoz commented Apr 15, 2016

True, gzip is probably not the right option. But I don't think it is nice to add a new option for every new encoding that comes up either.

One solution is to use the gzip option, since deflate and gzip are related. Not sure if this is backwards compatible though.

Another option is to include deflate as a new option, this will lead to a bit of code duplication, and too many options, I think.

I'm okay with either of the above options though (or any that I might not have thought of) :)

@simov
Copy link
Member

simov commented Apr 15, 2016

Yep let's keep it under gzip for now. I have a better approach for handling the options here.

Don't forget to add a test in the test-gzip file.

@czardoz
Copy link
Contributor Author

czardoz commented Apr 15, 2016

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants