Bad Save() implementation

[janekolszak]

Hi,
Great package, thank you soo much for providing this!

I see that Save(...) implementation relies on the value of session.Options.MaxAge
This is prone to race conditions since Options is a pointer. Several goroutines will be changing the same sessions.Options object.

The best solution I see (and is partially implemented) is to:

• add Expiration field to the data saved in RethinkDB
• remove conditional deletion of the data saved in RethinkDB

User will have to run periodical cleanup, but it shouldn't be a problem.

Do I have a green light? :)

[boj]

Thanks for the interest. I haven't used this library at all recently so it's fallen behind a bit.

https://github.com/boj/redistore is more up to date - maybe check what people have added there as a reference and keep the implementations close?

Either way, contributions are most welcome.

[janekolszak]

I think there's the same race condition in redistore.

Another solution is to create sessions.Options object separate for creating and deleting a cookie.

[samb1729]

Can the pointer just be changed to a value instead? Since the options are part of the store's configuration it doesn't make sense that any receiver of them should be able to directly mutate the options.

This detail is made most obvious here, and I think it should be fine to update the calls to sessions.NewCookie (which take *session.Options) to use &s.Options.

[janekolszak]

Current implementation (removing cookies based on MaxAge) misleads you into thinking you don't need to periodically cleanup the database and generates unneeded traffic.

In my opinion the whole feature should be removed. Do you agree?

[samb1729]

What is your proposed solution here, the session cookie just remaining set after expiration? If it's possible to do that in a way that doesn't make session values linger for too long, and doesn't cause problems setting a future session then sounds good to me.

I'm not yet intimately familiar with either this library or gorilla sessions, so I don't know any established practices. Take my thoughts with a pinch of salt.