sessions icon indicating copy to clipboard operation
sessions copied to clipboard

[BUG] FilesystemStore MaxAge does not allow cookies until browser closes

Open HTechHQ opened this issue 2 years ago • 3 comments

Is there an existing issue for this?

  • [X] 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.

HTechHQ avatar Aug 04 '23 01:08 HTechHQ

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.

maragunde93 avatar Oct 04 '23 21:10 maragunde93

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?

mengstr avatar Feb 07 '24 13:02 mengstr

Hey folks, feel free to open a PR with a failing test and we can prioritise a fix for this.

jaitaiwan avatar Feb 07 '24 22:02 jaitaiwan

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

jaitaiwan avatar Jun 15 '24 01:06 jaitaiwan

As to the error that's appearing, the if statement is supposed to cover that: https://github.com/gorilla/sessions/blob/bdabf0ac29ab2354c0674cb0dcd3a4f31f53589d/store.go#L219

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!

jaitaiwan avatar Jun 15 '24 02:06 jaitaiwan