Cookie Expiration Age Fix #7327
Conversation
| @@ -326,6 +335,7 @@ export const getCookieOptions = (req: express.Request): express.CookieOptions => | |||
| domain: getCookieDomain(url.host, req.args["proxy-domain"]), | |||
| path: normalize(url.pathname) || "/", | |||
| sameSite: "lax", | |||
| maxAge: getConfigCookieMaxAgeAsMilliseconds(req) || 0, | |||
There was a problem hiding this comment.
Oh wait is 0 the right default? This will delete the cookie instantly, I think? We may want -1.
There was a problem hiding this comment.
Hmm actually zero or negative immediately expires the cookie it seems.
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie#max-agenumber
https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2
Should it just be undefined? Also we may want ?? instead of || in case someone does explicitly set it to zero for some reason.
There was a problem hiding this comment.
I'd read it in the Express JS docs setting it to 0 makes it session
https://expressjs.com/en/api.html#res.cookie
They gave this explanation to the expires parameter, but also said maxAge is a "Convenient option for setting the expiry time relative to the current time in milliseconds". They didn't specify what to put here to make it session, nor what the default is.
There was a problem hiding this comment.
Yeah I also thought maybe it was an Express thing, but when I checked the source it seems to just immediately expire.
If maxAge is zero, it passes expires set to now and maxAge set to zero into cookie.serialize():
cookie.serialize() simply adds Max-Age=0 and Expires=<the now date> to the cookie:
Now, if expires is zero (although this is actually a type error), and there is no max age it does indeed skip setting Expires in that case, which would result in a session cookie. The difference is they do if (options.maxAge !== undefined) versus if (options.expires), so zero is not ignored in the max age case.
Fixes #7301