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

Node 18.18.2 Incorrectly copies headers onto fetches #2374

Closed
bbowman opened this issue Oct 25, 2023 · 6 comments
Closed

Node 18.18.2 Incorrectly copies headers onto fetches #2374

bbowman opened this issue Oct 25, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@bbowman
Copy link

bbowman commented Oct 25, 2023

Bug Description

Noticed that in our product using Node 18 that we encountered a slew of issues when moving forward to 18.18.2 that included a CVE fix for undici. Debugging into the issue it appears that in the CVE fix a class name was changed resulting in incorrect behavior. See the screenshot below and I believe it should be a simple fix. The headers copying logic fell down the wrong path when it failed to recognize that the passed in Object was already a Headers object.

Reproducible By

I dont have a minimal repro handy but it should be just creating a new Request with both an existing request with headers to copy and an init param.

Expected Behavior

When creating a new request with both a Request object to copy and an init set of options, the headers from the request are preserved.

Logs & Screenshots

image

Environment

Node v18.18.2

@bbowman bbowman added the bug Something isn't working label Oct 25, 2023
@metcoder95
Copy link
Member

cc: @KhafraDev @mcollina

@KhafraDev
Copy link
Member

KhafraDev commented Oct 25, 2023

duplicate of #2358

@bbowman
Copy link
Author

bbowman commented Oct 25, 2023

@KhafraDev thank you for the quick fix and sorry for not looking for dupes first! Do we have any plan worked out for if / when Node would take this for a 18.18.3? We are avoid taking 18.18x in the meantime but would love the latest security fixes once this is integrated.

@bbowman
Copy link
Author

bbowman commented Oct 25, 2023

nodejs/node#50274 might have that info for others looking.

@KhafraDev
Copy link
Member

No worries about reporting dupes, I don't mind at all. I don't have any idea when (or if) v18 will be updated; someone will have to take the time to backport the undici update changes. Looking at the issue linked here, is it fixed with the newest undici update?

@MajorTanya
Copy link

Not sure if that's exactly what you asked, but I can confirm it was fixed in Node v21.1.0 (released yesterday), which included the update to undici 5.26.4, so I'd assume that backporting that update would fix it for v18.x and v20.x as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants