fix: installation failed due to installation link redirection (v11) (#10286)

* fix: installation failed due to installation link redirection

* fix: handle all different cases of redirect locations

* docs: update changesets

* refactor: implement CR suggestion

---------

Co-authored-by: Zoltan Kochan <z@kochan.io>

.changeset/funny-melons-pay.md

@@ -0,0 +1,5 @@
+---
+"@pnpm/fetch": patch
+---
+
+When the node-fetch request redirects an installation link and returns a relative path, URL parsing may fail [#10286](https://github.com/pnpm/pnpm/pull/10286).

network/fetch/src/fetchFromRegistry.ts

@@ -81,10 +81,10 @@ export function createFetchFromRegistry (defaultOpts: CreateFetchFromRegistryOpt
         return response
       }
 
+      redirects++
       // This is a workaround to remove authorization headers on redirect.
       // Related pnpm issue: https://github.com/pnpm/pnpm/issues/1815
-      redirects++
-      urlObject = new URL(response.headers.get('location')!)
+      urlObject = resolveRedirectUrl(response, urlObject)
       if (!headers['authorization'] || originalHost === urlObject.host) continue
       delete headers.authorization
     }
@@ -116,3 +116,11 @@ function getHeaders (
   }
   return headers
 }
+
+function resolveRedirectUrl (response: Response, currentUrl: URL): URL {
+  const location = response.headers.get('location')
+  if (!location) {
+    throw new Error(`Redirect location header missing for ${currentUrl.toString()}`)
+  }
+  return new URL(location, currentUrl)
+}

network/fetch/test/fetchFromRegistry.test.ts

@@ -7,6 +7,10 @@ import fs from 'fs'
 
 const CERTS_DIR = path.join(import.meta.dirname, '__certs__')
 
+afterEach(() => {
+  nock.cleanAll()
+})
+
 test('fetchFromRegistry', async () => {
   const fetchFromRegistry = createFetchFromRegistry({})
   const res = await fetchFromRegistry('https://registry.npmjs.org/is-positive')
@@ -138,3 +142,48 @@ test('fail if the client certificate is not provided', async () => {
   }
   expect(err?.code).toMatch(/ECONNRESET|ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED/)
 })
+
+test('redirect to protocol-relative URL', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/foo')
+    .reply(302, '', { location: '//registry.other.org/foo' })
+  nock('http://registry.other.org/')
+    .get('/foo')
+    .reply(200, { ok: true })
+
+  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  const res = await fetchFromRegistry(
+    'http://registry.pnpm.io/foo'
+  )
+
+  expect(await res.json()).toStrictEqual({ ok: true })
+  expect(nock.isDone()).toBeTruthy()
+})
+
+test('redirect to relative URL', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/bar/baz')
+    .reply(302, '', { location: '../foo' })
+  nock('http://registry.pnpm.io/')
+    .get('/foo')
+    .reply(200, { ok: true })
+
+  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  const res = await fetchFromRegistry(
+    'http://registry.pnpm.io/bar/baz'
+  )
+
+  expect(await res.json()).toStrictEqual({ ok: true })
+  expect(nock.isDone()).toBeTruthy()
+})
+
+test('redirect without location header throws error', async () => {
+  nock('http://registry.pnpm.io/')
+    .get('/missing-location')
+    .reply(302, 'found')
+
+  const fetchFromRegistry = createFetchFromRegistry({ fullMetadata: true })
+  await expect(fetchFromRegistry(
+    'http://registry.pnpm.io/missing-location'
+  )).rejects.toThrow(/Redirect location header missing/)
+})