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

Use @fastify/cookie for cookie handling #243

Closed
2 tasks done
gurgunday opened this issue Apr 15, 2024 · 4 comments
Closed
2 tasks done

Use @fastify/cookie for cookie handling #243

gurgunday opened this issue Apr 15, 2024 · 4 comments
Labels
good first issue Good for newcomers

Comments

@gurgunday
Copy link
Member

Prerequisites

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

🚀 Feature Proposal

We should move from

session/lib/cookie.js

Lines 5 to 67 in 25b4f6b

module.exports = class Cookie {
constructor (cookie, request) {
const originalMaxAge = cookie.originalMaxAge || cookie.maxAge || null
this.path = cookie.path || '/'
this.secure = cookie.secure ?? null
this.sameSite = cookie.sameSite || null
this.domain = cookie.domain || null
this.httpOnly = cookie.httpOnly !== undefined ? cookie.httpOnly : true
this._expires = null
if (originalMaxAge) {
this.maxAge = originalMaxAge
} else if (cookie.expires) {
this.expires = new Date(cookie.expires)
this.originalMaxAge = null
} else {
this.originalMaxAge = originalMaxAge
}
if (this.secure === 'auto') {
if (isConnectionSecure(request)) {
this.secure = true
} else {
this.sameSite = 'Lax'
this.secure = false
}
}
}
set expires (date) {
this._expires = date
}
get expires () {
return this._expires
}
set maxAge (ms) {
this.expires = new Date(Date.now() + ms)
// we force the same originalMaxAge to match old behavior
this.originalMaxAge = ms
}
get maxAge () {
if (this.expires instanceof Date) {
return this.expires.valueOf() - Date.now()
} else {
return null
}
}
toJSON () {
return {
expires: this._expires,
originalMaxAge: this.originalMaxAge,
sameSite: this.sameSite,
secure: this.secure,
path: this.path,
httpOnly: this.httpOnly,
domain: this.domain
}
}
}
to https://github.com/fastify/fastify-cookie/blob/master/cookie.js

It requires moderate refactoring

Motivation

No response

Example

No response

@gurgunday gurgunday added the good first issue Good for newcomers label Apr 15, 2024
@gurgunday gurgunday changed the title Use @fastify/cookie implementation Use @fastify/cookie for cookie handling Apr 15, 2024
@karankraina
Copy link

Taking this up

@karankraina
Copy link

Hi @gurgunday The link to cookie.js file in the issue description is broken. Can you please guide what exactly we need to do here.

@jsumners
Copy link
Member

From the issue description: this module is using an embedded ("vendored") script to parse and generate cookies. Instead, it should use the @fastify/cookie module.

@gurgunday
Copy link
Member Author

gurgunday commented Oct 14, 2024

@jsumners is right, that was my intention but I took a look again and it seems mostly like a class/interface to manipulate cookie options before passing it to fastify-cookie

So it seems fine to me now

Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants