#7226 - fixes NodeJS adapter for multiple set-cookie headers (and oth…

…er header issues) (#7227)

* Utilizes the new standard WebAPI Fetch Headers.getSetCookie() function
to safely handle multiple set-cookie headers when converting from a
WebAPI Response to a NodeJS ServerResponse

Modifies the existing nodeMiddleware logic which first set AstroCookies
on ServerResponse.setHeader(...) and then called
ServerResponse.writeHead(status, Response.headers) which means any that
if the WebAPI Response had any set-cookie headers on it, they would
replace anything from AstroCookies.

The new logic delegates appending AstroCookie values onto the WebAPI
Response Headers object, so that a single unified function safely
converts the WebAPI Response Headers into a NodeJS compatible
OutgoingHttpHeaders object utilizing the new standard
Headers.getSetCookie() function provided by the undici WebAPI polyfills.

Plus extensive test coverage.

* #7226 - changeset for NodeJS adapter set-cookie fix

* fixing all double quotes to single quotes

---------

Co-authored-by: Alex Sherwin <alex.sherwin@acadia.inc>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

.changeset/chilled-rockets-flash.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/node': minor
+---
+
+Fixes NodeJS adapter for multiple set-cookie headers and combining AstroCookies and Response.headers cookies

packages/integrations/node/package.json

@@ -41,6 +41,7 @@
     "astro": "workspace:^2.5.7"
   },
   "devDependencies": {
+    "@types/node": "^18.7.21",
     "@types/send": "^0.17.1",
     "@types/server-destroy": "^1.0.1",
     "astro": "workspace:*",

packages/integrations/node/src/createOutgoingHttpHeaders.ts

@@ -0,0 +1,40 @@
+import type { OutgoingHttpHeaders } from 'http';
+
+/**
+ * Takes in a nullable WebAPI Headers object and produces a NodeJS OutgoingHttpHeaders object suitable for usage
+ * with ServerResponse.writeHead(..) or ServerResponse.setHeader(..)
+ * 
+ * @param webHeaders WebAPI Headers object
+ * @returns NodeJS OutgoingHttpHeaders object with multiple set-cookie handled as an array of values
+ */
+export const createOutgoingHttpHeaders = (webHeaders: Headers | undefined | null): OutgoingHttpHeaders | undefined => {
+  if (!webHeaders) {
+    return undefined;
+  }
+
+  // re-type to access Header.getSetCookie()
+  const headers = webHeaders as HeadersWithGetSetCookie;
+
+  // at this point, a multi-value'd set-cookie header is invalid (it was concatenated as a single CSV, which is not valid for set-cookie)
+  const nodeHeaders: OutgoingHttpHeaders = Object.fromEntries(headers.entries());
+
+  if (Object.keys(nodeHeaders).length === 0) {
+    return undefined;
+  }
+
+  // if there is > 1 set-cookie header, we have to fix it to be an array of values
+  if (headers.has('set-cookie')) {
+    const cookieHeaders = headers.getSetCookie();
+    if (cookieHeaders.length > 1) {
+      // the Headers.entries() API already normalized all header names to lower case so we can safely index this as 'set-cookie'
+      nodeHeaders['set-cookie'] = cookieHeaders;
+    }
+  }
+
+  return nodeHeaders;
+};
+
+interface HeadersWithGetSetCookie extends Headers {
+  // the @astrojs/webapi polyfill makes this available (as of undici@5.19.0), but tsc doesn't pick it up on the built-in Headers type from DOM lib
+  getSetCookie(): string[];
+}

packages/integrations/node/src/nodeMiddleware.ts

@@ -3,6 +3,7 @@ import type { IncomingMessage, ServerResponse } from 'http';
 import type { Readable } from 'stream';
 import { responseIterator } from './response-iterator';
 import type { Options } from './types';
+import { createOutgoingHttpHeaders } from './createOutgoingHttpHeaders';
 
 export default function (app: NodeApp, mode: Options['mode']) {
 	return async function (
@@ -44,12 +45,16 @@ async function writeWebResponse(app: NodeApp, res: ServerResponse, webResponse:
 
 	if (app.setCookieHeaders) {
 		const setCookieHeaders: Array<string> = Array.from(app.setCookieHeaders(webResponse));
+
 		if (setCookieHeaders.length) {
-			res.setHeader('Set-Cookie', setCookieHeaders);
+      for (const setCookieHeader of setCookieHeaders) {
+        webResponse.headers.append('set-cookie', setCookieHeader);
+      }
 		}
 	}
 
-	res.writeHead(status, Object.fromEntries(headers.entries()));
+  const nodeHeaders = createOutgoingHttpHeaders(headers);
+  res.writeHead(status, nodeHeaders);
 	if (webResponse.body) {
 		try {
 			for await (const chunk of responseIterator(webResponse) as unknown as Readable) {

packages/integrations/node/test/createOutgoingHttpHeaders.test.js

@@ -0,0 +1,78 @@
+import { expect } from 'chai';
+
+import { createOutgoingHttpHeaders } from '../dist/createOutgoingHttpHeaders.js';
+
+describe('createOutgoingHttpHeaders', () => {
+
+  it('undefined input headers', async () => {
+    const result = createOutgoingHttpHeaders(undefined);
+    expect(result).to.equal(undefined);
+  });
+
+  it('null input headers', async () => {
+    const result = createOutgoingHttpHeaders(undefined);
+    expect(result).to.equal(undefined);
+  });
+
+  it('Empty Headers', async () => {
+    const headers = new Headers();
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.equal(undefined);
+  });
+
+  it('Headers with single key', async () => {
+    const headers = new Headers();
+    headers.append('x-test', 'hello world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({ 'x-test': 'hello world' });
+  });
+
+  it('Headers with multiple keys', async () => {
+    const headers = new Headers();
+    headers.append('x-test1', 'hello');
+    headers.append('x-test2', 'world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({ 'x-test1': 'hello', 'x-test2': 'world' });
+  });
+
+  it('Headers with multiple values (not set-cookie)', async () => {
+    const headers = new Headers();
+    headers.append('x-test', 'hello');
+    headers.append('x-test', 'world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({ 'x-test': 'hello, world' });
+  });
+
+  it('Headers with multiple values (set-cookie special case)', async () => {
+    const headers = new Headers();
+    headers.append('set-cookie', 'hello');
+    headers.append('set-cookie', 'world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({ 'set-cookie': ['hello', 'world'] });
+  });
+
+  it('Headers with multiple values (set-cookie case handling)', async () => {
+    const headers = new Headers();
+    headers.append('Set-cookie', 'hello');
+    headers.append('Set-Cookie', 'world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({ 'set-cookie': ['hello', 'world'] });
+  });
+
+  it('Headers with all use cases', async () => {
+    const headers = new Headers();
+    headers.append('x-single', 'single');
+    headers.append('x-triple', 'one');
+    headers.append('x-triple', 'two');
+    headers.append('x-triple', 'three');
+    headers.append('Set-cookie', 'hello');
+    headers.append('Set-Cookie', 'world');
+    const result = createOutgoingHttpHeaders(headers);
+    expect(result).to.deep.equal({
+      'x-single': 'single',
+      'x-triple': 'one, two, three',
+      'set-cookie': ['hello', 'world'],
+    });
+  });
+
+});

packages/integrations/node/test/fixtures/headers/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@test/nodejs-headers",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*",
+    "@astrojs/node": "workspace:*"
+  }
+}

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-astro-cookies-multi.astro

@@ -0,0 +1,5 @@
+---
+Astro.cookies.set('from1', 'astro1');
+Astro.cookies.set('from2', 'astro2');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-astro-cookies-single.astro

@@ -0,0 +1,4 @@
+---
+Astro.cookies.set('from1', 'astro1');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-astro-response-cookie-multi.astro

@@ -0,0 +1,7 @@
+---
+Astro.response.headers.append('set-cookie', 'from1=response1');
+Astro.response.headers.append('set-cookie', 'from2=response2');
+Astro.cookies.set('from3', 'astro1');
+Astro.cookies.set('from4', 'astro2');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-astro-response-cookie-single.astro

@@ -0,0 +1,5 @@
+---
+Astro.response.headers.append('set-cookie', 'from1=response1');
+Astro.cookies.set('from1', 'astro1');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-response-cookies-multi.astro

@@ -0,0 +1,5 @@
+---
+Astro.response.headers.append('set-cookie', 'from1=value1');
+Astro.response.headers.append('set-cookie', 'from2=value2');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/astro/component-response-cookies-single.astro

@@ -0,0 +1,4 @@
+---
+Astro.response.headers.append('set-cookie', 'from1=value1');
+---
+<p>hello world</p>

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/astro-cookies-multi.ts

@@ -0,0 +1,9 @@
+import type { APIContext } from 'astro';
+
+export async function get({ request, cookies }: APIContext) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  cookies.set('from1', 'astro1');
+  cookies.set('from2', 'astro2');
+  return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/astro-cookies-single.ts

@@ -0,0 +1,8 @@
+import type { APIContext } from 'astro';
+
+export async function get({ request, cookies }: APIContext) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  cookies.set('from1', 'astro1');
+  return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/astro-response-cookie-multi.ts

@@ -0,0 +1,11 @@
+import type { APIContext } from 'astro';
+
+export async function get({ request, cookies }: APIContext) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('set-cookie', 'from1=response1');
+  headers.append('set-cookie', 'from2=response2');
+  cookies.set('from3', 'astro1');
+  cookies.set('from4', 'astro2');
+  return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/astro-response-cookie-single.ts

@@ -0,0 +1,9 @@
+import type { APIContext } from 'astro';
+
+export async function get({ request, cookies }: APIContext) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('set-cookie', 'from1=response1');
+  cookies.set('from1', 'astro1');
+  return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/kitchen-sink.ts

@@ -0,0 +1,11 @@
+export async function get({ request }: { request: Request }) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('x-SINGLE', 'single');
+  headers.append('X-triple', 'one');
+  headers.append('x-Triple', 'two');
+  headers.append('x-TRIPLE', 'three');
+  headers.append('SET-cookie', 'hello1=world1');
+  headers.append('Set-Cookie', 'hello2=world2');
+	return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/response-cookies-multi.ts

@@ -0,0 +1,7 @@
+export async function get({ request }: { request: Request }) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('Set-Cookie', 'hello1=world1');
+  headers.append('SET-COOKIE', 'hello2=world2');
+	return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/response-cookies-single.ts

@@ -0,0 +1,6 @@
+export async function get({ request }: { request: Request }) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('Set-Cookie', 'hello1=world1');
+	return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/response-empty-headers-object.ts

@@ -0,0 +1,4 @@
+export async function get({ request }: { request: Request }) {
+  const headers = new Headers();
+	return new Response('hello world', { headers });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/response-undefined-headers-object.ts

@@ -0,0 +1,3 @@
+export async function get({ request }: { request: Request }) {
+	return new Response('hello world', { headers: undefined });
+}

packages/integrations/node/test/fixtures/headers/src/pages/endpoints/simple.ts

@@ -0,0 +1,6 @@
+export async function get({ request }: { request: Request }) {
+  const headers = new Headers();
+  headers.append('content-type', 'text/plain;charset=utf-8');
+  headers.append('X-HELLO', 'world');
+	return new Response('hello world', { headers });
+}