fix(event-handler): handle set-cookie header values with multiple attributes #4990

nateiler · 2026-02-04T22:00:33Z

Summary

This fixes the scenario where a single set-cookie header value foo=bar; Max-Age=3600 is treated as two set-cookie values because multiple values may be separated by a ;.

Changes

When parsing the headers, set-cookie headers are handled using the getSetCookie method.

Logic to detect multiple header entries has been removed as multiple header values are appended to a single header (via comma).

Issue number: closes #4986

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

boring-cyborg · 2026-02-04T22:00:42Z

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #typescript channel on our Powertools for AWS Lambda Discord: Invite link

dreamorosi

Thank you for the PR @nateiler - appreciate it.

I have left a couple comments on the diff, would like to also get @svozza's review on the PR before we merge/

dreamorosi · 2026-02-05T08:53:54Z

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

+  // 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()));
+  }
+
+  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()) {
+    // Skip set-cookie as it's already handled above


Let's remove these comments please

dreamorosi · 2026-02-05T08:55:21Z

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

-    if (headers[key]) {
-      multiValueHeaders[key] = [headers[key], ...values];
-      delete headers[key];
-    } else if (values.length > 1) {
+    if (values.length > 1) {


Why are we removing this logic?

To my knowledge set-cookie is the only header that can have multiple keys. Other headers, when appended to, will append the value to the end via a comma. This logic would never be evaluated (which was verified through the code coverage)

dreamorosi · 2026-02-05T08:55:48Z

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

There's a lot of diffs that seem to be unrelated to this PR - any chance you could revert?

Ah, yeah. Looks like my own biome got the best of me.

Actually, it looks like these changes were automatically made via lint-staged commit hook. After reverting them and attempting to commit, my revert gets stashed and I'm left with an empty commit which is prevented.

svozza · 2026-02-05T10:13:55Z

packages/event-handler/tests/unit/http/converters.test.ts

      // Prepare
      const headers = new Headers({
-        'set-cookie': 'session=abc123; theme=dark',
+        'cache-control': 'no-cache; no-store',


How come we're changing these?

set-cookie is a header that shouldn't be separated by a semicolon. Keeping it as it were would result in a single set-cookie header with an unsupported 'theme' attribute which would cause the test to fail and ultimately seems to defeat the purpose of the test. cache-control represents a normal header that can be separated by a semicolon. Other tests cover the nuances of 'set-cookie'.

svozza · 2026-02-05T10:16:53Z

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

      headers: { 'content-type': 'application/json' },
-      multiValueHeaders: { 'set-cookie': ['session=abc123', 'theme=dark'] },
+      multiValueHeaders: {
+        'cache-control': ['no-cache', 'no-store'],


Where are these cache-control headers coming from?

I added them to illustrate a 'simple' multi-value header pair along with the set-cookie multi-value header which contains a semicolon.

nateiler · 2026-02-05T17:12:18Z

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

+      continue;
+    }
+
    const values = value.split(/[;,]/).map((v) => v.trimStart());


Ultimately, splitting on a semicolon seems out of place and is the root of this entire issue. If a goal wasn't to keep backwards compatibility I would suggest removing it and using a comma only. Headers by default a separated by a comma.

const headers = new Headers({ 'accept': 'application/json' }) headers.append('accept', 'text/html'); const acceptHeaders = headers.get('accept'); // Results in "application/json, text/html"

So the reason I wrote it this way was because I was under the impression that cookies were an exception in that they could be separated by a semicolon. I got this from the RFC 6265:

The user agent sends stored cookies to the origin server in the
Cookie header. If the server conforms to the requirements in
Section 4.1 (and the user agent conforms to the requirements in
Section 5), the user agent will send a Cookie header that conforms to
the following grammar:

cookie-header = "Cookie:" OWS cookie-string OWS
cookie-string = cookie-pair *( ";" SP cookie-pair )

It looks like I made a mistake and thought this referred to the Set-Cookie header rather than the Cookie header as indicated in the quote above.

What this means though is that to handle the Cookie header case correctly we do need to have special handling for the semicolon. Btw, regarding this:

If a goal wasn't to keep backwards compatibility

We don't need to preserve backward compatibility for what was a bug. If we change this so only Cookie gets this handling and that's fine.

Thanks for the references to the RFC. In this particular circumstance the webHeadersToApiGatewayV1Headers function is converting from server to user agent (not also user agent to server) so we should be able to omit special handling of the Cookie header as per RFC6265 4.2 the Cookie header is a user agent to server header.

It seems like the semicolon was included to handle the special usage of the 'Cookie' header which we don't actually need here.

We don't need to preserve backward compatibility for what was a bug. If we change this so only Cookie gets this handling and that's fine.

I see your point here; however I think only set-cookie with a semicolon is a bug. Since the current implementation accepts any header with a semicolon separated value (while it may not be popular) removing it might break current implementations.

I'm unsure of the best path here. Ultimately it seems like dropping the semicolon would simplify and unify the usage across the board (all headers are treated the same).

The issue here is larger though, splitting on ; causes worse problems. The accepted way to inidicate parameters in a header is with a semi colon, e.g., Accept: text/html; q=0.5, text/plain; q=0.2. With the current implementation, this ends up with a multivalue header value which is clearly wrong:

// ... multiValueHeaders: { accept: [ "text/html", "q=0.5", "text/plain", "q=0.2", ] } // ...

It's far more important that we handle this case correctly than headers that aren't Cookie erroneously delimited by ;.

sonarqubecloud · 2026-02-05T17:32:07Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
4.1% Duplication on New Code

See analysis details on SonarQube Cloud

@svozza

My comments were addressed, will leave to @svozza to bring the PR to merge state.

nateiler added 2 commits February 4, 2026 14:06

fix: support multiple attributes in set-cookie headers

969f0e1

fix: multiple header values are comma-separated except set-cookie

f0fe4ef

pull-request-size bot added the size/L PRs between 100-499 LOC label Feb 4, 2026

boring-cyborg bot added event-handler This item relates to the Event Handler Utility tests PRs that add or change tests labels Feb 4, 2026

dreamorosi assigned nateiler Feb 5, 2026

dreamorosi previously requested changes Feb 5, 2026

View reviewed changes

dreamorosi requested a review from svozza February 5, 2026 08:56

svozza reviewed Feb 5, 2026

View reviewed changes

nateiler commented Feb 5, 2026

View reviewed changes

style: removing comments within function

1bbe3c1

fix(event-handler): handle set-cookie header values with multiple attributes #4990

Are you sure you want to change the base?

fix(event-handler): handle set-cookie header values with multiple attributes #4990

Conversation

nateiler commented Feb 4, 2026

Summary

Changes

Uh oh!

boring-cyborg bot commented Feb 4, 2026

Uh oh!

dreamorosi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svozza Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svozza Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Feb 5, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

svozza Feb 5, 2026 •

edited

Loading

svozza Feb 5, 2026 •

edited

Loading