Skip to content
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

This is a bug or my usage is wrong? #7

Closed
dxvgef opened this issue Dec 24, 2016 · 1 comment
Closed

This is a bug or my usage is wrong? #7

dxvgef opened this issue Dec 24, 2016 · 1 comment

Comments

@dxvgef
Copy link

dxvgef commented Dec 24, 2016

I use echo v3, I repeat the request controller (refresh page), it will create a number of records in redis scs / session, this is a bug or I use the wrong method?
the code is probably this:

main.go

engine := redisstore.New(
	&redis.Pool{
		MaxIdle: 10,
		Dial: func() (redis.Conn, error) {
			return redis.Dial("tcp", "127.0.0.1:6379")
		},
	},
)

sessionManager := session.Manage(
	engine,
	session.HttpOnly(true),
	session.IdleTimeout(20*time.Minute),
	session.Lifetime(3*time.Hour),
)
App.Use(echo.WrapMiddleware(sessionManager))

Controller

func Index(ctx echo.Context) error {
	data := make(map[string]interface{})
	session.PutString(ctx.Request(), "sess", "ok")
	data["sess"], _ = session.GetString(ctx.Request(), "sess")
	return ctx.Render(200, "index.jet", data)
}

Refresh three pages, redis appeared in three records, I think this is not normal?

1
2
3

@dxvgef dxvgef changed the title This is a BUG or my usage is wrong? This is a bug or my usage is wrong? Dec 24, 2016
@alexedwards
Copy link
Owner

alexedwards commented Apr 23, 2017

Sorry for the (very) slow reply.

This is happening because the session cookie is never being written by the SCS middleware. I think the session cookie isn't being written because Echo's WrapMiddleware doesn't actually pass the underlying http.ResponseWriter onwards -- it replaces it entirely it's own c.Response() object instead and the session manager's Write method is therefore never called.

func WrapMiddleware(m func(http.Handler) http.Handler) MiddlewareFunc {
	return func(next HandlerFunc) HandlerFunc {
		return func(c Context) (err error) {
			m(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
				c.SetRequest(r)
				err = next(c)
			})).ServeHTTP(c.Response(), c.Request())
			return
		}
	}
}

Unless Echo can be changed so that WrapMiddleware extends the underlying http.ResponseWriter instead of replacing it, the only 'fix' I can see is to call session.Save manually in your handlers which modify the session data, like so:

func Index(ctx echo.Context) error {
	data := make(map[string]interface{})
	session.PutString(ctx.Request(), "sess", "ok")
        _ = session.Save(c.Response(), c.Request())
	data["sess"], _ = session.GetString(ctx.Request(), "sess")
	return ctx.Render(200, "index.jet", data)
}

This essentially writes the session data and cookie, instead of relying on the middleware to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants