-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
maxAge should translate to the max-age directive #58
Comments
Can you please explain the following/show me how to reproduce this? The reason I ask is because the rationale doesn't really make sense to me, as the time zone is included in the
|
Ok, that was my confusion. I was trying to set But why don't you allow the |
Gotcha, good to hear there doesn't seem to be an issue with the date :) I mean, this was an adopted module, so I cannot speak for the history (and of course changing it would be a breaking change and would need to be evaluated first). AFAIK it uses Here's also a blog about the two if it helps: http://mrcoles.com/blog/cookies-max-age-vs-expires/ |
the
Sadly incorrect UA clock is a common problem (at least for us), maxAge is a correct option (as the time is calculated relative to UA clock), but this is not an available option in this library. Hope this makes sense. |
Incorrect UA clock is something we also observe quite often, and the root cause if often broken clock sync, not incorrect timezone. From the link pasted:
..indicating that just setting both shouldn't pose a problem. And It's only IE <=8 that doesn't support max-age. I find it very strange that nobody has brought this up before in a module heavily in use because of eg. Koa. Edit: Saw the discussion in #94 now, sorry for adding to the confusion. I still think it's a more sane default to send both. |
@ruudud I'm happy to accept a PR that just sets both. The other PR was closed without making that change, so you're most welcome to make one! I can make the change as well, but just wanted to put that out there if you wanted to get credit for the work :D |
Did a bit of digging now.. Doesn't seem like Google use the I'm guessing the reason is that they just run a check client-side, comparing with their server clock, and if it's off by too much, warn the user or whatever. Seems like an OK solution if you ask me.. |
I've made a new PR for this (#107) as I need the ability to be able to use I'm sending both values so this shouldn't cause a problem with any older browsers, I can't see any downside to this. What do we think? |
the
maxAge
option is misleading. It should translate to themax-age
cookie directive, notexpires
withDate.now()
base.Date.now
causes issues whenmaxAge
is low (a few hours), since it uses the server date, and the client date may be different due to timezones.The text was updated successfully, but these errors were encountered: