From b79fb8c601ee6656472db5116b395c8221e36650 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Thu, 18 Nov 2021 06:52:24 +0100 Subject: [PATCH 1/7] Fixed crash when an invalid Location URL is returned from a redirect. Fixes #1386 --- src/index.js | 9 ++++++++- test/main.js | 10 ++++++++++ test/utils/server.js | 6 ++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index c98861eda..9b45d2309 100644 --- a/src/index.js +++ b/src/index.js @@ -130,7 +130,14 @@ 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; + try { + locationURL = location === null ? null : new URL(location, request.url); + } catch (e) { + reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect')); + finalize(); + return; + } // HTTP fetch step 5.5 switch (request.redirect) { diff --git a/test/main.js b/test/main.js index dc4198d75..56167b5c2 100644 --- a/test/main.js +++ b/test/main.js @@ -527,6 +527,16 @@ describe('node-fetch', () => { }); }); + it('should throw an error an 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 = { diff --git a/test/utils/server.js b/test/utils/server.js index 2a1e8e9b0..351a3cd73 100644 --- a/test/utils/server.js +++ b/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'); From edd3712ef20d3d59ad0480e8727c8b8f10e5e353 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Thu, 18 Nov 2021 06:55:43 +0100 Subject: [PATCH 2/7] CHANGELOG entry --- docs/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b3c987623..ea025740b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes will be recorded here. 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). +* fix(request): fix crash when an invalid redirection URL is encountered + ## 3.1.0 ## What's Changed From c466bba40ee3f09ace928764983ce98095745253 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Thu, 18 Nov 2021 06:56:07 +0100 Subject: [PATCH 3/7] CHANGELOG added link to PR --- docs/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ea025740b..445852b44 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes will be recorded here. 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). -* fix(request): fix crash when an invalid redirection URL is encountered +* fix(request): fix crash when an invalid redirection URL is encountered https://github.com/node-fetch/node-fetch/pull/1387 ## 3.1.0 From 33fe0279be0f9bf6b352a4687bfb80341bcfb01d Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Thu, 18 Nov 2021 06:57:40 +0100 Subject: [PATCH 4/7] changed catch(e) -> catch(error) to match rest of code, added comment --- src/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 9b45d2309..ab60725eb 100644 --- a/src/index.js +++ b/src/index.js @@ -133,7 +133,8 @@ export default async function fetch(url, options_) { let locationURL; try { locationURL = location === null ? null : new URL(location, request.url); - } catch (e) { + } catch (error) { + // error can only be invalid URL reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect')); finalize(); return; From 788d25b7422d93868ca6119ad524f45682d959e9 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Thu, 18 Nov 2021 20:24:06 +0100 Subject: [PATCH 5/7] linting --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index ab60725eb..fade2e2f8 100644 --- a/src/index.js +++ b/src/index.js @@ -133,7 +133,7 @@ export default async function fetch(url, options_) { let locationURL; try { locationURL = location === null ? null : new URL(location, request.url); - } catch (error) { + } catch (_error) { // error can only be invalid URL reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect')); finalize(); From 60be055663429978a73cf0a620a7601a59094935 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Mon, 22 Nov 2021 16:20:15 +0100 Subject: [PATCH 6/7] linting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Linus Unnebäck --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index fade2e2f8..009140177 100644 --- a/src/index.js +++ b/src/index.js @@ -133,7 +133,7 @@ export default async function fetch(url, options_) { let locationURL; try { locationURL = location === null ? null : new URL(location, request.url); - } catch (_error) { + } catch { // error can only be invalid URL reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect')); finalize(); From cf8c6b02fb46ed219c34f7e7d3bb864439fe9134 Mon Sep 17 00:00:00 2001 From: Tasos Bitsios Date: Mon, 22 Nov 2021 16:35:38 +0100 Subject: [PATCH 7/7] suppress error on invalid redirect URL when options.redirect == manual --- src/index.js | 14 +++++++++----- test/main.js | 14 +++++++++++++- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/index.js b/src/index.js index 009140177..57cef2053 100644 --- a/src/index.js +++ b/src/index.js @@ -130,14 +130,18 @@ export default async function fetch(url, options_) { const location = headers.get('Location'); // HTTP fetch step 5.3 - let locationURL; + let locationURL = null; try { locationURL = location === null ? null : new URL(location, request.url); } catch { - // error can only be invalid URL - reject(new FetchError(`uri requested responds with an invalid redirect URL: ${location}`, 'invalid-redirect')); - finalize(); - return; + // 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 diff --git a/test/main.js b/test/main.js index 56167b5c2..82938a034 100644 --- a/test/main.js +++ b/test/main.js @@ -527,7 +527,19 @@ describe('node-fetch', () => { }); }); - it('should throw an error an invalid redirect url', () => { + 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();