add cross-origin redirect header stripping

Author: mcollina

docs/docs/api/Dispatcher.md

@@ -995,7 +995,8 @@ Options:
 
 - **maxRedirections** `number` - Maximum number of redirections allowed.
 - **throwOnMaxRedirect** `boolean` - Throw when the maximum number of redirections is reached.
-- **stripHeadersOnRedirect** `string[]` - Header names to remove from redirected requests.
+- **stripHeadersOnRedirect** `string[]` - Header names to remove from all redirected requests.
+- **stripHeadersOnCrossOriginRedirect** `string[]` - Header names to remove from cross-origin redirected requests.
 
 **Example - Basic Redirect Interceptor**
 

docs/docs/api/RedirectHandler.md

@@ -8,7 +8,7 @@ Arguments:
 
 - **dispatch** `function` - The dispatch function to be called after every retry.
 - **maxRedirections** `number` - Maximum number of redirections allowed.
-- **opts** `object` - Options for handling redirection. Supports `throwOnMaxRedirect` and `stripHeadersOnRedirect`.
+- **opts** `object` - Options for handling redirection. Supports `throwOnMaxRedirect`, `stripHeadersOnRedirect`, and `stripHeadersOnCrossOriginRedirect`.
 - **handler** `object` - An object containing handlers for different stages of the request lifecycle.
 
 Returns: `RedirectHandler`
@@ -19,7 +19,8 @@ Returns: `RedirectHandler`
 - **maxRedirections** `number` (required) - Maximum number of redirections allowed.
 - **opts** `object` (required) - Options for handling redirection.
   - **throwOnMaxRedirect** `boolean` - Throw when the maximum number of redirections is reached.
-  - **stripHeadersOnRedirect** `string[]` - Header names to remove from redirected requests.
+  - **stripHeadersOnRedirect** `string[]` - Header names to remove from all redirected requests.
+  - **stripHeadersOnCrossOriginRedirect** `string[]` - Header names to remove from cross-origin redirected requests.
 - **handler** `object` (required) - Handlers for different stages of the request lifecycle.
 
 ### Properties

lib/handler/redirect-handler.js

@@ -27,16 +27,13 @@ class RedirectHandler {
       throw new InvalidArgumentError('throwOnMaxRedirect must be a boolean')
     }
 
-    if (opts.stripHeadersOnRedirect != null && !Array.isArray(opts.stripHeadersOnRedirect)) {
-      throw new InvalidArgumentError('stripHeadersOnRedirect must be an array')
-    }
-
     this.dispatch = dispatch
     this.location = null
-    const { maxRedirections: _, stripHeadersOnRedirect, ...cleanOpts } = opts
+    const { maxRedirections: _, stripHeadersOnRedirect, stripHeadersOnCrossOriginRedirect, ...cleanOpts } = opts
     this.opts = cleanOpts // opts must be a copy, exclude maxRedirections
     this.opts.body = util.wrapRequestBody(this.opts.body)
-    this.stripHeadersOnRedirect = normalizeStripHeadersOnRedirect(stripHeadersOnRedirect)
+    this.stripHeadersOnRedirect = normalizeStripHeaders(stripHeadersOnRedirect, 'stripHeadersOnRedirect')
+    this.stripHeadersOnCrossOriginRedirect = normalizeStripHeaders(stripHeadersOnCrossOriginRedirect, 'stripHeadersOnCrossOriginRedirect')
     this.maxRedirections = maxRedirections
     this.handler = handler
     this.history = []
@@ -105,7 +102,7 @@ class RedirectHandler {
     // 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
-    this.opts.headers = cleanRequestHeaders(this.opts.headers, statusCode === 303, this.opts.origin !== origin, this.stripHeadersOnRedirect)
+    this.opts.headers = cleanRequestHeaders(this.opts.headers, statusCode === 303, this.opts.origin !== origin, this.stripHeadersOnRedirect, this.stripHeadersOnCrossOriginRedirect)
     this.opts.path = path
     this.opts.origin = origin
     this.opts.query = null
@@ -157,53 +154,57 @@ class RedirectHandler {
 }
 
 // https://tools.ietf.org/html/rfc7231#section-6.4.4
-function shouldRemoveHeader (header, removeContent, unknownOrigin, stripHeaders) {
-  if (header.length === 4) {
-    return util.headerNameToString(header) === 'host'
+function shouldRemoveHeader (header, removeContent, unknownOrigin, stripHeaders, stripHeadersOnCrossOrigin) {
+  const name = util.headerNameToString(header)
+  if (name === 'host') {
+    return true
   }
-  if (stripHeaders?.has(util.headerNameToString(header))) {
+  if (stripHeaders?.has(name) || (unknownOrigin && stripHeadersOnCrossOrigin?.has(name))) {
     return true
   }
-  if (removeContent && util.headerNameToString(header).startsWith('content-')) {
+  if (removeContent && name.startsWith('content-')) {
     return true
   }
-  if (unknownOrigin && (header.length === 13 || header.length === 6 || header.length === 19)) {
-    const name = util.headerNameToString(header)
+  if (unknownOrigin) {
     return name === 'authorization' || name === 'cookie' || name === 'proxy-authorization'
   }
   return false
 }
 
 // https://tools.ietf.org/html/rfc7231#section-6.4
-function normalizeStripHeadersOnRedirect (headers) {
+function normalizeStripHeaders (headers, optionName) {
   if (headers == null) {
     return null
   }
 
+  if (!Array.isArray(headers)) {
+    throw new InvalidArgumentError(`${optionName} must be an array`)
+  }
+
   const normalized = new Set()
   for (const header of headers) {
     if (typeof header !== 'string') {
-      throw new InvalidArgumentError('stripHeadersOnRedirect must contain header names')
+      throw new InvalidArgumentError(`${optionName} must contain header names`)
     }
 
     normalized.add(util.headerNameToString(header))
   }
   return normalized
 }
 
-function cleanRequestHeaders (headers, removeContent, unknownOrigin, stripHeaders) {
+function cleanRequestHeaders (headers, removeContent, unknownOrigin, stripHeaders, stripHeadersOnCrossOrigin) {
   const ret = []
   if (Array.isArray(headers)) {
     for (let i = 0; i < headers.length; i += 2) {
-      if (!shouldRemoveHeader(headers[i], removeContent, unknownOrigin, stripHeaders)) {
+      if (!shouldRemoveHeader(headers[i], removeContent, unknownOrigin, stripHeaders, stripHeadersOnCrossOrigin)) {
         ret.push(headers[i], headers[i + 1])
       }
     }
   } else if (headers && typeof headers === 'object') {
     const entries = util.hasSafeIterator(headers) ? headers : Object.entries(headers)
 
     for (const [key, value] of entries) {
-      if (!shouldRemoveHeader(key, removeContent, unknownOrigin, stripHeaders)) {
+      if (!shouldRemoveHeader(key, removeContent, unknownOrigin, stripHeaders, stripHeadersOnCrossOrigin)) {
         ret.push(key, value)
       }
     }

lib/interceptor/redirect.js

@@ -2,16 +2,16 @@
 
 const RedirectHandler = require('../handler/redirect-handler')
 
-function createRedirectInterceptor ({ maxRedirections: defaultMaxRedirections, throwOnMaxRedirect: defaultThrowOnMaxRedirect, stripHeadersOnRedirect: defaultStripHeadersOnRedirect } = {}) {
+function createRedirectInterceptor ({ maxRedirections: defaultMaxRedirections, throwOnMaxRedirect: defaultThrowOnMaxRedirect, stripHeadersOnRedirect: defaultStripHeadersOnRedirect, stripHeadersOnCrossOriginRedirect: defaultStripHeadersOnCrossOriginRedirect } = {}) {
   return (dispatch) => {
     return function Intercept (opts, handler) {
-      const { maxRedirections = defaultMaxRedirections, throwOnMaxRedirect = defaultThrowOnMaxRedirect, stripHeadersOnRedirect = defaultStripHeadersOnRedirect, ...rest } = opts
+      const { maxRedirections = defaultMaxRedirections, throwOnMaxRedirect = defaultThrowOnMaxRedirect, stripHeadersOnRedirect = defaultStripHeadersOnRedirect, stripHeadersOnCrossOriginRedirect = defaultStripHeadersOnCrossOriginRedirect, ...rest } = opts
 
       if (maxRedirections == null || maxRedirections === 0) {
         return dispatch(opts, handler)
       }
 
-      const dispatchOpts = { ...rest, throwOnMaxRedirect, stripHeadersOnRedirect } // Stop sub dispatcher from also redirecting.
+      const dispatchOpts = { ...rest, throwOnMaxRedirect, stripHeadersOnRedirect, stripHeadersOnCrossOriginRedirect } // Stop sub dispatcher from also redirecting.
       const redirectHandler = new RedirectHandler(dispatch, maxRedirections, dispatchOpts, handler)
       return dispatch(dispatchOpts, redirectHandler)
     }

test/interceptors/redirect.js

@@ -898,6 +898,64 @@ test('same-origin redirects strip configured headers', async (t) => {
   strictEqual(text, 'redirected')
 })
 
+test('cross-origin redirects strip configured headers only across origins', async (t) => {
+  const { strictEqual } = tspl(t, { plan: 7 })
+
+  const server1 = createServer((req, res) => {
+    strictEqual(req.headers['x-custom'], undefined)
+    strictEqual(req.headers['x-keep'], 'present')
+    res.end('redirected')
+  }).listen(0)
+
+  const server2 = createServer((req, res) => {
+    if (req.url === '/redirect') {
+      strictEqual(req.headers['x-custom'], 'secret')
+      strictEqual(req.headers['x-keep'], 'present')
+
+      res.writeHead(302, {
+        Location: '/same-origin'
+      })
+      res.end()
+      return
+    }
+
+    strictEqual(req.headers['x-custom'], 'secret')
+    strictEqual(req.headers['x-keep'], 'present')
+
+    res.writeHead(302, {
+      Location: `http://localhost:${server1.address().port}`
+    })
+    res.end()
+  }).listen(0)
+
+  t.after(() => {
+    server1.close()
+    server2.close()
+  })
+
+  await Promise.all([
+    once(server1, 'listening'),
+    once(server2, 'listening')
+  ])
+
+  const dispatcher = new undici.Agent({}).compose(redirect({
+    maxRedirections: 2,
+    stripHeadersOnCrossOriginRedirect: ['X-Custom']
+  }))
+  after(() => dispatcher.close())
+
+  const res = await undici.request(`http://localhost:${server2.address().port}/redirect`, {
+    dispatcher,
+    headers: {
+      'X-Custom': 'secret',
+      'X-Keep': 'present'
+    }
+  })
+
+  const text = await res.body.text()
+  strictEqual(text, 'redirected')
+})
+
 test('Cross-origin redirects clear forbidden headers', async (t) => {
   const { strictEqual } = tspl(t, { plan: 6 })
 

test/types/redirect-interceptor.test-d.ts

@@ -7,8 +7,12 @@ expectAssignable<Interceptors.RedirectInterceptorOpts>({ throwOnMaxRedirect: tru
 expectAssignable<Interceptors.RedirectInterceptorOpts>({ maxRedirections: 3, throwOnMaxRedirect: true })
 expectAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnRedirect: ['x-custom'] })
 expectAssignable<Interceptors.RedirectInterceptorOpts>({ maxRedirections: 3, stripHeadersOnRedirect: ['x-custom'] })
+expectAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnCrossOriginRedirect: ['x-custom'] })
+expectAssignable<Interceptors.RedirectInterceptorOpts>({ maxRedirections: 3, stripHeadersOnCrossOriginRedirect: ['x-custom'] })
 
 expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ maxRedirections: 'INVALID' })
 expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ throwOnMaxRedirect: 'INVALID' })
 expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnRedirect: 'INVALID' })
 expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnRedirect: [1] })
+expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnCrossOriginRedirect: 'INVALID' })
+expectNotAssignable<Interceptors.RedirectInterceptorOpts>({ stripHeadersOnCrossOriginRedirect: [1] })

types/interceptors.d.ts

@@ -8,7 +8,7 @@ export default Interceptors
 declare namespace Interceptors {
   export type DumpInterceptorOpts = { maxSize?: number }
   export type RetryInterceptorOpts = RetryHandler.RetryOptions
-  export type RedirectInterceptorOpts = { maxRedirections?: number, throwOnMaxRedirect?: boolean, stripHeadersOnRedirect?: string[] }
+  export type RedirectInterceptorOpts = { maxRedirections?: number, throwOnMaxRedirect?: boolean, stripHeadersOnRedirect?: string[], stripHeadersOnCrossOriginRedirect?: string[] }
   export type DecompressInterceptorOpts = {
     skipErrorResponses?: boolean
     skipStatusCodes?: number[]