[BUG] FilesystemStore MaxAge==0 does set cookies that expire when browser closes
HTechHQ opened this issue · 5 comments
Is there an existing issue for this?
- I have searched the existing issues
Current Behavior
Using the file store sessions.NewFilesystemStore("", []byte("secret")) and
saving a new session with the Options.MaxAge of 0 will lead to
error: remove /tmp/session_36KLXHXBPO7WQCZCDAULFDR73W7ON6445Y2YCWYZMY26QPCYJQSA: no such file or directory
Expected Behavior
The session should be working as 0 as a value indicates to the browser to delete the cookie ones the browser is closed.
Steps To Reproduce
No response
Anything else?
The issue has been discussed in #96 and #103
The main objection seems to have been the breaking of depending projects.
As the current behaviour of the FilesystemStore is wildly different from the expected web standards, I think it is worth the step. Especially with the new revamp of the gorillas project, a new version could make this a a viable option for everyone.
Is there any update on this or a work around? We need to have 'Session' cookies too so it gets deleted when the client closes the browser.
Yes please. Working session (temporary) cookies are sorely missed. If patching the obvious(?) bug would make existing usages break, then just add some other functions to handle session cookies.
Having to fork this project to change if session.Options.MaxAge <= 0 { into if session.Options.MaxAge < 0 { feels so silly. Doesn't most prjoject nowadays need session cookies to ease the issues with GDPR and et.al?
Hey folks, feel free to open a PR with a failing test and we can prioritise a fix for this.
As far as I can tell folks, I looked at the http spec and this seems compliant:
If delta-seconds is less than or equal to zero (0), let expiry-time
be the earliest representable date and time.
From: https://datatracker.ietf.org/doc/html/rfc6265#section-5.2.2
What's the behaviour that you're wanting to see happen? So far as I can tell, the only way to ensure the session cookie stays around in the browser is to ensure that the max-age is set to a positive integer.
I replied about the specific logic in #271
As to the error that's appearing, the if statement is supposed to cover that:
Line 219 in bdabf0a
The !os.IsNotExists(err) should ensure that there's no error returned when the session doesn't exist.
If you have a different perspective feel free to speak out and I'll do my best to work with you. Thanks!