From 86262a627927aa0d480e0739db2dea83a6e551be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20De=20Boey?= Date: Mon, 28 Aug 2023 23:22:57 +0200 Subject: [PATCH] feat(fetch): backport `node-fetch`'s redirect bugfix (#77) * Backport node-fetch redirect bugfix node-fetch PR that this work is based upon: https://github.com/node-fetch/node-fetch/pull/1222 Co-authored-by: Jacob Ebey * chore: fix changeset --------- Co-authored-by: Ben Co-authored-by: Jacob Ebey --- .changeset/folk-metal-music.md | 5 +++ packages/fetch/src/fetch.js | 54 +++++++++++++++++++---------- packages/fetch/test/main.js | 30 ++++++++++++++++ packages/fetch/test/utils/server.js | 26 ++++++++++++++ 4 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 .changeset/folk-metal-music.md diff --git a/.changeset/folk-metal-music.md b/.changeset/folk-metal-music.md new file mode 100644 index 0000000..86b9589 --- /dev/null +++ b/.changeset/folk-metal-music.md @@ -0,0 +1,5 @@ +--- +"@web-std/fetch": minor +--- + +Fixes redirects failing when response is chunked but empty. This is backported from https://github.com/node-fetch/node-fetch/pull/1222 diff --git a/packages/fetch/src/fetch.js b/packages/fetch/src/fetch.js index 6e5402f..20ebd21 100644 --- a/packages/fetch/src/fetch.js +++ b/packages/fetch/src/fetch.js @@ -324,29 +324,47 @@ async function fetch(url, options_ = {}) { * @param {(error:Error) => void} errorCallback */ function fixResponseChunkedTransferBadEnding(request, errorCallback) { - /** @type {import('net').Socket} */ - let socket; + const LAST_CHUNK = Buffer.from('0\r\n\r\n'); - request.on('socket', s => { - socket = s; - }); + let isChunkedTransfer = false; + let properLastChunkReceived = false; + /** @type {Buffer | undefined} */ + let previousChunk; request.on('response', response => { - const {headers} = response; + isChunkedTransfer = headers['transfer-encoding'] === 'chunked' && !headers['content-length']; + }); - if (headers['transfer-encoding'] === 'chunked' && !headers['content-length']) { - socket.prependListener('close', hadError => { - // if a data listener is still present we didn't end cleanly - const hasDataListener = socket.listenerCount('data') > 0; - if (hasDataListener && !hadError) { - const err = Object.assign(new Error('Premature close'), { - code: 'ERR_STREAM_PREMATURE_CLOSE' - }) - errorCallback(err); - } - }); - } + request.on('socket', socket => { + const onSocketClose = () => { + if (isChunkedTransfer && !properLastChunkReceived) { + const error = Object.assign(new Error('Premature close'), { + code: 'ERR_STREAM_PREMATURE_CLOSE' + }); + errorCallback(error); + } + }; + + socket.prependListener('close', onSocketClose); + + request.on('abort', () => { + socket.removeListener('close', onSocketClose); + }); + + socket.on('data', buf => { + properLastChunkReceived = Buffer.compare(buf.slice(-5), LAST_CHUNK) === 0; + + // Sometimes final 0-length chunk and end of message code are in separate packets + if (!properLastChunkReceived && previousChunk) { + properLastChunkReceived = ( + Buffer.compare(previousChunk.slice(-3), LAST_CHUNK.slice(0, 3)) === 0 && + Buffer.compare(buf.slice(-2), LAST_CHUNK.slice(3)) === 0 + ); + } + + previousChunk = buf; + }); }); } diff --git a/packages/fetch/test/main.js b/packages/fetch/test/main.js index ee4a8a7..96dc2cd 100644 --- a/packages/fetch/test/main.js +++ b/packages/fetch/test/main.js @@ -665,6 +665,36 @@ describe('node-fetch', () => { }); }); + it('should follow redirect after empty chunked transfer-encoding', () => { + const url = `${base}redirect/chunked`; + return fetch(url).then(res => { + expect(res.status).to.equal(200); + expect(res.ok).to.be.true; + }); + }); + + it('should handle chunked response with more than 1 chunk in the final packet', () => { + const url = `${base}chunked/multiple-ending`; + return fetch(url).then(res => { + expect(res.ok).to.be.true; + + return res.text().then(result => { + expect(result).to.equal('foobar'); + }); + }); + }); + + it('should handle chunked response with final chunk and EOM in separate packets', () => { + const url = `${base}chunked/split-ending`; + return fetch(url).then(res => { + expect(res.ok).to.be.true; + + return res.text().then(result => { + expect(result).to.equal('foobar'); + }); + }); + }); + it.skip('should handle DNS-error response', () => { const url = 'http://domain.invalid'; return expect(fetch(url)).to.eventually.be.rejected diff --git a/packages/fetch/test/utils/server.js b/packages/fetch/test/utils/server.js index 56066e7..2c28ab8 100644 --- a/packages/fetch/test/utils/server.js +++ b/packages/fetch/test/utils/server.js @@ -310,6 +310,14 @@ export default class TestServer { res.socket.end('\r\n'); } + if (p === '/redirect/chunked') { + res.writeHead(301, { + Location: '/inspect', + 'Transfer-Encoding': 'chunked' + }); + setTimeout(() => res.end(), 10); + } + if (p === '/error/400') { res.statusCode = 400; res.setHeader('Content-Type', 'text/plain'); @@ -357,6 +365,24 @@ export default class TestServer { }, 400); } + if (p === '/chunked/split-ending') { + res.socket.write('HTTP/1.1 200\r\nTransfer-Encoding: chunked\r\n\r\n'); + res.socket.write('3\r\nfoo\r\n3\r\nbar\r\n'); + + setTimeout(() => { + res.socket.write('0\r\n'); + }, 10); + + setTimeout(() => { + res.socket.end('\r\n'); + }, 20); + } + + if (p === '/chunked/multiple-ending') { + res.socket.write('HTTP/1.1 200\r\nTransfer-Encoding: chunked\r\n\r\n'); + res.socket.write('3\r\nfoo\r\n3\r\nbar\r\n0\r\n\r\n'); + } + if (p === '/error/json') { res.statusCode = 200; res.setHeader('Content-Type', 'application/json');