Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedirectHandler cleans headers too aggressively #3773

Closed
joelschou opened this issue Oct 25, 2024 · 1 comment · Fixed by #3777
Closed

RedirectHandler cleans headers too aggressively #3773

joelschou opened this issue Oct 25, 2024 · 1 comment · Fixed by #3777
Labels
bug Something isn't working interceptors Pull requests or issues related to Dispatcher Interceptors

Comments

@joelschou
Copy link

Bug Description

Undici explicitly supports headers passed as a Headers object, but the RedirectHandler does not account for this object type in its internal cleanRequestHeaders function.

Reproducible By

  1. Configure global dispatcher to use redirect interceptor

  2. Use undici.request to send a request with headers constructed via new Headers(...)

    import { Headers, request } from 'undici';
    
    await request( '/local/test/url', { headers: new Headers({ 'x-my-header': true }) } );
  3. Your local test server should see 'x-my-header': 'true' in the request headers

  4. Your local test server should respond with a 3xx statusCode and a location: '/local/test/alt-url' header

  5. Undici should follow the redirect and make a request to the new location

  6. Your local test server WILL NOT see 'x-my-header': 'true' in the request headers

Compare to:

  1. same as above

  2. Send the request this way instead:

    await request( '/local/test/url', { headers: { 'x-my-header': true } } );
  3. same as above

  4. same as above

  5. same as above

  6. Your local test server WILL see 'x-my-header': 'true' in the request headers

Expected Behavior

  1. Headers on the initial request should be repeated on the redirect request(s) for all supported ways to pass headers

Logs & Screenshots

I believe I've tracked this down to this logic inside cleanRequestHeaders:

// ...
} else if (headers && typeof headers === 'object') {
    for (const key of Object.keys(headers)) {
      if (!shouldRemoveHeader(key, removeContent, unknownOrigin)) {
        ret.push(key, headers[key])
      }
    }
  }
// ...

For headers constructed with new Headers(...), the typeof headers === 'object' condition will return true, but Object.keys(...) does not behave as expected on a Headers object. As a result, this for loop does not push any keys/values into ret.

Environment

  • macOS 15.0.1 (current)
  • Node v20.16.0 (LTS, but two minors behind)

Additional context

@joelschou joelschou added the bug Something isn't working label Oct 25, 2024
@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@metcoder95 metcoder95 added the interceptors Pull requests or issues related to Dispatcher Interceptors label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working interceptors Pull requests or issues related to Dispatcher Interceptors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@mcollina @joelschou @metcoder95 and others