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

clearCookie does not include maxAge, neither allows to pass it as options #303

Closed
2 tasks done
aristofun opened this issue Oct 6, 2024 · 4 comments · Fixed by #304
Closed
2 tasks done

clearCookie does not include maxAge, neither allows to pass it as options #303

aristofun opened this issue Oct 6, 2024 · 4 comments · Fixed by #304

Comments

@aristofun
Copy link
Contributor

aristofun commented Oct 6, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

5.0

Plugin version

v10.0.1

Node.js version

any

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

any

Description

These are the problematic lines:

fastify-cookie/plugin.js

Lines 42 to 46 in ccb9053

const opts = Object.assign({ path: '/' }, options, {
expires: new Date(1),
signed: undefined,
maxAge: undefined
})

Without maxAge: 0 at least some of cookie jar implementations (for example https://www.npmjs.com/package/tough-cookie) keep actual cookie with empty value in the storage instead of deleting them (like they do when maxAge is set to zero).

Because of this flaw some users had to come up with workarounds like this.

Link to code that reproduces the bug

No response

Expected Behavior

maxAge should be set along with expires option

@jsumners
Copy link
Member

jsumners commented Oct 7, 2024

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@gurgunday
Copy link
Member

Latest RFC says:

Finally, to remove a cookie, the server returns a Set-Cookie header
with an expiration date in the past. The server will be successful
in removing the cookie only if the Path and the Domain attribute in
the Set-Cookie header match the values used when the cookie was
created.

We return new Date(1), which is Thu Jan 01 1970 01:00:00

Our implementation is correct

@aristofun
Copy link
Contributor Author

aristofun commented Oct 9, 2024

Our implementation is correct

Adding maxAge would not make it less correct, would it?

Yet it would improve devX for many users.

@aristofun
Copy link
Contributor Author

#304

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 a pull request may close this issue.

3 participants