From 969f0e12dda686d86516212b6f39600ca195cd84 Mon Sep 17 00:00:00 2001 From: Nate Iler Date: Wed, 4 Feb 2026 14:06:46 -0700 Subject: [PATCH 1/3] fix: support multiple attributes in set-cookie headers --- packages/event-handler/src/http/converters.ts | 20 +++++ .../tests/unit/http/converters.test.ts | 81 ++++++++++++++----- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/packages/event-handler/src/http/converters.ts b/packages/event-handler/src/http/converters.ts index 35e0575f5b..420e8dbda3 100644 --- a/packages/event-handler/src/http/converters.ts +++ b/packages/event-handler/src/http/converters.ts @@ -228,7 +228,27 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => { const headers: Record = {}; const multiValueHeaders: Record> = {}; + // Handle set-cookie headers specially using getSetCookie() + const setCookies = webHeaders.getSetCookie(); + // Some implementations may concatenate multiple set-cookie values with commas + // Split them out while preserving the cookie attributes (which use semicolons) + const allSetCookies: string[] = []; + for (const cookie of setCookies) { + allSetCookies.push(...cookie.split(',').map((v) => v.trimStart())); + } + + if (allSetCookies.length > 1) { + multiValueHeaders['set-cookie'] = allSetCookies; + } else if (allSetCookies.length === 1) { + headers['set-cookie'] = allSetCookies[0]; + } + for (const [key, value] of webHeaders.entries()) { + // Skip set-cookie as it's already handled above + if (key.toLowerCase() === 'set-cookie') { + continue; + } + const values = value.split(/[;,]/).map((v) => v.trimStart()); if (headers[key]) { diff --git a/packages/event-handler/tests/unit/http/converters.test.ts b/packages/event-handler/tests/unit/http/converters.test.ts index 558f94862c..1c4c6ee04b 100644 --- a/packages/event-handler/tests/unit/http/converters.test.ts +++ b/packages/event-handler/tests/unit/http/converters.test.ts @@ -773,7 +773,7 @@ describe('Converters', () => { const response = new Response('Hello', { status: 200, headers: { - 'Set-Cookie': 'cookie1=value1; cookie2=value2', + 'Set-Cookie': 'cookie1=value1; HttpOnly, cookie2=value2', 'Content-type': 'application/json', }, }); @@ -784,7 +784,7 @@ describe('Converters', () => { // Assess expect(result.headers).toEqual({ 'content-type': 'application/json' }); expect(result.multiValueHeaders).toEqual({ - 'set-cookie': ['cookie1=value1', 'cookie2=value2'], + 'set-cookie': ['cookie1=value1; HttpOnly', 'cookie2=value2'], }); }); @@ -881,7 +881,7 @@ describe('Converters', () => { const response = new Response('Hello', { status: 200, headers: { - 'Set-Cookie': 'cookie1=value1; cookie2=value2', + 'Set-Cookie': 'cookie1=value1; HttpOnly, cookie2=value2', 'Content-type': 'application/json', }, }); @@ -892,7 +892,7 @@ describe('Converters', () => { // Assess expect(result.headers).toEqual({ 'content-type': 'application/json' }); expect(result.multiValueHeaders).toEqual({ - 'set-cookie': ['cookie1=value1', 'cookie2=value2'], + 'set-cookie': ['cookie1=value1; HttpOnly', 'cookie2=value2'], }); }); @@ -960,7 +960,7 @@ describe('Converters', () => { const response = new Response('Hello', { status: 200, headers: { - 'Set-Cookie': 'session=abc, theme=dark', + 'Set-Cookie': 'session=abc; HttpOnly, theme=dark', 'content-type': 'application/json', }, }); @@ -970,7 +970,7 @@ describe('Converters', () => { // Assess expect(result.headers).toEqual({ 'content-type': 'application/json' }); - expect(result.cookies).toEqual(['session=abc', 'theme=dark']); + expect(result.cookies).toEqual(['session=abc; HttpOnly', 'theme=dark']); }); it('handles multiple Set-Cookie headers', async () => { @@ -978,7 +978,8 @@ describe('Converters', () => { const response = new Response('Hello', { status: 200, headers: { - 'Set-Cookie': 'cookie1=value1, cookie2=value2, cookie3=value3', + 'Set-Cookie': + 'cookie1=value1; HttpOnly; Max-Age=3600, cookie2=value2, cookie3=value3', }, }); @@ -987,7 +988,7 @@ describe('Converters', () => { // Assess expect(result.cookies).toEqual([ - 'cookie1=value1', + 'cookie1=value1; HttpOnly; Max-Age=3600', 'cookie2=value2', 'cookie3=value3', ]); @@ -1065,7 +1066,7 @@ describe('Converters', () => { body: 'test', headers: { 'content-type': 'application/json' }, multiValueHeaders: { - 'Set-Cookie': ['cookie1=value1', 'cookie2=value2'], + 'Cache-Control': ['no-cache', 'no-store'], }, isBase64Encoded: false, }; @@ -1076,9 +1077,7 @@ describe('Converters', () => { // Assess expect(result.status).toBe(HttpStatusCodes.OK); expect(result.headers.get('content-type')).toBe('application/json'); - expect(result.headers.get('Set-Cookie')).toBe( - 'cookie1=value1, cookie2=value2' - ); + expect(result.headers.get('Cache-Control')).toBe('no-cache, no-store'); }); it('converts plain object to JSON Response with default headers', () => { @@ -1247,7 +1246,7 @@ describe('Converters', () => { it('handles multi-value headers split by semicolon', () => { // Prepare const headers = new Headers({ - 'set-cookie': 'session=abc123; theme=dark', + 'cache-control': 'no-cache; no-store', }); // Act @@ -1257,7 +1256,7 @@ describe('Converters', () => { expect(result).toEqual({ headers: {}, multiValueHeaders: { - 'set-cookie': ['session=abc123', 'theme=dark'], + 'cache-control': ['no-cache', 'no-store'], }, }); }); @@ -1266,7 +1265,7 @@ describe('Converters', () => { // Prepare const headers = new Headers({ accept: 'application/json, text/html', - 'set-cookie': 'session=abc; theme=dark', + 'cache-control': 'no-cache; no-store', }); // Act @@ -1277,7 +1276,7 @@ describe('Converters', () => { headers: {}, multiValueHeaders: { accept: ['application/json', 'text/html'], - 'set-cookie': ['session=abc', 'theme=dark'], + 'cache-control': ['no-cache', 'no-store'], }, }); }); @@ -1341,7 +1340,7 @@ describe('Converters', () => { // Prepare const headers = new Headers({ accept: 'application/json, text/html ,text/plain', - 'set-cookie': 'session=abc; theme=dark ; user=john', + 'cache-control': 'no-cache ; no-store', }); // Act @@ -1352,7 +1351,7 @@ describe('Converters', () => { headers: {}, multiValueHeaders: { accept: ['application/json', 'text/html ', 'text/plain'], - 'set-cookie': ['session=abc', 'theme=dark ', 'user=john'], + 'cache-control': ['no-cache ', 'no-store'], }, }); }); @@ -1377,7 +1376,7 @@ describe('Converters', () => { 'content-type': 'application/json', accept: 'application/json, text/html', authorization: 'Bearer token123', - 'set-cookie': 'session=abc; theme=dark', + 'set-cookie': 'session=abc; HttpOnly, theme=dark', }); // Act @@ -1391,7 +1390,49 @@ describe('Converters', () => { }, multiValueHeaders: { accept: ['application/json', 'text/html'], - 'set-cookie': ['session=abc', 'theme=dark'], + 'set-cookie': ['session=abc; HttpOnly', 'theme=dark'], + }, + }); + }); + + it('handles single set-cookie header with multiple attributes', () => { + // Prepare + const headers = new Headers({ + 'set-cookie': 'session=abc; HttpOnly', + }); + + // Act + const result = webHeadersToApiGatewayHeaders(headers, 'ApiGatewayV1'); + + // Assess + expect(result).toEqual({ + headers: { + 'set-cookie': 'session=abc; HttpOnly', + }, + multiValueHeaders: {}, + }); + }); + + it('handles set-cookie headers with multiple attributes', () => { + // Prepare + const headers = new Headers({ + 'set-cookie': 'session=abc; HttpOnly, foo=bar; HttpOnly', + }); + + headers.append('set-cookie', 'theme=dark; Max-Age=3600'); + + // Act + const result = webHeadersToApiGatewayHeaders(headers, 'ApiGatewayV1'); + + // Assess + expect(result).toEqual({ + headers: {}, + multiValueHeaders: { + 'set-cookie': [ + 'session=abc; HttpOnly', + 'foo=bar; HttpOnly', + 'theme=dark; Max-Age=3600', + ], }, }); }); From f0fe4ef75d9faaed7d1489481f3bb1c351077c29 Mon Sep 17 00:00:00 2001 From: Nate Iler Date: Wed, 4 Feb 2026 14:50:05 -0700 Subject: [PATCH 2/3] fix: multiple header values are comma-separated except set-cookie --- packages/event-handler/src/http/converters.ts | 23 +- .../unit/http/Router/basic-routing.test.ts | 242 +++++++++--------- 2 files changed, 137 insertions(+), 128 deletions(-) diff --git a/packages/event-handler/src/http/converters.ts b/packages/event-handler/src/http/converters.ts index 420e8dbda3..08fd414660 100644 --- a/packages/event-handler/src/http/converters.ts +++ b/packages/event-handler/src/http/converters.ts @@ -229,18 +229,18 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => { const multiValueHeaders: Record> = {}; // Handle set-cookie headers specially using getSetCookie() - const setCookies = webHeaders.getSetCookie(); - // Some implementations may concatenate multiple set-cookie values with commas + const cookies = webHeaders.getSetCookie(); + // Some legacy implementations may concatenate multiple set-cookie values with commas // Split them out while preserving the cookie attributes (which use semicolons) - const allSetCookies: string[] = []; - for (const cookie of setCookies) { - allSetCookies.push(...cookie.split(',').map((v) => v.trimStart())); + const allCookies: string[] = []; + for (const cookie of cookies) { + allCookies.push(...cookie.split(',').map((v) => v.trimStart())); } - if (allSetCookies.length > 1) { - multiValueHeaders['set-cookie'] = allSetCookies; - } else if (allSetCookies.length === 1) { - headers['set-cookie'] = allSetCookies[0]; + if (allCookies.length > 1) { + multiValueHeaders['set-cookie'] = allCookies; + } else if (allCookies.length === 1) { + headers['set-cookie'] = allCookies[0]; } for (const [key, value] of webHeaders.entries()) { @@ -251,10 +251,7 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => { const values = value.split(/[;,]/).map((v) => v.trimStart()); - if (headers[key]) { - multiValueHeaders[key] = [headers[key], ...values]; - delete headers[key]; - } else if (values.length > 1) { + if (values.length > 1) { multiValueHeaders[key] = values; } else { headers[key] = value; diff --git a/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts b/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts index e6e72f388d..6d5bb37a8b 100644 --- a/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts +++ b/packages/event-handler/tests/unit/http/Router/basic-routing.test.ts @@ -29,73 +29,71 @@ describe.each([ ['HEAD', 'head'], ['OPTIONS', 'options'], ]; - it.each(httpMethods)( - 'routes %s requests with object response', - async (method, verb) => { - // Prepare - const app = new Router(); - ( - app[verb as Lowercase] as ( - path: string, - handler: RouteHandler - ) => void - )('/test', async () => ({ result: `${verb}-test` })); - - // Act - const actual = await app.resolve(createEvent('/test', method), context); - - // Assess - expect(actual.statusCode).toBe(200); - expect(actual.body).toBe(JSON.stringify({ result: `${verb}-test` })); - expect(actual.headers?.['content-type']).toBe('application/json'); - expect(actual.isBase64Encoded).toBe(false); - } - ); - - it.each(httpMethods)( - 'routes %s requests with array response', - async (method, verb) => { - // Prepare - const app = new Router(); - ( - app[verb as Lowercase] as ( - path: string, - handler: RouteHandler - ) => void - )('/test', async () => [ + it.each( + httpMethods + )('routes %s requests with object response', async (method, verb) => { + // Prepare + const app = new Router(); + ( + app[verb as Lowercase] as ( + path: string, + handler: RouteHandler + ) => void + )('/test', async () => ({ result: `${verb}-test` })); + + // Act + const actual = await app.resolve(createEvent('/test', method), context); + + // Assess + expect(actual.statusCode).toBe(200); + expect(actual.body).toBe(JSON.stringify({ result: `${verb}-test` })); + expect(actual.headers?.['content-type']).toBe('application/json'); + expect(actual.isBase64Encoded).toBe(false); + }); + + it.each( + httpMethods + )('routes %s requests with array response', async (method, verb) => { + // Prepare + const app = new Router(); + ( + app[verb as Lowercase] as ( + path: string, + handler: RouteHandler + ) => void + )('/test', async () => [ + { id: 1, result: `${verb}-test-1` }, + { id: 2, result: `${verb}-test-2` }, + ]); + + // Act + const actual = await app.resolve(createEvent('/test', method), context); + + // Assess + expect(actual.statusCode).toBe(200); + expect(actual.body).toBe( + JSON.stringify([ { id: 1, result: `${verb}-test-1` }, { id: 2, result: `${verb}-test-2` }, - ]); - - // Act - const actual = await app.resolve(createEvent('/test', method), context); - - // Assess - expect(actual.statusCode).toBe(200); - expect(actual.body).toBe( - JSON.stringify([ - { id: 1, result: `${verb}-test-1` }, - { id: 2, result: `${verb}-test-2` }, - ]) - ); - expect(actual.headers?.['content-type']).toBe('application/json'); - expect(actual.isBase64Encoded).toBe(false); - } - ); + ]) + ); + expect(actual.headers?.['content-type']).toBe('application/json'); + expect(actual.isBase64Encoded).toBe(false); + }); - it.each([['CONNECT'], ['TRACE']])( - 'throws MethodNotAllowedError for %s requests', - async (method) => { - // Prepare - const app = new Router(); + it.each([ + ['CONNECT'], + ['TRACE'], + ])('throws MethodNotAllowedError for %s requests', async (method) => { + // Prepare + const app = new Router(); - // Act & Assess - const result = await app.resolve(createEvent('/test', method), context); + // Act & Assess + const result = await app.resolve(createEvent('/test', method), context); - expect(result.statusCode).toBe(HttpStatusCodes.METHOD_NOT_ALLOWED); - expect(result.body ?? '').toBe(''); - } - ); + expect(result.statusCode).toBe(HttpStatusCodes.METHOD_NOT_ALLOWED); + expect(result.body ?? '').toBe(''); + }); it('accepts multiple HTTP methods', async () => { // Act @@ -273,7 +271,10 @@ describe('Class: Router - V1 Multivalue Headers Support', () => { statusCode: 200, body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] }, + multiValueHeaders: { + 'cache-control': ['no-cache', 'no-store'], + 'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'], + }, })); // Act @@ -284,7 +285,10 @@ describe('Class: Router - V1 Multivalue Headers Support', () => { statusCode: 200, body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] }, + multiValueHeaders: { + 'cache-control': ['no-cache', 'no-store'], + 'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'], + }, isBase64Encoded: false, }); }); @@ -298,7 +302,7 @@ describe('Class: Router - V2 Cookies Support', () => { statusCode: 200, body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - cookies: ['session=abc123', 'theme=dark'], + cookies: ['session=abc123; Max-Age=3600', 'theme=dark'], })); // Act @@ -312,7 +316,7 @@ describe('Class: Router - V2 Cookies Support', () => { statusCode: 200, body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - cookies: ['session=abc123', 'theme=dark'], + cookies: ['session=abc123; Max-Age=3600', 'theme=dark'], isBase64Encoded: false, }); }); @@ -347,7 +351,10 @@ describe('Class: Router - ALB Support', () => { statusCode: 200, body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] }, + multiValueHeaders: { + 'cache-control': ['no-cache', 'no-store'], + 'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'], + }, })); // Act @@ -362,7 +369,10 @@ describe('Class: Router - ALB Support', () => { statusDescription: '200 OK', body: JSON.stringify({ message: 'success' }), headers: { 'content-type': 'application/json' }, - multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] }, + multiValueHeaders: { + 'cache-control': ['no-cache', 'no-store'], + 'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'], + }, isBase64Encoded: false, }); }); @@ -391,31 +401,31 @@ describe('Class: Router - ALB Support', () => { statusCode: Number(code), expectedDescription: `${code} ${text}`, })) - )( - 'returns statusDescription "$expectedDescription" for status code $statusCode', - async ({ statusCode, expectedDescription }) => { - // Prepare - const app = new Router(); - const noBodyStatuses = new Set([201, 204, 205, 304]); - const body = noBodyStatuses.has(statusCode) - ? null - : JSON.stringify({ status: statusCode }); - app.get('/test', () => new Response(body, { status: statusCode })); - - // Act - const result = await app.resolve( - createTestALBEvent('/test', 'GET'), - context - ); - - // Assess - expect(result.statusCode).toBe(statusCode); - expect(result).toHaveProperty('statusDescription', expectedDescription); - if (!noBodyStatuses.has(statusCode)) { - expect(result.body).toBe(JSON.stringify({ status: statusCode })); - } + )('returns statusDescription "$expectedDescription" for status code $statusCode', async ({ + statusCode, + expectedDescription, + }) => { + // Prepare + const app = new Router(); + const noBodyStatuses = new Set([201, 204, 205, 304]); + const body = noBodyStatuses.has(statusCode) + ? null + : JSON.stringify({ status: statusCode }); + app.get('/test', () => new Response(body, { status: statusCode })); + + // Act + const result = await app.resolve( + createTestALBEvent('/test', 'GET'), + context + ); + + // Assess + expect(result.statusCode).toBe(statusCode); + expect(result).toHaveProperty('statusDescription', expectedDescription); + if (!noBodyStatuses.has(statusCode)) { + expect(result.body).toBe(JSON.stringify({ status: statusCode })); } - ); + }); }); describe.each([ @@ -475,29 +485,31 @@ describe.each([ expect(result.isBase64Encoded).toBe(true); }); - it.each([['image/png'], ['image/jpeg'], ['audio/mpeg'], ['video/mp4']])( - 'sets isBase64Encoded for %s content-type', - async (contentType) => { - // Prepare - const app = new Router(); - app.get( - '/media', - () => - new Response('binary data', { - headers: { 'content-type': contentType }, - }) - ); - - // Act - const result = await app.resolve(createEvent('/media', 'GET'), context); - - // Assess - expect(result.statusCode).toBe(200); - expect(result.body).toBe(Buffer.from('binary data').toString('base64')); - expect(result.headers?.['content-type']).toBe(contentType); - expect(result.isBase64Encoded).toBe(true); - } - ); + it.each([ + ['image/png'], + ['image/jpeg'], + ['audio/mpeg'], + ['video/mp4'], + ])('sets isBase64Encoded for %s content-type', async (contentType) => { + // Prepare + const app = new Router(); + app.get( + '/media', + () => + new Response('binary data', { + headers: { 'content-type': contentType }, + }) + ); + + // Act + const result = await app.resolve(createEvent('/media', 'GET'), context); + + // Assess + expect(result.statusCode).toBe(200); + expect(result.body).toBe(Buffer.from('binary data').toString('base64')); + expect(result.headers?.['content-type']).toBe(contentType); + expect(result.isBase64Encoded).toBe(true); + }); it('does not set isBase64Encoded for text content-types', async () => { // Prepare From 1bbe3c1a023de95b62b0d8f5528d5092bf8c45f7 Mon Sep 17 00:00:00 2001 From: Nate Iler Date: Thu, 5 Feb 2026 10:17:04 -0700 Subject: [PATCH 3/3] style: removing comments within function --- packages/event-handler/src/http/converters.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/event-handler/src/http/converters.ts b/packages/event-handler/src/http/converters.ts index 08fd414660..d4eb52e23e 100644 --- a/packages/event-handler/src/http/converters.ts +++ b/packages/event-handler/src/http/converters.ts @@ -228,10 +228,7 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => { const headers: Record = {}; const multiValueHeaders: Record> = {}; - // Handle set-cookie headers specially using getSetCookie() const cookies = webHeaders.getSetCookie(); - // Some legacy implementations may concatenate multiple set-cookie values with commas - // Split them out while preserving the cookie attributes (which use semicolons) const allCookies: string[] = []; for (const cookie of cookies) { allCookies.push(...cookie.split(',').map((v) => v.trimStart())); @@ -244,7 +241,6 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => { } for (const [key, value] of webHeaders.entries()) { - // Skip set-cookie as it's already handled above if (key.toLowerCase() === 'set-cookie') { continue; }