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

Issue with undici.request and Headers Global Object #2700

Closed
JaoodxD opened this issue Feb 5, 2024 · 5 comments · Fixed by #2708
Closed

Issue with undici.request and Headers Global Object #2700

JaoodxD opened this issue Feb 5, 2024 · 5 comments · Fixed by #2708
Labels
enhancement New feature or request

Comments

@JaoodxD
Copy link
Contributor

JaoodxD commented Feb 5, 2024

Bug Description

Undici's request function appears to be skipping headers that are constructed using the Headers global object.

Reproducible By

  1. Create an instance of Headers class.
  2. Add header with set/append.
  3. Pass headers instance to request.
const headers = new Headers()
headers.append('random-header', 'hello world')

const res = await request('http://localhost:3001', { headers })

Full reproducable example can be found in following repo

Expected Behavior

I expect the undici.request function to treat instances of the Headers class the same way it handles plain JavaScript objects passed to options.
Specifically, I anticipate that the headers, when constructed using the Headers class, should be seamlessly attached to the HTTP request.

Logs & Screenshots

{
  host: 'localhost:3000',
  connection: 'keep-alive',
  'random-header': 'hello world' // passed as plain object
}
{ host: 'localhost:3001', connection: 'keep-alive' } // as Headers instance

Environment

OS: Windows 10
Nodejs: v20.8.0
undici: v6.6.1

@JaoodxD JaoodxD added the bug Something isn't working label Feb 5, 2024
@JaoodxD
Copy link
Contributor Author

JaoodxD commented Feb 5, 2024

As we can see here https://github.com/nodejs/undici/blob/main/lib/core/request.js#L159-L174 request constructor expects only Array or plain Object.

@ronag ronag added enhancement New feature or request and removed bug Something isn't working labels Feb 5, 2024
@ronag
Copy link
Member

ronag commented Feb 5, 2024

Headers object are not expected to work. Though I wouldn't mind adding support for it. PR welcome.

@JaoodxD
Copy link
Contributor Author

JaoodxD commented Feb 5, 2024

Headers object are not expected to work. Though I wouldn't mind adding support for it. PR welcome.

I would like to implement it.

@KhafraDev
Copy link
Member

KhafraDev commented Feb 5, 2024

it might be worth it to allow Map-like objects instead (ie. something that implements .get()/an iterator) instead of only Headers.

@JaoodxD
Copy link
Contributor Author

JaoodxD commented Feb 6, 2024

it might be worth it to allow Map-like objects instead (ie. something that implements .get()/an iterator) instead of only Headers.

Good idea. Would it be a sufficient to implement it like it's done in fetch? (check for symbol.iterator)

undici/lib/fetch/headers.js

Lines 573 to 576 in 8e14078

if (webidl.util.Type(V) === 'Object') {
if (V[Symbol.iterator]) {
return webidl.converters['sequence<sequence<ByteString>>'](V)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants