Skip to content

Conversation

@johankrijt
Copy link
Contributor

Use H3's splitCookiesString function to split the set-cookie header without choking on commas that are within a single set-cookie field-value, such as in the Expires portion.

This splits them without choking on commas that are within a single set-cookie field-value, such as in the Expires portion.
@johankrijt johankrijt requested a review from danielroe as a code owner July 19, 2024 12:50
@bolt-new-by-stackblitz
Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.


```ts [composables/fetch.ts]
import { appendResponseHeader, H3Event } from 'h3'
import { appendResponseHeader, H3Event, splitCookiesString } from 'h3'
Copy link
Member

@pi0 pi0 Jul 19, 2024

Choose a reason for hiding this comment

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

Please use splitSetCookieString from cookie-es directly

const res = await $fetch.raw(url)
/* Get the cookies from the response */
const cookies = (res.headers.get('set-cookie') || '').split(',')
const cookies = splitCookiesString(res.headers.get('set-cookie') || '')
Copy link
Member

Choose a reason for hiding this comment

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

We can instead also use native headers.getSetCookie that returns properly splitter cookies (please double check if works within current nitro v1 env)

Copy link
Member

Choose a reason for hiding this comment

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

works well!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even nicer! Sorry I was to late to the party to safe you the work, as I see it's already fixed.

@danielroe danielroe merged commit 228b9f6 into nuxt:main Jul 19, 2024
@github-actions github-actions bot mentioned this pull request Jul 19, 2024
@danielroe
Copy link
Member

Sorry, didn't spot your comments @pi0!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants