Drop Node 14 in CI for Node 16 and add Node 18 to the matrix (#5768)

* ci(node): Move CI to Node 16 and add Node 18 to the matrix

* fix(netlify): Fix set-cookie not working on Node 18

* fix(netlify): Handle if `set-cookie` is already somehow an array (apparently it can?)

* test(node): Fix `toPromise` to match Astro's

* fix(tests): Use the actual underlying ArrayBuffer instance to create the buffer in toPromise

* chore: changeset

.changeset/modern-bulldogs-film.md

@@ -0,0 +1,5 @@
+---
+'@astrojs/netlify': patch
+---
+
+Fix set-cookies not working in certain cases when using Node 18+

.github/workflows/benchmark.yml

@@ -16,13 +16,13 @@ jobs:
  permissions:
  contents: read
  outputs:
- PR-BENCH-14: ${{ steps.benchmark-pr.outputs.BENCH_RESULT14 }}
  PR-BENCH-16: ${{ steps.benchmark-pr.outputs.BENCH_RESULT16 }}
- MAIN-BENCH-14: ${{ steps.benchmark-main.outputs.BENCH_RESULT14 }}
+ PR-BENCH-18: ${{ steps.benchmark-pr.outputs.BENCH_RESULT18 }}
  MAIN-BENCH-16: ${{ steps.benchmark-main.outputs.BENCH_RESULT16 }}
+ MAIN-BENCH-18: ${{ steps.benchmark-main.outputs.BENCH_RESULT18 }}
  strategy:
  matrix:
- node-version: [14, 16]
+ node-version: [16, 18]
  steps:
  - uses: actions/checkout@v3
  with:
@@ -89,12 +89,12 @@ jobs:
  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
  pr_number: ${{ github.event.issue.number }}
  message: |
- **Node**: 14
- **PR**: ${{ needs.benchmark.outputs.PR-BENCH-14 }}
- **MAIN**: ${{ needs.benchmark.outputs.MAIN-BENCH-14 }}
- 
- ---
- 
  **Node**: 16
  **PR**: ${{ needs.benchmark.outputs.PR-BENCH-16 }}
  **MAIN**: ${{ needs.benchmark.outputs.MAIN-BENCH-16 }}
+
+ ---
+
+ **Node**: 18
+ **PR**: ${{ needs.benchmark.outputs.PR-BENCH-18 }}
+ **MAIN**: ${{ needs.benchmark.outputs.MAIN-BENCH-18 }}

.github/workflows/ci.yml

@@ -82,7 +82,7 @@ jobs:
  strategy:
  matrix:
  OS: [ubuntu-latest]
- NODE_VERSION: [14]
+ NODE_VERSION: [16]
  fail-fast: true
  steps:
  - name: Checkout
@@ -111,11 +111,10 @@ jobs:
  strategy:
  matrix:
  OS: [ubuntu-latest, windows-latest]
- # TODO: Enable node@18!
- NODE_VERSION: [14, 16]
+ NODE_VERSION: [16, 18]
  include:
  - os: macos-latest
- NODE_VERSION: 14
+ NODE_VERSION: 16
  fail-fast: false
  env:
  NODE_VERSION: ${{ matrix.NODE_VERSION }}
@@ -154,7 +153,7 @@ jobs:
  strategy:
  matrix:
  OS: [ubuntu-latest, windows-latest]
- NODE_VERSION: [14]
+ NODE_VERSION: [16]
  fail-fast: false
  env:
  NODE_VERSION: ${{ matrix.NODE_VERSION }}
@@ -188,7 +187,7 @@ jobs:
  strategy:
  matrix:
  OS: [ubuntu-latest, windows-latest]
- NODE_VERSION: [14]
+ NODE_VERSION: [16]
  env:
  NODE_VERSION: ${{ matrix.NODE_VERSION }}
  steps:

packages/astro/test/units/test-utils.js

@@ -1,9 +1,9 @@
-import httpMocks from 'node-mocks-http';
 import { EventEmitter } from 'events';
 import { Volume } from 'memfs';
+import httpMocks from 'node-mocks-http';
 import realFS from 'node:fs';
-import { fileURLToPath } from 'url';
 import npath from 'path';
+import { fileURLToPath } from 'url';
 import { unixify } from './correct-path.js';
 
 class VirtualVolume extends Volume {
@@ -125,7 +125,7 @@ export function toPromise(res) {
  const write = res.write;
  res.write = function (data, encoding) {
  if (ArrayBuffer.isView(data) && !Buffer.isBuffer(data)) {
- data = Buffer.from(data);
+ data = Buffer.from(data.buffer);
  }
  return write.call(this, data, encoding);
  };

packages/integrations/netlify/src/netlify-functions.ts

@@ -104,19 +104,29 @@ export const createExports = (manifest: SSRManifest, args: Args) => {
  // The fetch API does not have a way to get multiples of a single header, but instead concatenates
  // them. There are non-standard ways to do it, and node-fetch gives us headers.raw()
  // See https://github.com/whatwg/fetch/issues/973 for discussion
- if (response.headers.has('set-cookie') && 'raw' in response.headers) {
- // Node fetch allows you to get the raw headers, which includes multiples of the same type.
- // This is needed because Set-Cookie *must* be called for each cookie, and can't be
- // concatenated together.
- type HeadersWithRaw = Headers & {
- raw: () => Record<string, string[]>;
- };
-
- const rawPacked = (response.headers as HeadersWithRaw).raw();
- if ('set-cookie' in rawPacked) {
- fnResponse.multiValueHeaders = {
- 'set-cookie': rawPacked['set-cookie'],
+ if (response.headers.has('set-cookie')) {
+ if ('raw' in response.headers) {
+ // Node fetch allows you to get the raw headers, which includes multiples of the same type.
+ // This is needed because Set-Cookie *must* be called for each cookie, and can't be
+ // concatenated together.
+ type HeadersWithRaw = Headers & {
+ raw: () => Record<string, string[]>;
  };
+
+ const rawPacked = (response.headers as HeadersWithRaw).raw();
+ if ('set-cookie' in rawPacked) {
+ fnResponse.multiValueHeaders = {
+ 'set-cookie': rawPacked['set-cookie'],
+ };
+ }
+ } else {
+ const cookies = response.headers.get('set-cookie');
+
+ if (cookies) {
+ fnResponse.multiValueHeaders = {
+ 'set-cookie': Array.isArray(cookies) ? cookies : splitCookiesString(cookies),
+ };
+ }
  }
  }
 
@@ -135,3 +145,86 @@ export const createExports = (manifest: SSRManifest, args: Args) => {
 
  return { handler };
 };
+
+/*
+ From: https://github.com/nfriedly/set-cookie-parser/blob/5cae030d8ef0f80eec58459e3583d43a07b984cb/lib/set-cookie.js#L144
+ Set-Cookie header field-values are sometimes comma joined in one string. This splits them without choking on commas
+ that are within a single set-cookie field-value, such as in the Expires portion.
+ This is uncommon, but explicitly allowed - see https://tools.ietf.org/html/rfc2616#section-4.2
+ Node.js does this for every header *except* set-cookie - see https://github.com/nodejs/node/blob/d5e363b77ebaf1caf67cd7528224b651c86815c1/lib/_http_incoming.js#L128
+ React Native's fetch does this for *every* header, including set-cookie.
+ Based on: https://github.com/google/j2objc/commit/16820fdbc8f76ca0c33472810ce0cb03d20efe25
+ Credits to: https://github.com/tomball for original and https://github.com/chrusart for JavaScript implementation
+*/
+function splitCookiesString(cookiesString: string): string[] {
+ if (Array.isArray(cookiesString)) {
+ return cookiesString;
+ }
+ if (typeof cookiesString !== 'string') {
+ return [];
+ }
+
+ let cookiesStrings = [];
+ let pos = 0;
+ let start;
+ let ch;
+ let lastComma;
+ let nextStart;
+ let cookiesSeparatorFound;
+
+ function skipWhitespace() {
+ while (pos < cookiesString.length && /\s/.test(cookiesString.charAt(pos))) {
+ pos += 1;
+ }
+ return pos < cookiesString.length;
+ }
+
+ function notSpecialChar() {
+ ch = cookiesString.charAt(pos);
+
+ return ch !== '=' && ch !== ';' && ch !== ',';
+ }
+
+ while (pos < cookiesString.length) {
+ start = pos;
+ cookiesSeparatorFound = false;
+
+ while (skipWhitespace()) {
+ ch = cookiesString.charAt(pos);
+ if (ch === ',') {
+ // ',' is a cookie separator if we have later first '=', not ';' or ','
+ lastComma = pos;
+ pos += 1;
+
+ skipWhitespace();
+ nextStart = pos;
+
+ while (pos < cookiesString.length && notSpecialChar()) {
+ pos += 1;
+ }
+
+ // currently special character
+ if (pos < cookiesString.length && cookiesString.charAt(pos) === '=') {
+ // we found cookies separator
+ cookiesSeparatorFound = true;
+ // pos is inside the next cookie, so back up and return it.
+ pos = nextStart;
+ cookiesStrings.push(cookiesString.substring(start, lastComma));
+ start = pos;
+ } else {
+ // in param ',' or param separator ';',
+ // we continue from that comma
+ pos = lastComma + 1;
+ }
+ } else {
+ pos += 1;
+ }
+ }
+
+ if (!cookiesSeparatorFound || pos >= cookiesString.length) {
+ cookiesStrings.push(cookiesString.substring(start, cookiesString.length));
+ }
+ }
+
+ return cookiesStrings;
+}

packages/integrations/node/test/test-utils.js

@@ -1,6 +1,6 @@
-import { loadFixture as baseLoadFixture } from '../../../astro/test/test-utils.js';
-import httpMocks from 'node-mocks-http';
 import { EventEmitter } from 'events';
+import httpMocks from 'node-mocks-http';
+import { loadFixture as baseLoadFixture } from '../../../astro/test/test-utils.js';
 
 /**
  * @typedef {import('../../../astro/test/test-utils').Fixture} Fixture
@@ -33,6 +33,15 @@ export function createRequestAndResponse(reqOptions) {
 
 export function toPromise(res) {
  return new Promise((resolve) => {
+ // node-mocks-http doesn't correctly handle non-Buffer typed arrays,
+ // so override the write method to fix it.
+ const write = res.write;
+ res.write = function (data, encoding) {
+ if (ArrayBuffer.isView(data) && !Buffer.isBuffer(data)) {
+ data = Buffer.from(data.buffer);
+ }
+ return write.call(this, data, encoding);
+ };
  res.on('end', () => {
  let chunks = res._getChunks();
  resolve(chunks);