From e4d1b4db2c95489f050545ff40996c00e7f71620 Mon Sep 17 00:00:00 2001 From: Phil Pluckthun Date: Sun, 1 Feb 2026 23:09:27 +0000 Subject: [PATCH] fix: Handle invalid `Location` headers for redirects (#26) --- .changeset/fresh-states-punch.md | 5 +++++ src/__tests__/fetch.test.ts | 27 +++++++++++++++++---------- src/fetch.ts | 24 ++++++++++++++++++------ 3 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 .changeset/fresh-states-punch.md diff --git a/.changeset/fresh-states-punch.md b/.changeset/fresh-states-punch.md new file mode 100644 index 0000000..b5fb392 --- /dev/null +++ b/.changeset/fresh-states-punch.md @@ -0,0 +1,5 @@ +--- +'fetch-nodeshim': patch +--- + +Protect against invalid `Location` URI diff --git a/src/__tests__/fetch.test.ts b/src/__tests__/fetch.test.ts index 9797b9b..c9fdd29 100644 --- a/src/__tests__/fetch.test.ts +++ b/src/__tests__/fetch.test.ts @@ -290,16 +290,23 @@ describe(fetch, () => { }); }); - it.each([['follow'], ['manual']] as const)( - 'should treat broken redirect as ordinary response (%s)', - async redirect => { - const response = await fetch(new URL('redirect/no-location', baseURL), { - redirect, - }); - expect(response.status).toBe(301); - expect(response.headers.has('location')).toBe(false); - } - ); + it('should treat broken redirect as ordinary response for redirect: "manual"', async () => { + const response = await fetch(new URL('redirect/no-location', baseURL), { + redirect: 'manual', + }); + expect(response.status).toBe(301); + expect(response.headers.has('location')).toBe(false); + }); + + it('should throw on broken redirects for redirect: "follow"', async () => { + await expect(() => + fetch(new URL('redirect/no-location', baseURL), { + redirect: 'follow', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `[Error: URI requested responds with an invalid redirect URL]` + ); + }); it('should throw a TypeError on an invalid redirect option', async () => { await expect(() => diff --git a/src/fetch.ts b/src/fetch.ts index e2cf863..6fefc12 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -11,6 +11,14 @@ import { getHttpsAgent, getHttpAgent } from './agent'; /** Maximum allowed redirects (matching Chromium's limit) */ const MAX_REDIRECTS = 20; +const parseURL = (input: string, base?: string | URL): URL | null => { + try { + return new URL(input, base); + } catch { + return null; + } +}; + /** Convert Node.js raw headers array to Headers */ const headersOfRawHeaders = (rawHeaders: readonly string[]): Headers => { const headers = new Headers(); @@ -186,19 +194,23 @@ async function _fetch( if (isRedirectCode(init.status)) { const location = init.headers.get('Location'); const locationURL = - location != null ? new URL(location, requestUrl) : null; + location != null ? parseURL(location, requestUrl) : null; if (redirect === 'error') { - // TODO: do we need a special Error instance here? reject( new Error( 'URI requested responds with a redirect, redirect mode is set to error' ) ); return; - } else if (redirect === 'manual' && locationURL !== null) { - init.headers.set('Location', locationURL.toString()); - } else if (redirect === 'follow' && locationURL !== null) { - if (++redirects > MAX_REDIRECTS) { + } else if (redirect === 'manual' && location) { + init.headers.set('Location', locationURL?.href ?? location); + } else if (redirect === 'follow') { + if (locationURL === null) { + reject( + new Error('URI requested responds with an invalid redirect URL') + ); + return; + } else if (++redirects > MAX_REDIRECTS) { reject(new Error(`maximum redirect reached at: ${requestUrl}`)); return; } else if ( -- 2.51.2