feat(fetch): backport node-fetch's redirect bugfix (#77)

* Backport node-fetch redirect bugfix node-fetch PR that this work is based upon: node-fetch/node-fetch#1222 Co-authored-by: Jacob Ebey <[email protected]> * chore: fix changeset --------- Co-authored-by: Ben <[email protected]> Co-authored-by: Jacob Ebey <[email protected]>
web-std · Aug 28, 2023 · 86262a6 · 86262a6
1 parent fd274bd
commit 86262a6
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 18 deletions.
diff --git a/.changeset/folk-metal-music.md 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
@@ -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
@@ -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
@@ -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');