fix(server/node-fetch): iterate set cookie headers individually

ardatan · Nov 23, 2023 · ea508c5 · ea508c5
1 parent fc49f7c
commit ea508c5
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 53 deletions.
diff --git a/.changeset/stupid-rats-judge.md b/.changeset/stupid-rats-judge.md
@@ -0,0 +1,6 @@
+---
+'@whatwg-node/node-fetch': patch
+'@whatwg-node/server': patch
+---
+
+Iterate set-cookie headers correctly
diff --git a/packages/node-fetch/src/Headers.ts b/packages/node-fetch/src/Headers.ts
@@ -10,22 +10,26 @@ export class PonyfillHeaders implements Headers {
   private _map: Map<string, string> | undefined;
   private objectNormalizedKeysOfHeadersInit: string[] = [];
   private objectOriginalKeysOfHeadersInit: string[] = [];
+  private _setCookies: string[] = [];
 
   constructor(private headersInit?: PonyfillHeadersInit) {}
 
   // perf: we don't need to build `this.map` for Requests, as we can access the headers directly
   private _get(key: string) {
+    const normalized = key.toLowerCase();
+    if (normalized === 'set-cookie') {
+      return this._setCookies.join(', ');
+    }
     // If the map is built, reuse it
     if (this._map) {
-      return this._map.get(key.toLowerCase()) || null;
+      return this._map.get(normalized) || null;
     }
 
     // If the map is not built, try to get the value from the this.headersInit
     if (this.headersInit == null) {
       return null;
     }
 
-    const normalized = key.toLowerCase();
     if (Array.isArray(this.headersInit)) {
       return this.headersInit.find(header => header[0].toLowerCase() === normalized)?.[1] || null;
     } else if (isHeadersLike(this.headersInit)) {
@@ -58,10 +62,22 @@ export class PonyfillHeaders implements Headers {
     if (!this._map) {
       if (this.headersInit != null) {
         if (Array.isArray(this.headersInit)) {
-          this._map = new Map(this.headersInit);
+          this._map = new Map();
+          this.headersInit.forEach(([key, value]) => {
+            const normalizedKey = key.toLowerCase();
+            if (normalizedKey === 'set-cookie') {
+              this._setCookies.push(value);
+              return;
+            }
+            this._map!.set(normalizedKey, value);
+          });
         } else if (isHeadersLike(this.headersInit)) {
           this._map = new Map();
           this.headersInit.forEach((value, key) => {
+            if (key === 'set-cookie') {
+              this._setCookies.push(value);
+              return;
+            }
             this._map!.set(key, value);
           });
         } else {
@@ -70,6 +86,10 @@ export class PonyfillHeaders implements Headers {
             const initValue = this.headersInit[initKey];
             if (initValue != null) {
               const normalizedKey = initKey.toLowerCase();
+              if (normalizedKey === 'set-cookie') {
+                this._setCookies.push(initValue);
+                continue;
+              }
               this._map.set(normalizedKey, initValue);
             }
           }
@@ -84,6 +104,10 @@ export class PonyfillHeaders implements Headers {
 
   append(name: string, value: string): void {
     const key = name.toLowerCase();
+    if (key === 'set-cookie') {
+      this._setCookies.push(value);
+      return;
+    }
     const existingValue = this.getMap().get(key);
     const finalValue = existingValue ? `${existingValue}, ${value}` : value;
     this.getMap().set(key, finalValue);
@@ -100,20 +124,34 @@ export class PonyfillHeaders implements Headers {
   }
 
   has(name: string): boolean {
+    if (name === 'set-cookie') {
+      return this._setCookies.length > 0;
+    }
     return !!this._get(name); // we might need to check if header exists and not just check if it's not nullable
   }
 
   set(name: string, value: string): void {
     const key = name.toLowerCase();
+    if (key === 'set-cookie') {
+      this._setCookies = [value];
+      return;
+    }
     this.getMap().set(key, value);
   }
 
   delete(name: string): void {
     const key = name.toLowerCase();
+    if (key === 'set-cookie') {
+      this._setCookies = [];
+      return;
+    }
     this.getMap().delete(key);
   }
 
   forEach(callback: (value: string, key: string, parent: Headers) => void): void {
+    this._setCookies.forEach(setCookie => {
+      callback(setCookie, 'set-cookie', this);
+    });
     if (!this._map) {
       if (this.headersInit) {
         if (Array.isArray(this.headersInit)) {
@@ -139,57 +177,67 @@ export class PonyfillHeaders implements Headers {
     });
   }
 
-  keys(): IterableIterator<string> {
+  *keys(): IterableIterator<string> {
+    if (this._setCookies.length) {
+      yield 'set-cookie';
+    }
     if (!this._map) {
       if (this.headersInit) {
         if (Array.isArray(this.headersInit)) {
-          return this.headersInit.map(([key]) => key)[Symbol.iterator]();
+          yield* this.headersInit.map(([key]) => key)[Symbol.iterator]();
+          return;
         }
         if (isHeadersLike(this.headersInit)) {
-          return this.headersInit.keys();
+          yield* this.headersInit.keys();
+          return;
         }
-        return Object.keys(this.headersInit)[Symbol.iterator]();
+        yield* Object.keys(this.headersInit)[Symbol.iterator]();
+        return;
       }
     }
-    return this.getMap().keys();
+    yield* this.getMap().keys();
   }
 
-  values(): IterableIterator<string> {
+  *values(): IterableIterator<string> {
+    yield* this._setCookies;
     if (!this._map) {
       if (this.headersInit) {
         if (Array.isArray(this.headersInit)) {
-          return this.headersInit.map(([, value]) => value)[Symbol.iterator]();
+          yield* this.headersInit.map(([, value]) => value)[Symbol.iterator]();
+          return;
         }
         if (isHeadersLike(this.headersInit)) {
-          return this.headersInit.values();
+          yield* this.headersInit.values();
+          return;
         }
-        return Object.values(this.headersInit)[Symbol.iterator]();
+        yield* Object.values(this.headersInit)[Symbol.iterator]();
+        return;
       }
     }
-    return this.getMap().values();
+    yield* this.getMap().values();
   }
 
-  entries(): IterableIterator<[string, string]> {
+  *entries(): IterableIterator<[string, string]> {
+    yield* this._setCookies.map(cookie => ['set-cookie', cookie] as [string, string]);
     if (!this._map) {
       if (this.headersInit) {
         if (Array.isArray(this.headersInit)) {
-          return this.headersInit[Symbol.iterator]();
+          yield* this.headersInit;
+          return;
         }
         if (isHeadersLike(this.headersInit)) {
-          return this.headersInit.entries();
+          yield* this.headersInit.entries();
+          return;
         }
-        return Object.entries(this.headersInit)[Symbol.iterator]();
+        yield* Object.entries(this.headersInit);
+        return;
       }
     }
-    return this.getMap().entries();
+    yield* this.getMap().entries();
   }
 
   getSetCookie() {
-    const setCookieHeader = this.get('set-cookie');
-    if (!setCookieHeader) {
-      return [];
-    }
-    return splitSetCookieHeader(setCookieHeader);
+    return this._setCookies;
   }
 
   [Symbol.iterator](): IterableIterator<[string, string]> {
@@ -200,40 +248,11 @@ export class PonyfillHeaders implements Headers {
     const record: Record<string, string[] | string> = {};
     this.forEach((value, key) => {
       if (key === 'set-cookie') {
-        record['set-cookie'] = this.getSetCookie();
+        record['set-cookie'] = this._setCookies;
       } else {
         record[key] = value.includes(',') ? value.split(',').map(el => el.trim()) : value;
       }
     });
     return `Headers ${inspect(record)}`;
   }
 }
-
-export function splitSetCookieHeader(setCookieHeader: string) {
-  const setCookieHeaders: string[] = [];
-  let currentStr = '';
-  let ignoreComma = false;
-  for (const ch of setCookieHeader) {
-    if (currentStr.endsWith('Expires=')) {
-      ignoreComma = true;
-    }
-    if (ignoreComma) {
-      if (ch === ';') {
-        ignoreComma = false;
-      }
-      if (ch === ',' && currentStr.split('Expires=')[1].length > 3) {
-        ignoreComma = false;
-      }
-    }
-    if (ch === ',' && !ignoreComma) {
-      setCookieHeaders.push(currentStr.trim());
-      currentStr = '';
-    } else {
-      currentStr += ch;
-    }
-  }
-  if (currentStr) {
-    setCookieHeaders.push(currentStr.trim());
-  }
-  return setCookieHeaders;
-}
diff --git a/packages/node-fetch/tests/Headers.spec.ts b/packages/node-fetch/tests/Headers.spec.ts
@@ -42,4 +42,17 @@ describe('Headers', () => {
     headers.set('X-Header', 'foo');
     expect(inspect(headers)).toBe("Headers { 'x-header': 'foo' }");
   });
+  it('should iterate each set-cookie individually', () => {
+    const headers = new PonyfillHeaders();
+    headers.append('set-cookie', 'foo');
+    headers.append('set-cookie', 'bar');
+    const headerEntries: [string, string][] = [];
+    headers.forEach((value, key) => {
+      headerEntries.push([key, value]);
+    });
+    expect(headerEntries).toEqual([
+      ['set-cookie', 'foo'],
+      ['set-cookie', 'bar'],
+    ]);
+  });
 });
diff --git a/packages/server/src/utils.ts b/packages/server/src/utils.ts
@@ -257,8 +257,13 @@ export function sendNodeResponse(
   serverResponse.statusCode = fetchResponse.status;
   serverResponse.statusMessage = fetchResponse.statusText;
 
+  let setCookiesSet = false;
   fetchResponse.headers.forEach((value, key) => {
     if (key === 'set-cookie') {
+      if (setCookiesSet) {
+        return;
+      }
+      setCookiesSet = true;
       const setCookies = fetchResponse.headers.getSetCookie?.();
       if (setCookies) {
         serverResponse.setHeader('set-cookie', setCookies);