fix(Redirect): Better handle wrong redirect header in a response (#1387)

* Fixed crash when an invalid Location URL is returned from a redirect. Fixes #1386

* CHANGELOG entry

* changed catch(e) -> catch(error) to match rest of code, added comment

Co-authored-by: Linus Unnebäck <linus@folkdatorn.se>

* suppress error on invalid redirect URL when options.redirect == manual

Co-authored-by: Tasos Bitsios <tasos.bitsios@import.io>
Co-authored-by: Linus Unnebäck <linus@folkdatorn.se>

docs/CHANGELOG.md

@@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+* fix(request): fix crash when an invalid redirection URL is encountered https://github.com/node-fetch/node-fetch/pull/1387
 * fix: handle errors from the request body stream by @mdmitry01 in https://github.com/node-fetch/node-fetch/pull/1392
 
 ## 3.1.0

src/index.js

@@ -130,7 +130,19 @@ export default async function fetch(url, options_) {
 				const location = headers.get('Location');
 
 				// HTTP fetch step 5.3
-				const locationURL = location === null ? null : new URL(location, request.url);
+				let locationURL = null;
+				try {
+					locationURL = location === null ? null : new URL(location, request.url);
+				} catch {
+					// error here can only be invalid URL in Location: header
+					// do not throw when options.redirect == manual
+					// let the user extract the errorneous redirect URL
+					if (request.redirect !== 'manual') {
+						reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect'));
+						finalize();
+						return;
+					}
+				}
 
 				// HTTP fetch step 5.5
 				switch (request.redirect) {

test/main.js

@@ -527,6 +527,28 @@ describe('node-fetch', () => {
 		});
 	});
 
+	it('should process an invalid redirect (manual)', () => {
+		const url = `${base}redirect/301/invalid`;
+		const options = {
+			redirect: 'manual'
+		};
+		return fetch(url, options).then(res => {
+			expect(res.url).to.equal(url);
+			expect(res.status).to.equal(301);
+			expect(res.headers.get('location')).to.equal('//super:invalid:url%/');
+		});
+	});
+
+	it('should throw an error on invalid redirect url', () => {
+		const url = `${base}redirect/301/invalid`;
+		return fetch(url).then(() => {
+			expect.fail();
+		}, error => {
+			expect(error).to.be.an.instanceof(FetchError);
+			expect(error.message).to.equal('uri requested responds with an invalid redirect URL: //super:invalid:url%/');
+		});
+	});
+
 	it('should throw a TypeError on an invalid redirect option', () => {
 		const url = `${base}redirect/301`;
 		const options = {

test/utils/server.js

@@ -239,6 +239,12 @@ export default class TestServer {
 			res.end();
 		}
 
+		if (p === '/redirect/301/invalid') {
+			res.statusCode = 301;
+			res.setHeader('Location', '//super:invalid:url%/');
+			res.end();
+		}
+
 		if (p === '/redirect/302') {
 			res.statusCode = 302;
 			res.setHeader('Location', '/inspect');