fix: detect and prevent redirect loops with Client/Pool (#4361)

Author: mcollina

lib/handler/redirect-handler.js

@@ -133,6 +133,16 @@ class RedirectHandler {
     const { origin, pathname, search } = util.parseURL(new URL(this.location, this.opts.origin && new URL(this.opts.path, this.opts.origin)))
     const path = search ? `${pathname}${search}` : pathname
 
+    // Check for redirect loops by seeing if we've already visited this URL in our history
+    // This catches the case where Client/Pool try to handle cross-origin redirects but fail
+    // and keep redirecting to the same URL in an infinite loop
+    const redirectUrlString = `${origin}${path}`
+    for (const historyUrl of this.history) {
+      if (historyUrl.toString() === redirectUrlString) {
+        throw new InvalidArgumentError(`Redirect loop detected. Cannot redirect to ${origin}. This typically happens when using a Client or Pool with cross-origin redirects. Use an Agent for cross-origin redirects.`)
+      }
+    }
+
     // Remove headers referring to the original URL.
     // By default it is Host only, unless it's a 303 (see below), which removes also all Content-* headers.
     // https://tools.ietf.org/html/rfc7231#section-6.4

test/interceptors/redirect-cross-origin-fix.js

@@ -0,0 +1,123 @@
+'use strict'
+
+const { test, after } = require('node:test')
+const { createServer } = require('node:http')
+const { once } = require('node:events')
+const { tspl } = require('@matteo.collina/tspl')
+const undici = require('../..')
+
+const {
+  interceptors: { redirect }
+} = undici
+
+test('Client should throw redirect loop error for cross-origin redirect', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const serverA = createServer((req, res) => {
+    res.writeHead(301, {
+      Location: 'http://localhost:9999/target' // Different port = cross-origin
+    })
+    res.end()
+  })
+
+  serverA.listen(0)
+  after(() => serverA.close())
+  await once(serverA, 'listening')
+
+  const client = new undici.Client(`http://localhost:${serverA.address().port}`).compose(
+    redirect({ maxRedirections: 2 })  // Keep low to avoid long waits
+  )
+  after(() => client.close())
+
+  try {
+    await client.request({
+      method: 'GET',
+      path: '/test'
+    })
+    t.fail('Expected error but request succeeded')
+  } catch (error) {
+    t.ok(error.message.includes('Redirect loop detected'), 'Error message indicates redirect loop')
+    t.ok(error.message.includes('Client or Pool'), 'Error message mentions Client or Pool')
+  }
+
+  await t.completed
+})
+
+test('Pool should throw redirect loop error for cross-origin redirect', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const serverA = createServer((req, res) => {
+    res.writeHead(301, {
+      Location: 'http://localhost:9998/target' // Different port = cross-origin
+    })
+    res.end()
+  })
+
+  serverA.listen(0)
+  after(() => serverA.close())
+  await once(serverA, 'listening')
+
+  const pool = new undici.Pool(`http://localhost:${serverA.address().port}`).compose(
+    redirect({ maxRedirections: 2 })  // Keep low to avoid long waits
+  )
+  after(() => pool.close())
+
+  try {
+    await pool.request({
+      method: 'GET',
+      path: '/test'
+    })
+    t.fail('Expected error but request succeeded')
+  } catch (error) {
+    t.ok(error.message.includes('Redirect loop detected'), 'Error message indicates redirect loop')
+    t.ok(error.message.includes('Client or Pool'), 'Error message mentions Client or Pool')
+  }
+
+  await t.completed
+})
+
+test('Agent should successfully follow cross-origin redirect', async (t) => {
+  t = tspl(t, { plan: 2 })
+
+  const serverB = createServer((req, res) => {
+    res.writeHead(200)
+    res.end('Cross-origin redirect success')
+  })
+
+  const serverA = createServer((req, res) => {
+    res.writeHead(301, {
+      Location: `http://localhost:${serverB.address().port}/success`
+    })
+    res.end()
+  })
+
+  serverA.listen(0)
+  serverB.listen(0)
+  after(() => {
+    serverA.close()
+    serverB.close()
+  })
+
+  await Promise.all([
+    once(serverA, 'listening'),
+    once(serverB, 'listening')
+  ])
+
+  const agent = new undici.Agent().compose(
+    redirect({ maxRedirections: 2 })
+  )
+  after(() => agent.close())
+
+  const response = await agent.request({
+    origin: `http://localhost:${serverA.address().port}`,
+    method: 'GET',
+    path: '/test'
+  })
+
+  const body = await response.body.text()
+
+  t.strictEqual(response.statusCode, 200, 'Response has 200 status code')
+  t.ok(body.includes('Cross-origin redirect success'), 'Response body indicates successful cross-origin redirect')
+
+  await t.completed
+})