Skip to content

setCookie [SSR]: Do not override existing cookies with new one.#183

Open
misterpekert wants to merge 4 commits intonuxt-community:masterfrom
misterpekert:master
Open

setCookie [SSR]: Do not override existing cookies with new one.#183
misterpekert wants to merge 4 commits intonuxt-community:masterfrom
misterpekert:master

Conversation

@misterpekert
Copy link
Copy Markdown

As per setHeader documentation:

If this header already exists in the to-be-sent headers, its value will be replaced

So, whenever $storage.setCookie() is called on SSR side, it replaces all cookies which were set before and it's impossible to set 2 different cookies using $storage.setCookie() on SSR, as only the last one will get into "Set-Cookies" header.

This PR aims to fix this issue.

@misterpekert misterpekert changed the title setCookie [SSR]: Do not override cookies which were previously set with new one. setCookie [SSR]: Do not override existing cookies with new one. May 25, 2021
@misterpekert
Copy link
Copy Markdown
Author

@pi0 any chance this will get reviewed and released? 🤔

@misterpekert
Copy link
Copy Markdown
Author

Any updates?

@misterpekert
Copy link
Copy Markdown
Author

misterpekert commented Aug 27, 2021

@farzadso is this project still being maintained? This PR is already 3m without any feedback.

@misterpekert
Copy link
Copy Markdown
Author

1 year passed, who cares, right? :)

const existingCookies = this.ctx.res.getHeader('Set-Cookie')
const cookies = Array.isArray(existingCookies)
? [...existingCookies, serializedCookie]
: [existingCookies, serializedCookie]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What happens if existingCookies is null?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@farzadso should be good now. Thanks for noticing!

@misterpekert misterpekert requested a review from farzadso August 30, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants