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.

Sign up for GitHub

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

Jump to bottom

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 •
edited

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.

@GwynethLlewelyn GwynethLlewelyn mentioned this issue Jun 28, 2020
Open
@KeiichiHirobe KeiichiHirobe linked a pull request Mar 19, 2022 that will close this issue
Open
@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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

delete context.Clear and dependency to gorilla/context KeiichiHirobe/sessions
5 participants
@maximerety @GwynethLlewelyn @marvin-min @xuwei0455 @KeiichiHirobe