Skip to content

fix: copy a set-cookie array passed to reply.header instead of storing it - #7068

Merged
mcollina merged 1 commit into
fastify:mainfrom
fadiroot:fix/set-cookie-array-copy
Oct 8, 2026
Merged

mcollina merged 1 commit into
fastify:mainfrom
fadiroot:fix/set-cookie-array-copy

Conversation

@fadiroot

Copy link
Copy Markdown
Contributor

reply.header('set-cookie', value) stores value in the reply's header map. When value is an array, the array object itself is stored, and every subsequent set-cookie call on the reply pushes into it (lib/reply.js, the Array.prototype.push.apply / .push branch). The string case already copies into a fresh array before pushing, so only the array case leaks the caller's object.

If the array is shared, for example a module-level default, it grows by one entry on every request, and responses carry more and more duplicate cookies:

const defaults = ['a=1']
fastify.get('/', (req, reply) => reply.header('set-cookie', defaults).header('set-cookie', 'b=2').send({}))
// 1st response: a=1, b=2   2nd: a=1, b=2, b=2   3rd: a=1, b=2, b=2, b=2 ...

reply.headers({ 'set-cookie': defaults }) goes through the same path.

This PR stores a copy of the array, so later pushes stay local to the reply.

Checklist

  • run npm run test and npm run benchmark — test/internals/reply.test.js 86/86 with the change; the new test reply.header does not mutate a set-cookie array passed by the caller fails on main (the second response carries b=2 twice and the caller's array has grown) and passes with this change. eslint is clean.
  • tests and/or benchmarks are included
  • documentation is changed or added — n/a, behaviour now matches the documented one
  • commit message follows commit guidelines

…g it

reply.header('set-cookie', value) stored the caller's array as is, and
every later set-cookie call pushed into that same array. When the array
was a shared value, such as a module-level default, it grew by one entry
on each request, and responses carried more and more duplicate cookies.
The string branch already copies into a fresh array before pushing.

Store a copy of the array so later pushes stay local to the reply.

Signed-off-by: Fadi Romdhan <[email protected]>
@Tony133
Tony133 requested a review from a team September 30, 2026 16:04

@gurgunday gurgunday left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

yeah this does seem like a problem to me

@gurgunday
gurgunday requested review from a team and Eomm October 3, 2026 11:41
@mcollina
mcollina merged commit 5c081d4 into fastify:main Oct 8, 2026
28 checks passed
@mcollina mcollina added the backport 5.x Issue or pr that should be backported to Fastify v5 label Oct 8, 2026
Tony133 added a commit that referenced this pull request Oct 12, 2026
…stead of storing it (#7087)

Backport 5c081d4 from #7068.

Signed-off-by: Fadi Romdhan <[email protected]>
Co-authored-by: fadi romdhan <[email protected]>
Co-authored-by: Fadi Romdhan <[email protected]>
Co-authored-by: Antonio Tripodi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 5.x Issue or pr that should be backported to Fastify v5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants