Separate modification and saving of session data

[alexedwards]

See discussion in https://github.com/alexedwards/scs/issues/23

This could be addressed by implementing Middleware which uses a custom ResponseWriter to intercept HTTP Responses, save the session data to the store and set the session cookie - similar to v0.1 of SCS (https://github.com/alexedwards/scs/blob/v0.1.0/session/manager.go)

Downside is that this causes problems with frameworks/applications which also try to call WriteHeader. See https://github.com/alexedwards/scs/issues/15

A workaround might be to set the session cookie header each time the session is modified (or even loaded) and only handle saving to the store in the middleware. I think that would work for most stores, except the CookieStore, which needs the response headers to not have already been written.

[jpfluger]

Ultimately scs is one key ingredient to a greater solution. Since the greater solution is often based on a framework (with its own custom http handlers), it seems that fixing this issue is best left to middleware which would need to check if there's session data awaiting save and then to actually save the session data.

I now have a working example of separating writeHeader from saveStore. Although I touched multiple files, here's a gist for sessions.go. It's backwards compatible - no breaking changes.

I implemented the AutoSave idea successfully tested with Redis but should work for CookieStore. It will need to be cleaned up for locking mutex. In the gist, key modifications include:

• func write is reworked and now has two additional params for writeHeader and saveStore
• func put has checks for autoSave and saveCounter
• func Save should be called at the end of middleware
• func TouchHeader is called at the beginning of middleware, just after Load. I did look at changing load per @alexedwards suggestion but thought it would be a breaking change to include in manager.Load the ResponseWriter. For this reason TouchHeader was created.
• all other functions that call s.write are automatically set to write the header and save the data.

Here's a small example of using it in echo middleware

session := config.SessionManager.Load(c.Request())

if config.SessionManager.Opts().IdleTimeout() <= 0 {
    err := session.TouchHeader(c.Response())
    if err != nil {
        c.Error(fmt.Errorf("could not touch header for session; %v", err))
        return nil
    }
} else {
    err := session.Touch(c.Response())
    if err != nil {
        c.Error(fmt.Errorf("could not touch session; %v", err))
        return nil
    }
}

defer session.Save(c.Response())

if err := next(c); err != nil {
    c.Error(err)
}

return nil

Hopefully this helps move the conversation forward.

[alexedwards]

This issue was fixed as part of the v2.0.0 release.