From 46a7404e6c217228f83285e3395a95bb6064b038 Mon Sep 17 00:00:00 2001 From: Pedro Del Moral Lopez Date: Fri, 5 Jun 2026 11:29:14 -0400 Subject: [PATCH] fix: preserve response cookie intent --- .changeset/cookie-intent-semantics.md | 6 + .../express/src/adapter/ExpressAdapter.ts | 6 + .../src/adapter/tests/ExpressAdapter.test.ts | 27 ++ packages/core/src/http/TangoHeaders.ts | 232 +++++++++++++----- packages/core/src/http/TangoResponse.ts | 19 +- .../core/src/http/tests/TangoHeaders.test.ts | 67 +++++ .../core/src/http/tests/TangoResponse.test.ts | 27 ++ 7 files changed, 315 insertions(+), 69 deletions(-) create mode 100644 .changeset/cookie-intent-semantics.md diff --git a/.changeset/cookie-intent-semantics.md b/.changeset/cookie-intent-semantics.md new file mode 100644 index 00000000..c7572387 --- /dev/null +++ b/.changeset/cookie-intent-semantics.md @@ -0,0 +1,6 @@ +--- +"@danceroutine/tango-core": patch +"@danceroutine/tango-adapters-express": patch +--- + +Fix response cookie intent handling so repeated `setCookie()` calls replace the prior cookie for the same name, domain, and path while `appendCookie()` continues to emit additional `Set-Cookie` lines. Express responses now forward multiple `Set-Cookie` lines without collapsing them into a single header value. diff --git a/packages/adapters/express/src/adapter/ExpressAdapter.ts b/packages/adapters/express/src/adapter/ExpressAdapter.ts index d8f568d7..ac1ada5f 100644 --- a/packages/adapters/express/src/adapter/ExpressAdapter.ts +++ b/packages/adapters/express/src/adapter/ExpressAdapter.ts @@ -256,9 +256,15 @@ export class ExpressAdapter implements FrameworkAdapter { + if (key.toLowerCase() === 'set-cookie') return; res.setHeader(key, value); }); + const setCookie = tangoResponse.headers.getSetCookie(); + if (setCookie.length > 0) { + res.setHeader('set-cookie', setCookie); + } + if (response.body === null) { res.end(); return; diff --git a/packages/adapters/express/src/adapter/tests/ExpressAdapter.test.ts b/packages/adapters/express/src/adapter/tests/ExpressAdapter.test.ts index 318d3cee..9384a5e2 100644 --- a/packages/adapters/express/src/adapter/tests/ExpressAdapter.test.ts +++ b/packages/adapters/express/src/adapter/tests/ExpressAdapter.test.ts @@ -107,6 +107,33 @@ describe(ExpressAdapter, () => { expect(next).not.toHaveBeenCalled(); }); + it('forwards set-cookie header lines as one Express header array', async () => { + const adapter = new ExpressAdapter(); + const routeHandler = adapter.adapt(async () => { + const response = textResponse('ok'); + response.setCookie('session', 'old'); + response.setCookie('session', 'new'); + response.appendCookie('theme', 'dark'); + return response; + }); + + const req = anExpressRequest({ + originalUrl: '/users', + url: '/users', + }); + const res = anExpressResponse(); + const next = vi.fn() as unknown as NextFunction; + + await routeHandler(req, res, next); + + const setHeader = vi.mocked(res.setHeader); + const setCookieCalls = setHeader.mock.calls.filter(([name]) => String(name).toLowerCase() === 'set-cookie'); + + expect(setCookieCalls).toEqual([['set-cookie', ['session=new; Path=/', 'theme=dark; Path=/']]]); + expect(res.send).toHaveBeenCalledWith('ok'); + expect(next).not.toHaveBeenCalled(); + }); + it('normalizes Express requests into Tango query params', () => { const adapter = new ExpressAdapter(); const req = anExpressRequest({ diff --git a/packages/core/src/http/TangoHeaders.ts b/packages/core/src/http/TangoHeaders.ts index 61427903..355b9c30 100644 --- a/packages/core/src/http/TangoHeaders.ts +++ b/packages/core/src/http/TangoHeaders.ts @@ -1,5 +1,30 @@ import { isArrayBuffer, isBlob, isUint8Array } from '../runtime/index'; +type CookieOptions = { + domain?: string; + expires?: Date; + httpOnly?: boolean; + maxAge?: number; + path?: string; + sameSite?: 'Strict' | 'Lax' | 'None'; + secure?: boolean; + priority?: 'Low' | 'Medium' | 'High'; + partitioned?: boolean; +}; + +type DeleteCookieOptions = Omit; + +type CookieIdentity = { + name: string; + domain: string; + path: string; +}; + +type CookieEntry = { + line: string; + identity: CookieIdentity | null; +}; + /** * TangoHeaders extends the Web Headers class, adding ergonomic helpers * for common HTTP header patterns, convenience features, and a consistent API for @@ -17,6 +42,12 @@ import { isArrayBuffer, isBlob, isUint8Array } from '../runtime/index'; export class TangoHeaders extends Headers { static readonly BRAND = 'tango.http.headers' as const; readonly __tangoBrand: typeof TangoHeaders.BRAND = TangoHeaders.BRAND; + private cookieEntries: CookieEntry[] = []; + + constructor(init?: HeadersInit) { + super(); + this.appendHeadersInit(init); + } /** * Narrow an unknown value to `TangoHeaders`. @@ -32,21 +63,7 @@ export class TangoHeaders extends Headers { /** * Serialize a cookie for the Set-Cookie header line. */ - private static serializeCookie( - name: string, - value: string, - options: { - domain?: string; - expires?: Date; - httpOnly?: boolean; - maxAge?: number; - path?: string; - sameSite?: 'Strict' | 'Lax' | 'None'; - secure?: boolean; - priority?: 'Low' | 'Medium' | 'High'; - partitioned?: boolean; - } = {} - ): string { + private static serializeCookie(name: string, value: string, options: CookieOptions = {}): string { let cookie = encodeURIComponent(name) + '=' + encodeURIComponent(value ?? ''); if (options.domain) cookie += `; Domain=${options.domain}`; if (options.path) cookie += `; Path=${options.path}`; @@ -61,6 +78,34 @@ export class TangoHeaders extends Headers { return cookie; } + private static getCookieIdentity( + name: string, + options: Pick = {} + ): CookieIdentity { + return { + name, + domain: options.domain?.toLowerCase() ?? '', + path: options.path ?? '/', + }; + } + + private static hasCookieIdentity(entry: CookieEntry, identity: CookieIdentity): boolean { + return ( + entry.identity !== null && + entry.identity.name === identity.name && + entry.identity.domain === identity.domain && + entry.identity.path === identity.path + ); + } + + private static isSetCookieName(name: string): boolean { + return name.toLowerCase() === 'set-cookie'; + } + + private static hasHeaderEntries(value: HeadersInit): value is Headers { + return typeof (value as { entries?: unknown }).entries === 'function'; + } + private static hasNumberSize(value: unknown): value is { size: number } { return typeof value === 'object' && value !== null && typeof (value as { size?: unknown }).size === 'number'; } @@ -78,6 +123,36 @@ export class TangoHeaders extends Headers { ); } + override append(name: string, value: string): void { + if (!TangoHeaders.isSetCookieName(name)) { + super.append(name, value); + return; + } + + this.cookieEntries.push({ line: value, identity: null }); + this.syncSetCookieHeader(); + } + + override set(name: string, value: string): void { + if (!TangoHeaders.isSetCookieName(name)) { + super.set(name, value); + return; + } + + this.cookieEntries = [{ line: value, identity: null }]; + this.syncSetCookieHeader(); + } + + override delete(name: string): void { + if (!TangoHeaders.isSetCookieName(name)) { + super.delete(name); + return; + } + + this.cookieEntries = []; + super.delete('Set-Cookie'); + } + /** * Sets the Content-Disposition header with type "inline" and the specified filename. * This is useful to indicate that the content should be displayed inline in the browser, @@ -107,8 +182,15 @@ export class TangoHeaders extends Headers { clone(): TangoHeaders { const copy = new TangoHeaders(); for (const [name, value] of this.entries()) { - copy.append(name, value); + if (!TangoHeaders.isSetCookieName(name)) { + copy.append(name, value); + } } + copy.cookieEntries = this.cookieEntries.map((entry) => ({ + line: entry.line, + identity: entry.identity === null ? null : { ...entry.identity }, + })); + copy.syncSetCookieHeader(); return copy; } @@ -175,64 +257,35 @@ export class TangoHeaders extends Headers { } /** - * Set a cookie header (for Set-Cookie). - * @param name - * @param value - * @param options + * Set a `Set-Cookie` line, replacing prior helper-managed intent for the same name, domain, and path. */ - setCookie( - name: string, - value: string, - options?: { - domain?: string; - expires?: Date; - httpOnly?: boolean; - maxAge?: number; - path?: string; - sameSite?: 'Strict' | 'Lax' | 'None'; - secure?: boolean; - priority?: 'Low' | 'Medium' | 'High'; - partitioned?: boolean; - } - ): void { - this.append('Set-Cookie', TangoHeaders.serializeCookie(name, value, options)); + setCookie(name: string, value: string, options?: CookieOptions): void { + const identity = TangoHeaders.getCookieIdentity(name, options); + this.replaceCookieIdentity(identity, TangoHeaders.serializeCookie(name, value, options)); } /** - * Append (additionally) a new cookie. + * Append another `Set-Cookie` line without replacing earlier cookie intent. */ - appendCookie( - name: string, - value: string, - options?: { - domain?: string; - expires?: Date; - httpOnly?: boolean; - maxAge?: number; - path?: string; - sameSite?: 'Strict' | 'Lax' | 'None'; - secure?: boolean; - priority?: 'Low' | 'Medium' | 'High'; - partitioned?: boolean; - } - ): void { - this.append('Set-Cookie', TangoHeaders.serializeCookie(name, value, options)); + appendCookie(name: string, value: string, options?: CookieOptions): void { + this.cookieEntries.push({ + line: TangoHeaders.serializeCookie(name, value, options), + identity: TangoHeaders.getCookieIdentity(name, options), + }); + this.syncSetCookieHeader(); } /** - * Delete a cookie ("unset" it via expired date). + * Return each `Set-Cookie` header line separately. */ - deleteCookie( - name: string, - options?: { - domain?: string; - path?: string; - sameSite?: 'Strict' | 'Lax' | 'None'; - secure?: boolean; - priority?: 'Low' | 'Medium' | 'High'; - partitioned?: boolean; - } - ): void { + override getSetCookie(): string[] { + return this.cookieEntries.map((entry) => entry.line); + } + + /** + * Expire a cookie, replacing prior helper-managed intent for the same name, domain, and path. + */ + deleteCookie(name: string, options?: DeleteCookieOptions): void { this.setCookie(name, '', { ...options, expires: new Date(0), @@ -474,4 +527,53 @@ export class TangoHeaders extends Headers { getResponseTime(): string | null { return this.get('X-Response-Time'); } + + private appendHeadersInit(init: HeadersInit | undefined): void { + if (!init) return; + + if (TangoHeaders.isTangoHeaders(init)) { + for (const [name, value] of init.entries()) { + if (!TangoHeaders.isSetCookieName(name)) { + super.append(name, value); + } + } + this.cookieEntries = init.cookieEntries.map((entry) => ({ + line: entry.line, + identity: entry.identity === null ? null : { ...entry.identity }, + })); + this.syncSetCookieHeader(); + return; + } + + if (Array.isArray(init)) { + for (const [name, value] of init) { + this.append(name, value); + } + return; + } + + if (TangoHeaders.hasHeaderEntries(init)) { + for (const [name, value] of init.entries()) { + this.append(name, value); + } + return; + } + + for (const [name, value] of Object.entries(init)) { + this.append(name, value); + } + } + + private replaceCookieIdentity(identity: CookieIdentity, line: string): void { + this.cookieEntries = this.cookieEntries.filter((entry) => !TangoHeaders.hasCookieIdentity(entry, identity)); + this.cookieEntries.push({ line, identity }); + this.syncSetCookieHeader(); + } + + private syncSetCookieHeader(): void { + super.delete('Set-Cookie'); + for (const entry of this.cookieEntries) { + super.append('Set-Cookie', entry.line); + } + } } diff --git a/packages/core/src/http/TangoResponse.ts b/packages/core/src/http/TangoResponse.ts index 7e5dcfa8..46dc5ac7 100644 --- a/packages/core/src/http/TangoResponse.ts +++ b/packages/core/src/http/TangoResponse.ts @@ -706,19 +706,19 @@ export class TangoResponse implements Response { } /** - * Add a `Set-Cookie` header that replaces prior application intent. + * Add a `Set-Cookie` header that replaces prior helper-managed intent for the same name, domain, and path. */ setCookie(name: string, value: string, options?: Parameters[2]): void { this.headers.setCookie(name, value, options); } /** - * Append another `Set-Cookie` header. + * Append another `Set-Cookie` header without replacing earlier cookie intent. */ appendCookie(name: string, value: string, options?: Parameters[2]): void { this.headers.appendCookie(name, value, options); } /** - * Expire a cookie by issuing a matching deletion cookie header. + * Expire a cookie by replacing prior helper-managed intent for the same name, domain, and path. */ deleteCookie(name: string, options?: Parameters[1]): void { this.headers.deleteCookie(name, options); @@ -847,9 +847,20 @@ export class TangoResponse implements Response { toWebResponse(): Response { const responseForTransfer = !this.bodyUsed && isReadableStream(this.bodySource) ? this.clone() : this; const body = TangoResponse.normalizeWebBody(responseForTransfer.bodySource); + const headers = new Headers(); + + responseForTransfer.headers.forEach((value, key) => { + if (key.toLowerCase() !== 'set-cookie') { + headers.append(key, value); + } + }); + + for (const cookie of responseForTransfer.headers.getSetCookie()) { + headers.append('Set-Cookie', cookie); + } return new Response(body, { - headers: new Headers(responseForTransfer.headers), + headers, status: responseForTransfer.status, statusText: responseForTransfer.statusText, }); diff --git a/packages/core/src/http/tests/TangoHeaders.test.ts b/packages/core/src/http/tests/TangoHeaders.test.ts index c37429ca..a6e67033 100644 --- a/packages/core/src/http/tests/TangoHeaders.test.ts +++ b/packages/core/src/http/tests/TangoHeaders.test.ts @@ -78,6 +78,73 @@ describe(TangoHeaders, () => { expect(headers.get('Content-Type')).toBe('application/json'); }); + it('replaces helper-managed cookie intent by name, domain, and path', () => { + const headers = new TangoHeaders(); + + headers.setCookie('session', 'old'); + headers.setCookie('session', 'new'); + headers.setCookie('session', 'api', { path: '/api' }); + headers.setCookie('session', 'domain', { domain: 'example.com' }); + headers.appendCookie('session', 'extra'); + headers.deleteCookie('session'); + + const setCookie = headers.getSetCookie(); + expect(setCookie).not.toContain('session=old; Path=/'); + expect(setCookie).not.toContain('session=new; Path=/'); + expect(setCookie).toContain('session=api; Path=/api'); + expect(setCookie).toContain('session=domain; Domain=example.com; Path=/'); + expect(setCookie.filter((cookie) => cookie.startsWith('session=extra;'))).toHaveLength(0); + expect(setCookie).toContain('session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Max-Age=0'); + }); + + it('preserves appended and cloned cookie lines', () => { + const headers = new TangoHeaders(); + headers.appendCookie('session', 'one'); + headers.appendCookie('session', 'two'); + headers.append('Set-Cookie', 'raw=1; Path=/'); + + const copy = headers.clone(); + copy.setCookie('session', 'three'); + + expect(headers.getSetCookie()).toEqual(['session=one; Path=/', 'session=two; Path=/', 'raw=1; Path=/']); + expect(copy.getSetCookie()).toEqual(['raw=1; Path=/', 'session=three; Path=/']); + }); + + it('preserves set-cookie lines from header init values and raw mutations', () => { + const source = new TangoHeaders(); + source.setHeader('X-Test', 'source'); + source.setCookie('session', 'source'); + source.append('Set-Cookie', 'raw=1; Path=/'); + + const fromTangoHeaders = new TangoHeaders(source); + expect(fromTangoHeaders.get('X-Test')).toBe('source'); + expect(fromTangoHeaders.getSetCookie()).toEqual(['session=source; Path=/', 'raw=1; Path=/']); + + const native = new Headers(); + native.set('X-Test', 'native'); + native.append('Set-Cookie', 'native=1; Path=/'); + const fromNativeHeaders = new TangoHeaders(native); + expect(fromNativeHeaders.get('X-Test')).toBe('native'); + expect(fromNativeHeaders.getSetCookie()).toEqual(['native=1; Path=/']); + + const fromArray = new TangoHeaders([ + ['X-Test', 'array'], + ['Set-Cookie', 'array=1; Path=/'], + ]); + expect(fromArray.get('X-Test')).toBe('array'); + expect(fromArray.getSetCookie()).toEqual(['array=1; Path=/']); + + const fromObject = new TangoHeaders({ 'Set-Cookie': 'object=1; Path=/' }); + expect(fromObject.getSetCookie()).toEqual(['object=1; Path=/']); + + fromArray.set('Set-Cookie', 'reset=1; Path=/'); + expect(fromArray.getSetCookie()).toEqual(['reset=1; Path=/']); + + fromArray.delete('Set-Cookie'); + expect(fromArray.getSetCookie()).toEqual([]); + expect(fromArray.has('Set-Cookie')).toBe(false); + }); + it('infers content metadata and preserves trace headers', () => { const headers = new TangoHeaders(); headers.setContentTypeByFile('x', 'a.txt'); diff --git a/packages/core/src/http/tests/TangoResponse.test.ts b/packages/core/src/http/tests/TangoResponse.test.ts index b16f8476..85c7f506 100644 --- a/packages/core/src/http/tests/TangoResponse.test.ts +++ b/packages/core/src/http/tests/TangoResponse.test.ts @@ -26,6 +26,11 @@ function streamFromText(text: string): ReadableStream { }); } +function getSetCookie(headers: Headers): string[] { + const withSetCookie = headers as Headers & { getSetCookie?: () => string[] }; + return withSetCookie.getSetCookie?.() ?? []; +} + describe(TangoResponse, () => { it('identifies tango responses and uses the default response state', () => { const response = new TangoResponse(); @@ -74,6 +79,28 @@ describe(TangoResponse, () => { expect(response.headers.get('server-timing')).toBe('c'); }); + it('replaces helper-managed cookie intent and preserves cookie lines in web responses', () => { + const response = new TangoResponse(); + + response.setCookie('session', 'old'); + response.setCookie('session', 'new'); + response.appendCookie('theme', 'dark'); + response.setCookie('session', 'api', { path: '/api' }); + + expect(response.headers.getSetCookie()).toEqual([ + 'session=new; Path=/', + 'theme=dark; Path=/', + 'session=api; Path=/api', + ]); + + const webResponse = response.toWebResponse(); + expect(getSetCookie(webResponse.headers)).toEqual([ + 'session=new; Path=/', + 'theme=dark; Path=/', + 'session=api; Path=/api', + ]); + }); + it('serializes JSON, text, HTML, and stream bodies', async () => { const json = TangoResponse.json({ a: 1 }); expect(json.headers.get('Content-Type')).toContain('application/json');