BUG: Headers being duplicated after v0.6.24

[arthurfiorette]

After I upgraded elysiajs to v0.6.24. Most of https://github.com/elysiajs/elysia-html tests are failing due to headers being duplicated.

You can reproduce it by cloning the repo and running the tests with v0.6.24 and v0.6.23.

Current elysiajs/html has some bugs in the headers workflow but that's not the case here as with .23 tests are passing. I did not have time do debug this issue within the elysia's codebase, but may just be that the way derive is used with headers changed from .23 to .24 and it needs to be updated inside the html plugin.

P.S: Is there a reason to not have CI pipelines? This could've been caught automatically in elysiajs/html codebase if automatic tests were correctly set up. (#179)

[bogeychan]

This PR causes the new bug #167

The following lines of code in mapEarlyResponse breaks it:

elysia/src/handler.ts

Lines 73 to 74 in 4d74086

	for (const key in inherits)
	(response as Response).headers.append(key, inherits[key])

The Content-Type Header exists in both inherits and response.headers.

See

• https://github.com/elysiajs/elysia-html/blob/5dee1438c0f19b1de6eed243e65b81fc39a3bf4c/src/html.ts#L45
  • set.headers['content-type'] = options.contentType!
• https://github.com/elysiajs/elysia-html/blob/5dee1438c0f19b1de6eed243e65b81fc39a3bf4c/src/html.ts#L50
  • return new Response(response, set)

We need to somehow merge the headers or define whether it is allowed to use new Response(response, set).

idk if it can be merged at all, in case someone needs duplicate headers (name and value?).. any ideas @SaltyAom

[arthurfiorette]

Why is merge the default behavior? There's little to none headers where this would be useful. Server, content length, content type, authorization and many others in tests are also being duplicated.

The current Elysia html implementation is faulty, however it is also breaking when multiple libraries interact with each other and in tests. Headers by default should overwrite unless .append method is manually called somewhere.

[bogeychan]

Merge

Using merge as the default to be some fault tolerant. the headers in the response itself would have a higher priority, like (pseudo code):

for (const key in inherits) {
  if (response.headers.has(key)) continue;
  response.headers.append(key, inherits[key]);
}

Define

but it's also valid to define that when someone returns a Response object they must take care of setting the headers:

return new Response(response, set)

and therefore remove these lines of code from mapEarlyResponse:

elysia/src/handler.ts

Lines 73 to 74 in 4d74086

	for (const key in inherits)
	(response as Response).headers.append(key, inherits[key])

[SaltyAom]

Should be fixed in 0fc2f0a, and seems not to happen in the latest version.

Feels free to reopen the issue if still persists.