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

Remove the dependency of github.com/gorilla/context #92

Open
xuwei0455 opened this issue Jul 23, 2019 · 4 comments · May be fixed by #151
Open

Remove the dependency of github.com/gorilla/context #92

xuwei0455 opened this issue Jul 23, 2019 · 4 comments · May be fixed by #151

Comments

@xuwei0455
Copy link

I found github.com/gorilla/context had been in maintenance mode, After some search of dependency for it.

I truly just find one place that uses the gorilla context, as follows:
https://github.com/gin-contrib/sessions/blob/master/sessions.go#L64

func Sessions(name string, store Store) gin.HandlerFunc {
	return func(c *gin.Context) {
		s := &session{name, c.Request, store, nil, false, c.Writer}
		c.Set(DefaultKey, s)
		defer context.Clear(c.Request)
		c.Next()
	}
}

even github.com/gorilla/sessionshad removed the dependency, so why we need to keep the dependency?

@maximerety
Copy link

maximerety commented Jun 23, 2020

There are two occurrences now, with the addition of SessionsMany.

This was an emulation of context from a time when it wasn't a part of the standard library yet (golang < 1.7).

But what's bothering me the most is that the session is actually stored in a gin.Context (c.Set(DefaultKey, s)), which is definitely not the context that is cleared by defer context.Clear(c.Request).

Either I'm missing something or this does not make sense.

@GwynethLlewelyn
Copy link

Ah! That certainly explains why I cannot clear a session, no matter how hard I try. It does not make any sense to me, either... see also #89 — I believe it might be related to this.

@KeiichiHirobe
Copy link

@maximerety
FYI: #151

@marvin-min
Copy link

Got the same issue. Why 151 didn't get merged?

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

Successfully merging a pull request may close this issue.

5 participants