From 86d30054261befce7b8bf407ba8817bda1ba4a38 Mon Sep 17 00:00:00 2001 From: Travis Warlick Date: Mon, 26 Jul 2021 14:56:57 -0400 Subject: [PATCH 1/2] Fix redirect failing when response is chunked but empty. #1220 #1064 --- src/index.js | 8 +++++++- test/main.js | 8 ++++++++ test/utils/server.js | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 2cad269f9..9e1151bc7 100644 --- a/src/index.js +++ b/src/index.js @@ -303,12 +303,18 @@ function fixResponseChunkedTransferBadEnding(request, errorCallback) { properLastChunkReceived = Buffer.compare(buf.slice(-3), LAST_CHUNK) === 0; }); - socket.prependListener('close', () => { + const onSocketClose = () => { if (!properLastChunkReceived) { const error = new Error('Premature close'); error.code = 'ERR_STREAM_PREMATURE_CLOSE'; errorCallback(error); } + }; + + socket.prependListener('close', onSocketClose); + + request.on('abort', () => { + socket.removeListener('close', onSocketClose); }); } }); diff --git a/test/main.js b/test/main.js index c50860e2f..a777cf583 100644 --- a/test/main.js +++ b/test/main.js @@ -684,6 +684,14 @@ 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 DNS-error response', () => { const url = 'http://domain.invalid'; return expect(fetch(url)).to.eventually.be.rejected diff --git a/test/utils/server.js b/test/utils/server.js index b9ba35188..32aa070fe 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -297,6 +297,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'); From d19fdac797c9c62b1ca5b34c6968541522295ad1 Mon Sep 17 00:00:00 2001 From: Travis Warlick Date: Tue, 27 Jul 2021 21:12:07 -0400 Subject: [PATCH 2/2] Handle chunked responses where the final chunk and EOM code are in separate packets and where there is an additional data chunk in the same packet before the final chunk and EOM code. --- src/index.js | 53 ++++++++++++++++++++++++++------------------ test/main.js | 22 ++++++++++++++++++ test/utils/server.js | 18 +++++++++++++++ 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/src/index.js b/src/index.js index 9e1151bc7..d906fffa8 100644 --- a/src/index.js +++ b/src/index.js @@ -287,35 +287,44 @@ export default async function fetch(url, options_) { } function fixResponseChunkedTransferBadEnding(request, errorCallback) { - const LAST_CHUNK = Buffer.from('0\r\n'); - let socket; + const LAST_CHUNK = Buffer.from('0\r\n\r\n'); - request.on('socket', s => { - socket = s; - }); + let isChunkedTransfer = false; + let properLastChunkReceived = false; + let previousChunk; request.on('response', response => { const {headers} = response; - if (headers['transfer-encoding'] === 'chunked' && !headers['content-length']) { - let properLastChunkReceived = false; + isChunkedTransfer = headers['transfer-encoding'] === 'chunked' && !headers['content-length']; + }); - socket.on('data', buf => { - properLastChunkReceived = Buffer.compare(buf.slice(-3), LAST_CHUNK) === 0; - }); + request.on('socket', socket => { + const onSocketClose = () => { + if (isChunkedTransfer && !properLastChunkReceived) { + const error = new Error('Premature close'); + error.code = 'ERR_STREAM_PREMATURE_CLOSE'; + errorCallback(error); + } + }; - const onSocketClose = () => { - if (!properLastChunkReceived) { - const error = new Error('Premature close'); - error.code = 'ERR_STREAM_PREMATURE_CLOSE'; - errorCallback(error); - } - }; + socket.prependListener('close', onSocketClose); - socket.prependListener('close', onSocketClose); + request.on('abort', () => { + socket.removeListener('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/test/main.js b/test/main.js index a777cf583..1e1f368c3 100644 --- a/test/main.js +++ b/test/main.js @@ -692,6 +692,28 @@ describe('node-fetch', () => { }); }); + 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('should handle DNS-error response', () => { const url = 'http://domain.invalid'; return expect(fetch(url)).to.eventually.be.rejected diff --git a/test/utils/server.js b/test/utils/server.js index 32aa070fe..9bfe1c5af 100644 --- a/test/utils/server.js +++ b/test/utils/server.js @@ -352,6 +352,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');