Skip to content

Commit

Permalink
More lenient gzip decompression (#239)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregory90 authored and TimothyGu committed Feb 27, 2017
1 parent 047799b commit a1e76b9
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 3 deletions.
15 changes: 12 additions & 3 deletions src/index.js
Expand Up @@ -141,9 +141,18 @@ export default function fetch(url, opts) {
return;
}

// Be less strict when decoding compressed responses, since sometimes
// servers send slightly invalid responses that are still accepted
// by common browsers.
// Always using Z_SYNC_FLUSH is what cURL does.

This comment has been minimized.

Copy link
@bitinn

bitinn Feb 27, 2017

Collaborator

I think these comments can be better, as it doesn't really explain what the code does... it simply says, browsers and curls are doing it so this is fine.

We should mention this is because node >= 6 update, using Z_SYNC_FLUSH makes zlib outputs response more digestible by Node gzip decoder.

const zlibOptions = {
flush: zlib.Z_SYNC_FLUSH,
finishFlush: zlib.Z_SYNC_FLUSH
};

// for gzip
if (codings == 'gzip' || codings == 'x-gzip') {
body = body.pipe(zlib.createGunzip());
body = body.pipe(zlib.createGunzip(zlibOptions));
resolve(new Response(body, response_options));
return;
}
Expand All @@ -156,9 +165,9 @@ export default function fetch(url, opts) {
raw.once('data', chunk => {
// see http://stackoverflow.com/questions/37519828
if ((chunk[0] & 0x0F) === 0x08) {
body = body.pipe(zlib.createInflate());
body = body.pipe(zlib.createInflate(zlibOptions));
} else {
body = body.pipe(zlib.createInflateRaw());
body = body.pipe(zlib.createInflateRaw(zlibOptions));

This comment has been minimized.

Copy link
@bitinn

bitinn Feb 27, 2017

Collaborator

I am not sure why the original fix (for request) changes both gzip and deflate options, as far as I know it's only necessary to do this for gzip stream.

This comment has been minimized.

Copy link
@TimothyGu

TimothyGu Feb 28, 2017

Collaborator

Heh, good point. I'll revert the change to createInflate() then

}
resolve(new Response(body, response_options));
});
Expand Down
10 changes: 10 additions & 0 deletions test/server.js
Expand Up @@ -70,6 +70,16 @@ export default class TestServer {
});
}

if (p === '/gzip-truncated') {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
res.setHeader('Content-Encoding', 'gzip');
zlib.gzip('hello world', function(err, buffer) {
// truncate the CRC checksum and size check at the end of the stream
res.end(buffer.slice(0, buffer.length - 8));
});
}

if (p === '/deflate') {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
Expand Down
11 changes: 11 additions & 0 deletions test/test.js
Expand Up @@ -513,6 +513,17 @@ describe('node-fetch', () => {
});
});

it('should decompress slightly invalid gzip response', function() {
url = `${base}gzip-truncated`;
return fetch(url).then(res => {
expect(res.headers.get('content-type')).to.equal('text/plain');
return res.text().then(result => {
expect(result).to.be.a('string');
expect(result).to.equal('hello world');
});
});
});

it('should decompress deflate response', function() {
url = `${base}deflate`;
return fetch(url).then(res => {
Expand Down

0 comments on commit a1e76b9

Please sign in to comment.