Document Conflict between gorilla/context & Go 1.7's http.Request.WithContext()

[elithrar]

RE: gorilla/mux#182

OK, so this happens because r.WithContext() in Go 1.7, which gorilla/mux uses in Go 1.7, creates a shallow copy of the request: https://github.com/golang/go/blob/master/src/net/http/request.go#L288-L298

This shallow copy, as you'd expect, changes the address of the request in gorilla/context's map[*http.Request]map..., effectively islanding any context values.

This is a shortcoming in gorilla/context: any library, mux included, will break it come Go 1.7 and the http.Request.WithContext() method.`

I'm not sure if there's a clean way to fix this: one option would be to call context.GetAll to copy all values from the old request before we call WithContext, and then save them back into the (shallow) copy, but if you have private types as keys (which is recommended), then we can't reliably achieve that. It would also add context back as a dependency to mux.

• Add a warning to the README
• Document public API so that godoc users also notice the warning when using getters/setters.

cc/ @kisielk

[jordan-wright]

Hi there!

I have a project that uses gorilla/context to store context info on a request. Now that Go 1.7 is out, things aren't working anymore - it's related to this issue.

What's the recommended way to keep context info that's backwards compatible? Is it something like this?

Any help is greatly appreciated - I'm a big fan of the gorilla suite.

[elithrar]

@jordan-wright Yep, use 1.7's context in its entirety. Mixing and matching that and any kind of request map (like gorilla/context) will break things.

[jordan-wright]

Gotcha. Thanks for the quick reply! The issue that I foresee is that the context package was under a different path pre 1.7, and backwards compatibility is important.

You're saying that it's probably best to basically create an abstracted package (like the one I linked to used by mux) that will use the right context package depending on the Go version using build flags?

[elithrar]

@jordan-wright Yep. There really isn't a better way. mux could have kept using gorilla/context in 1.7, but any user who uses (directly, or via a lib) the new http.Request.WithContext() function would break things.

[jordan-wright]

mux could have kept using gorilla/context in 1.7

So I guess that's the part that's throwing me for a loop. I thought gorilla/context was just broken in 1.7.

To give a concrete (simplified) example, my setup is something like this - I've tried to slim it down as much as possible:

import (
    ctx "github.com/gorilla/context"
    "github.com/gorilla/mux"
    "github.com/gorilla/sessions"
    "github.com/gorilla/securecookie"
)

var Store = sessions.NewCookieStore(
    []byte(securecookie.GenerateRandomKey(64)),
    []byte(securecookie.GenerateRandomKey(32)))

router := mux.NewRouter()
router.HandleFunc("/login", Login)

func Login(w http.ResponseWriter, r *http.Request) {
    // The session SET'ing is actually handled in middleware but
    // I wanted to keep things simple
    session, _ := Store.Get(r, "gophish")
    ctx.Set(r, "session", session)
    session := ctx.Get(r, "session").(*sessions.Session)
    // do login stuff
}

So I'm using mux, sessions, and context. The issue I'm running into strictly with Go 1.7 when hitting /login on the line that ctx.Get's the session and type converts is this:

http: panic: interface conversion: interface {} is nil, not *sessions.Session

I'm not using http.Request.WithContext() anywhere... Is the issue that I'm using mux which now uses the new stdlib context package or do you think the issue lies somewhere else?

[elithrar]

mux uses the new stdlib context, hence the issue. Other libraries reported
issues with mux (before it changed), so mux was updated.

Any mixture of any packages that mix the two causes the issue.
gorilla/context is effectively deprecated from Go 1.7 onwards.
On Sun, Sep 11, 2016 at 5:17 PM Jordan Wright notifications@github.com
wrote:

mux could have kept using gorilla/context in 1.7

So I guess that's the part that's throwing me for a loop. I thought
gorilla/context was just broken in 1.7.

To give a concrete (simplified) example, my setup is something like this -
I've tried to slim it down as much as possible:

import (
ctx "github.com/gorilla/context"
"github.com/gorilla/mux"
"github.com/gorilla/sessions"
"github.com/gorilla/securecookie"
)

var Store = sessions.NewCookieStore(
[]byte(securecookie.GenerateRandomKey(64)),
[]byte(securecookie.GenerateRandomKey(32)))

router := mux.NewRouter()
router.HandleFunc("/login", Login)

func Login(w http.ResponseWriter, r _http.Request) {
// The session SET'ing is actually handled in middleware but
// I wanted to keep things simple
session, _ := Store.Get(r, "gophish")
ctx.Set(r, "session", session)
session := ctx.Get(r, "session").(_sessions.Session)
// do login stuff
}

So I'm using mux, sessions, and context. The issue I'm running into
strictly with Go 1.7 when hitting /login on the line that ctx.Get's the
session and type converts is this:

http: panic: interface conversion: interface {} is nil, not *sessions.Session

I'm not using http.Request.WithContext() anywhere... Is the issue that
I'm using mux which now uses the new stdlib context package or do you think
the issue lies somewhere else?

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#32 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABIcNnRp1C0Qlmb1yBe3PwKR9iWcESLks5qpJokgaJpZM4Jjt1d
.

[jordan-wright]

Roger that. Thanks for the quick responses, and for all your great work on the Gorilla suite!

[codeskyblue]

@elithrar But it looks gorilla/context is easy to use and understand then the http.Request.WithContext()
Really hope this lib still works.

[elithrar]

@codeskyblue It "works", but if you - or any library you import - uses the new WithContext method on the request, it will break gorilla/context (thus, it doesn't work in practice, and can't be reasonably fixed).

[codeskyblue]

Finally, I modified context, replace *http.Request to *url.URL to make it work after go1.7
master...codeskyblue:master