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

mergeHeaders unexpected behaviour / type issue #865

Open
Timo972 opened this issue Aug 14, 2024 · 3 comments
Open

mergeHeaders unexpected behaviour / type issue #865

Timo972 opened this issue Aug 14, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@Timo972
Copy link

Timo972 commented Aug 14, 2024

Environment

Nuxt 3.12.2 with Nitro 2.9.6
development mode

Reproduction

const headers = new Headers()
if (session.token)
    headers.set("Authorization", `Bearer ${session.token}`)
else
    console.warn(`[PROXY] No token set`)
if (session.domain)
    headers.set("X-Domain", session.domain)
else
    console.warn(`[PROXY] No domain set`)

// note: no 'Authorization' header in the event headers
const mergedHeaders = mergeHeaders(event.headers, headers)

console.log(mergedHeaders.get("Authorization"))
// log: undefined

Describe the bug

The mergeHeaders utility won't merge multiple instances of Headers. This is a result of the underlying logic using Object.entries to get a list of key-value pairs which returns an empty array on a Headers instance.

Since Headers satisfies the HeadersInit type, this should either work, or the types should be changed appropriately.
Once it's clear how we should continue, I will open a PR.

Additional context

I found this issue when trying to use the proxyRequest / sendProxy utility, basically all of the other utilities depending on mergeHeaders suffer from the type / logic issue and it's not that trivial to find, because you think something is wrong in your code.

Logs

No response

@Timo972 Timo972 added the bug Something isn't working label Aug 14, 2024
@gemmadlou
Copy link

gemmadlou commented Aug 29, 2024

Hi @Timo972

I didn't want to create a PR as you said you would do so already, and I'm not sure if it's the direction you want to go either, but I made an attempt here:
gemmadlou@019f0e8.

Since Headers satisfies the HeadersInit type, this should either work, or the types should be changed appropriately.
Once it's clear how we should continue, I will open a PR.

I saw this note but noted that Object.entries(input!) didn't do anything if the input was instanceof Headers.

Doing a quick test:

let headers = new Headers()
headers.append('x-domain', '1234')

console.log(
    Object.entries(headers)
)

console.log(
    headers.entries()
)
// -> []
// -> Headers Iterator {}

@Timo972
Copy link
Author

Timo972 commented Aug 30, 2024

That's what I meant by

This is a result of the underlying logic using Object.entries to get a list of key-value pairs which returns an empty array on a Headers instance.

I guess I will open a pull request in the next few days which will fix this

@gemmadlou
Copy link

That's what I meant by

This is a result of the underlying logic using Object.entries to get a list of key-value pairs which returns an empty array on a Headers instance.

I guess I will open a pull request in the next few days which will fix this

I get you now. 👍 Of course. Makes senses. And thanks for the PR. In my version, I was just trying to understand the underlying problem. In the meantime, there's nothing stopping from not using the Header object.

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

2 participants