Limitation on set-cookie in responseHeaders

[AlbertSabate]

Describe the bug
There is a limitation when setting cookies header (set-cookie) in the responseHeader.
You cannot set more than one cookie (in an elegant way)

To Reproduce
const responseHeaders = new Header();
responseHeaders.append('set-cookie', 'cookie1...');
responseHeaders.append('set-cookie', 'cookie2...');

return responseHeaders.toJSON();

Only one cookie will be set

Expected behavior
Be able to set multiple cookie in one responseHeader.

[AlbertSabate]

As a workaround, if you need to set more than 1 cookie in one responseHeader you can do it. Because JS is an awesome technology you can currently do this:

return {
      'set-cookie': cookie[0],
      'Set-Cookie': cookie[1],
      'sEt-Cookie': cookies[2],
    };

This works, but of course is not elegant. We should support the format 'set-cookies': [cookie[0],cookie[1]], which currently is not supported.

[aralroca]

@AlbertSabate What version of Bun do you have? With Bun 1.2.2:

const h = new Headers();

h.append('set-cookies', 'cookie1');
h.append('set-cookies', 'cookie2');

console.log(h.toJSON())

The log is:

{
  "set-cookies": "cookie1, cookie2",
}

And, if you have an array of cookies, probably you can use .join?

[AlbertSabate]

Forgot to reply you !!

Bun works perfect, this should give you an array though

{
  "set-cookies": ["cookie1", "cookie2"],
}

The headers needs to be duplicated on the response, not together. If my memory is not wrong some broweser will understand and other browsers will not. So the ask here is that Brisa should handle the object when receiving the above return (with array). Currently does not, and only the last set-cookies will prevail.

The reason why my example works is because Brisa does not receive an array type, in fact will receive:

{
  "set-cookies": "cookie1",
  "Set-cookies": "cookie2",
}

And when you return this in the Response, the response will put all back together perfectly, all lowered cased as standard.

[AlbertSabate]

I didn't test it with console logs very deeply but the issue should be:

on the method getPageComponentWithHeaders in file packages/brisa/src/utils/get-page-component-with-headers/index.ts

It does look correct though according to types: export type HeadersInit = string[][] | Record<string, string | ReadonlyArray<string>> | Headers

But should be very easy to debug and fix if necessary. (Super good for Winter Of Code 4.0)

[aralroca]

@AlbertSabate There are more multi-value headers with the same problem:

Header	Example Value
Set-Cookie	Set-Cookie: session=abc
WWW-Authenticate	WWW-Authenticate: Basic realm="..."
Link	Link: <url>; rel="next"
Authorization	Authorization: Bearer token123
Accept	Accept: text/html, application/xml
Accept-Encoding	Accept-Encoding: gzip, deflate
Cache-Control	Cache-Control: no-store, no-cache
Access-Control-Allow-Methods	Access-Control-Allow-Methods: GET, POST
Vary	Vary: Accept-Encoding, User-Agent

The solution should be generic and merge all the headers. Ex:

export default function mergeHeaders(
  ...inputHeaders: (Headers | Record<string, string>)[]
): Headers {
  const headers = new Headers();

  for (const header of inputHeaders) {
    const entries = header?.entries?.() ?? Object.entries(header ?? {});

    for (const [key, value] of entries) {
      // Merge multiple values for the same header (Set-Cookie, WWW-Authenticate, ...)
      if (Array.isArray(value)) {
        for (const v of value) headers.append(key, v);
      } else {
        headers.append(key, value);
      }
    }
  }

  return headers;
}

And then we can replace this:

brisa/packages/brisa/src/utils/get-page-component-with-headers/index.ts

Lines 34 to 43 in e3a4e48

	const pageHeaders = new Headers({
	'cache-control': HEADERS.CACHE_CONTROL,
	...middlewareResponseHeaders,
	...layoutResponseHeaders,
	...pageResponseHeaders,
	...headers,
	'transfer-encoding': 'chunked',
	vary: 'Accept-Encoding',
	'content-type': 'text/html; charset=utf-8',
	});

To:

const pageHeaders = mergeHeaders(
  {
    'cache-control': HEADERS.CACHE_CONTROL,
  },
  middlewareResponseHeaders,
  layoutResponseHeaders,
  pageResponseHeaders,
  headers,
  {
    'transfer-encoding': 'chunked',
    vary: 'Accept-Encoding',
    'content-type': 'text/html; charset=utf-8',
  }
);