h3 icon indicating copy to clipboard operation
h3 copied to clipboard

mergeHeaders unexpected behaviour / type issue

Open Timo972 opened this issue 1 year ago • 3 comments

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 avatar Aug 14 '24 18:08 Timo972

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: https://github.com/gemmadlou/h3/commit/019f0e8214b42329533095925239d8287b99124e.

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 {}

gemmadlou avatar Aug 29 '24 22:08 gemmadlou

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

Timo972 avatar Aug 30 '24 04:08 Timo972

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.

gemmadlou avatar Aug 30 '24 05:08 gemmadlou