Skip to content

Commit 42317fe

Browse files
nateilersvozza
nateiler
and
svozza
authored
fix(event-handler): handle set-cookie header values with multiple attributes (#4990)
Co-authored-by: Stefano Vozza <svozza@amazon.com>
1 parent 8e4da8a commit 42317fe

3 files changed

Lines changed: 209 additions & 143 deletions

File tree

‎packages/event-handler/src/http/converters.ts‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,26 @@ const webHeadersToApiGatewayV1Headers = (webHeaders: Headers) => {
228228
const headers: Record<string, string> = {};
229229
const multiValueHeaders: Record<string, Array<string>> = {};
230230

231+
const cookies = webHeaders.getSetCookie();
232+
const allCookies: string[] = [];
233+
for (const cookie of cookies) {
234+
allCookies.push(...cookie.split(',').map((v) => v.trimStart()));
235+
}
236+
237+
if (allCookies.length > 1) {
238+
multiValueHeaders['set-cookie'] = allCookies;
239+
} else if (allCookies.length === 1) {
240+
headers['set-cookie'] = allCookies[0];
241+
}
242+
231243
for (const [key, value] of webHeaders.entries()) {
232-
const values = value.split(/[;,]/).map((v) => v.trimStart());
244+
if (key.toLowerCase() === 'set-cookie') {
245+
continue;
246+
}
247+
248+
const values = value.split(',').map((v) => v.trimStart());
233249

234-
if (headers[key]) {
235-
multiValueHeaders[key] = [headers[key], ...values];
236-
delete headers[key];
237-
} else if (values.length > 1) {
250+
if (values.length > 1) {
238251
multiValueHeaders[key] = values;
239252
} else {
240253
headers[key] = value;

‎packages/event-handler/tests/unit/http/Router/basic-routing.test.ts‎

Lines changed: 127 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -29,73 +29,71 @@ describe.each([
2929
['HEAD', 'head'],
3030
['OPTIONS', 'options'],
3131
];
32-
it.each(httpMethods)(
33-
'routes %s requests with object response',
34-
async (method, verb) => {
35-
// Prepare
36-
const app = new Router();
37-
(
38-
app[verb as Lowercase<HttpMethod>] as (
39-
path: string,
40-
handler: RouteHandler
41-
) => void
42-
)('/test', async () => ({ result: `${verb}-test` }));
43-
44-
// Act
45-
const actual = await app.resolve(createEvent('/test', method), context);
46-
47-
// Assess
48-
expect(actual.statusCode).toBe(200);
49-
expect(actual.body).toBe(JSON.stringify({ result: `${verb}-test` }));
50-
expect(actual.headers?.['content-type']).toBe('application/json');
51-
expect(actual.isBase64Encoded).toBe(false);
52-
}
53-
);
54-
55-
it.each(httpMethods)(
56-
'routes %s requests with array response',
57-
async (method, verb) => {
58-
// Prepare
59-
const app = new Router();
60-
(
61-
app[verb as Lowercase<HttpMethod>] as (
62-
path: string,
63-
handler: RouteHandler
64-
) => void
65-
)('/test', async () => [
32+
it.each(
33+
httpMethods
34+
)('routes %s requests with object response', async (method, verb) => {
35+
// Prepare
36+
const app = new Router();
37+
(
38+
app[verb as Lowercase<HttpMethod>] as (
39+
path: string,
40+
handler: RouteHandler
41+
) => void
42+
)('/test', async () => ({ result: `${verb}-test` }));
43+
44+
// Act
45+
const actual = await app.resolve(createEvent('/test', method), context);
46+
47+
// Assess
48+
expect(actual.statusCode).toBe(200);
49+
expect(actual.body).toBe(JSON.stringify({ result: `${verb}-test` }));
50+
expect(actual.headers?.['content-type']).toBe('application/json');
51+
expect(actual.isBase64Encoded).toBe(false);
52+
});
53+
54+
it.each(
55+
httpMethods
56+
)('routes %s requests with array response', async (method, verb) => {
57+
// Prepare
58+
const app = new Router();
59+
(
60+
app[verb as Lowercase<HttpMethod>] as (
61+
path: string,
62+
handler: RouteHandler
63+
) => void
64+
)('/test', async () => [
65+
{ id: 1, result: `${verb}-test-1` },
66+
{ id: 2, result: `${verb}-test-2` },
67+
]);
68+
69+
// Act
70+
const actual = await app.resolve(createEvent('/test', method), context);
71+
72+
// Assess
73+
expect(actual.statusCode).toBe(200);
74+
expect(actual.body).toBe(
75+
JSON.stringify([
6676
{ id: 1, result: `${verb}-test-1` },
6777
{ id: 2, result: `${verb}-test-2` },
68-
]);
69-
70-
// Act
71-
const actual = await app.resolve(createEvent('/test', method), context);
72-
73-
// Assess
74-
expect(actual.statusCode).toBe(200);
75-
expect(actual.body).toBe(
76-
JSON.stringify([
77-
{ id: 1, result: `${verb}-test-1` },
78-
{ id: 2, result: `${verb}-test-2` },
79-
])
80-
);
81-
expect(actual.headers?.['content-type']).toBe('application/json');
82-
expect(actual.isBase64Encoded).toBe(false);
83-
}
84-
);
78+
])
79+
);
80+
expect(actual.headers?.['content-type']).toBe('application/json');
81+
expect(actual.isBase64Encoded).toBe(false);
82+
});
8583

86-
it.each([['CONNECT'], ['TRACE']])(
87-
'throws MethodNotAllowedError for %s requests',
88-
async (method) => {
89-
// Prepare
90-
const app = new Router();
84+
it.each([
85+
['CONNECT'],
86+
['TRACE'],
87+
])('throws MethodNotAllowedError for %s requests', async (method) => {
88+
// Prepare
89+
const app = new Router();
9190

92-
// Act & Assess
93-
const result = await app.resolve(createEvent('/test', method), context);
91+
// Act & Assess
92+
const result = await app.resolve(createEvent('/test', method), context);
9493

95-
expect(result.statusCode).toBe(HttpStatusCodes.METHOD_NOT_ALLOWED);
96-
expect(result.body ?? '').toBe('');
97-
}
98-
);
94+
expect(result.statusCode).toBe(HttpStatusCodes.METHOD_NOT_ALLOWED);
95+
expect(result.body ?? '').toBe('');
96+
});
9997

10098
it('accepts multiple HTTP methods', async () => {
10199
// Act
@@ -273,7 +271,10 @@ describe('Class: Router - V1 Multivalue Headers Support', () => {
273271
statusCode: 200,
274272
body: JSON.stringify({ message: 'success' }),
275273
headers: { 'content-type': 'application/json' },
276-
multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] },
274+
multiValueHeaders: {
275+
'cache-control': ['no-cache', 'no-store'],
276+
'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'],
277+
},
277278
}));
278279

279280
// Act
@@ -284,7 +285,10 @@ describe('Class: Router - V1 Multivalue Headers Support', () => {
284285
statusCode: 200,
285286
body: JSON.stringify({ message: 'success' }),
286287
headers: { 'content-type': 'application/json' },
287-
multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] },
288+
multiValueHeaders: {
289+
'cache-control': ['no-cache', 'no-store'],
290+
'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'],
291+
},
288292
isBase64Encoded: false,
289293
});
290294
});
@@ -298,7 +302,7 @@ describe('Class: Router - V2 Cookies Support', () => {
298302
statusCode: 200,
299303
body: JSON.stringify({ message: 'success' }),
300304
headers: { 'content-type': 'application/json' },
301-
cookies: ['session=abc123', 'theme=dark'],
305+
cookies: ['session=abc123; Max-Age=3600', 'theme=dark'],
302306
}));
303307

304308
// Act
@@ -312,7 +316,7 @@ describe('Class: Router - V2 Cookies Support', () => {
312316
statusCode: 200,
313317
body: JSON.stringify({ message: 'success' }),
314318
headers: { 'content-type': 'application/json' },
315-
cookies: ['session=abc123', 'theme=dark'],
319+
cookies: ['session=abc123; Max-Age=3600', 'theme=dark'],
316320
isBase64Encoded: false,
317321
});
318322
});
@@ -347,7 +351,10 @@ describe('Class: Router - ALB Support', () => {
347351
statusCode: 200,
348352
body: JSON.stringify({ message: 'success' }),
349353
headers: { 'content-type': 'application/json' },
350-
multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] },
354+
multiValueHeaders: {
355+
'cache-control': ['no-cache', 'no-store'],
356+
'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'],
357+
},
351358
}));
352359

353360
// Act
@@ -362,7 +369,10 @@ describe('Class: Router - ALB Support', () => {
362369
statusDescription: '200 OK',
363370
body: JSON.stringify({ message: 'success' }),
364371
headers: { 'content-type': 'application/json' },
365-
multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] },
372+
multiValueHeaders: {
373+
'cache-control': ['no-cache', 'no-store'],
374+
'set-cookie': ['session=abc123; Max-Age=3600', 'theme=dark'],
375+
},
366376
isBase64Encoded: false,
367377
});
368378
});
@@ -391,31 +401,31 @@ describe('Class: Router - ALB Support', () => {
391401
statusCode: Number(code),
392402
expectedDescription: `${code} ${text}`,
393403
}))
394-
)(
395-
'returns statusDescription "$expectedDescription" for status code $statusCode',
396-
async ({ statusCode, expectedDescription }) => {
397-
// Prepare
398-
const app = new Router();
399-
const noBodyStatuses = new Set([201, 204, 205, 304]);
400-
const body = noBodyStatuses.has(statusCode)
401-
? null
402-
: JSON.stringify({ status: statusCode });
403-
app.get('/test', () => new Response(body, { status: statusCode }));
404-
405-
// Act
406-
const result = await app.resolve(
407-
createTestALBEvent('/test', 'GET'),
408-
context
409-
);
410-
411-
// Assess
412-
expect(result.statusCode).toBe(statusCode);
413-
expect(result).toHaveProperty('statusDescription', expectedDescription);
414-
if (!noBodyStatuses.has(statusCode)) {
415-
expect(result.body).toBe(JSON.stringify({ status: statusCode }));
416-
}
404+
)('returns statusDescription "$expectedDescription" for status code $statusCode', async ({
405+
statusCode,
406+
expectedDescription,
407+
}) => {
408+
// Prepare
409+
const app = new Router();
410+
const noBodyStatuses = new Set([201, 204, 205, 304]);
411+
const body = noBodyStatuses.has(statusCode)
412+
? null
413+
: JSON.stringify({ status: statusCode });
414+
app.get('/test', () => new Response(body, { status: statusCode }));
415+
416+
// Act
417+
const result = await app.resolve(
418+
createTestALBEvent('/test', 'GET'),
419+
context
420+
);
421+
422+
// Assess
423+
expect(result.statusCode).toBe(statusCode);
424+
expect(result).toHaveProperty('statusDescription', expectedDescription);
425+
if (!noBodyStatuses.has(statusCode)) {
426+
expect(result.body).toBe(JSON.stringify({ status: statusCode }));
417427
}
418-
);
428+
});
419429
});
420430

421431
describe.each([
@@ -475,29 +485,31 @@ describe.each([
475485
expect(result.isBase64Encoded).toBe(true);
476486
});
477487

478-
it.each([['image/png'], ['image/jpeg'], ['audio/mpeg'], ['video/mp4']])(
479-
'sets isBase64Encoded for %s content-type',
480-
async (contentType) => {
481-
// Prepare
482-
const app = new Router();
483-
app.get(
484-
'/media',
485-
() =>
486-
new Response('binary data', {
487-
headers: { 'content-type': contentType },
488-
})
489-
);
490-
491-
// Act
492-
const result = await app.resolve(createEvent('/media', 'GET'), context);
493-
494-
// Assess
495-
expect(result.statusCode).toBe(200);
496-
expect(result.body).toBe(Buffer.from('binary data').toString('base64'));
497-
expect(result.headers?.['content-type']).toBe(contentType);
498-
expect(result.isBase64Encoded).toBe(true);
499-
}
500-
);
488+
it.each([
489+
['image/png'],
490+
['image/jpeg'],
491+
['audio/mpeg'],
492+
['video/mp4'],
493+
])('sets isBase64Encoded for %s content-type', async (contentType) => {
494+
// Prepare
495+
const app = new Router();
496+
app.get(
497+
'/media',
498+
() =>
499+
new Response('binary data', {
500+
headers: { 'content-type': contentType },
501+
})
502+
);
503+
504+
// Act
505+
const result = await app.resolve(createEvent('/media', 'GET'), context);
506+
507+
// Assess
508+
expect(result.statusCode).toBe(200);
509+
expect(result.body).toBe(Buffer.from('binary data').toString('base64'));
510+
expect(result.headers?.['content-type']).toBe(contentType);
511+
expect(result.isBase64Encoded).toBe(true);
512+
});
501513

502514
it('does not set isBase64Encoded for text content-types', async () => {
503515
// Prepare

0 commit comments

Comments
 (0)