Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: secure attribute cannot be set when deleting a cookie #65649

Closed
wants to merge 3 commits into from

Conversation

nonoakij
Copy link
Contributor

@nonoakij nonoakij commented May 11, 2024

fixes #56632

To delete a cookie with the prefix __Secure-, it must be secure.

Previously, the value of secure was not set correctly.
(In typescript, d.ts is options: Omit<ResponseCookie, 'value' | 'expires'>, so it is not an error but not actually set).
This PR solves this problem.

cookies().delete({ name: "Secure-cookie", secure: true })
screenshot

reproduce repository

https://github.com/nonoakij/reproduce-cookie-delete

@ijjk
Copy link
Member

ijjk commented May 11, 2024

Allow CI Workflow Run

  • approve CI run for commit: 190f373

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

@nonoakij nonoakij changed the title fix: cookie delete args fix: secure directive cannot be set when deleting a cookie May 12, 2024
@nonoakij nonoakij changed the title fix: secure directive cannot be set when deleting a cookie fix: secure attribute cannot be set when deleting a cookie May 12, 2024
@nonoakij nonoakij changed the title fix: secure attribute cannot be set when deleting a cookie Fix: secure attribute cannot be set when deleting a cookie May 12, 2024
Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Looks like this needs to be updated upstream as this is just the vendored version of https://github.com/vercel/edge-runtime/tree/main/packages/cookies

@nonoakij nonoakij closed this May 14, 2024
@nonoakij nonoakij deleted the fix-cookie-delete-args branch May 14, 2024 01:26
@nonoakij
Copy link
Contributor Author

@ijjk
Thank you for your review.
I did not fully understand the structure.
I will attempt to make a fix to vercel/edge-runtime/packages/cookies/src/response-cookies.ts.

@nonoakij
Copy link
Contributor Author

@ijjk I have created a pull request, could you please check it?
vercel/edge-runtime#890

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.

ResponseCookies#delete does not work with __Secure-/__Host- cookie prefixes.
2 participants