Unable to set multiple cookies from within service method handler

[tcarnes]

Describe the bug

We are using @bufbuild/connect-express to connect to an express application, but cannot create multiple set-cookie headers to enable browsers to set multiple cookies based on response. For this header in particular, we need the ability to set separate headers with same key instead of combining into single value.

To Reproduce

Simply try appending multiple values to ctx.responseHeader from within a service handler, and only one header gets sets. Trying to set once with multiple values fails as well.

Environment (please complete the following information):

• @bufbuild/connect-web version: 0.9.1
• @bufbuild/connect-node version: 0.9.1
• Frontend framework and version: angular@14.1.0
• Node.js version: 18.16.0
• Browser and version: Google Chrome 115.0.5790.170

[timostamm]

Hey Tim, good call. Special handling of the Set-Cookie header was added the the fetch specification in whatwg/fetch#1346.

The feature was subsequently added to undici (the fetch implementation of Node.js) in v5.19.0.

Node 18 updated to this version of undici in v18.14.1:

$ fnm exec --using=v18.14.0 node t.mjs
getSetCookie: unavailable
forEach: custom: a, b
forEach: set-cookie: a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT

$ fnm exec --using=v18.14.1 node t.mjs
getSetCookie: [
  'a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT',
  'b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT'
]
forEach: custom: a, b
forEach: set-cookie: a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT
forEach: set-cookie: b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT

Node 16 got the update in v16.19.1:

$ fnm exec --using=v16.19.0 node --experimental-fetch t.mjs
getSetCookie: unavailable
forEach: custom: a, b
forEach: set-cookie: a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT, b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT

$ fnm exec --using=v16.19.1 node --experimental-fetch t.mjs
getSetCookie: [
  'a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT',
  'b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT'
]
forEach: custom: a, b
forEach: set-cookie: a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT
forEach: set-cookie: b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT

Node 20 had it from the start.

t.mjs

const h = new Headers();
h.append("Custom", "a");
h.append("Custom", "b");
h.append("Set-Cookie", "a=a; Expires=Wed, 21 Oct 2015 07:28:00 GMT");
h.append("Set-Cookie", "b=b; Expires=Wed, 21 Oct 2015 07:28:00 GMT");
if ("getSetCookie" in h && typeof h.getSetCookie == "function") {
    const setCookies = h.getSetCookie();
    console.log("getSetCookie:", setCookies);
} else {
    console.log("getSetCookie: unavailable");
}
h.forEach((value, key) => {
    console.log(`forEach: ${key}: ${value}`);
});

@bufbuild/connect-node applies a polyfill with headers-polyfill package on import, which unfortunately does not handle the Set-cookie header yet.

We should definitely support the new handling of the header.

We might want to switch our polyfill to use undici.

[timostamm]

For posterity:

We might want to switch our polyfill to use undici.

We just did in #791.