justinas/nosurf

Token value error

hellower opened this issue · 6 comments

------ context_legacy.go --------- (good)
func Token(req *http.Request) string {
cmMutex.RLock()
defer cmMutex.RUnlock()

ctx, ok := contextMap[req]

if !ok {
	return ""
}

return ctx.token

}

---------- context.go ---------------------(error)
// Token takes an HTTP request and returns
// the CSRF token for that request
// or an empty string if the token does not exist. <----------- ERROR
//
func Token(req *http.Request) string {
ctx := req.Context().Value(nosurfKey).(*csrfContext)

return ctx.token

}

...an empty string if the token does not exist. --> panic occurred !!
PANIC: interface conversion: interface {} is nil, not *nosurf.csrfContext

sorry for poor english.

nosurf.nosurfKey is a private variable of a private type, so you cannot reference the token from the context directly. You should use the methods nosurf provides for interacting with tokens.

@hellower Thanks for pointing this out, this is an incosistency between the legacy version and the net/context version.

The reason why you try to retrieve a Token while context does not exist yet might be one of these:

  1. Caused by another inconsistency: addNosurfContext() will actually allocate an empty context struct in Go 1.7, but in older versions it will not create one until a Reason or a Token is set.
  2. You are using Token() in a route that is not protected by nosurf.
  3. You are using Token() in outer middleware that wraps nosurf.

Nevertheless, I will be fixing this soon as this is unwanted and inconsistent behavior in any case. In the meantime, could you supply me with your code and Go version?

I just came to open an issue on the same thing, which bit me when trying to unit test a handler. A minimal example:

r, _ := http.NewRequest("GET", "/", nil)
token := nosurf.Token(r)
log.Println(token)

Results in a panic panic: interface conversion: interface {} is nil, not *nosurf.csrfContext.

In itself that's OK, but I was expecting the empty string as specified in the documentation for the Token function:

// Token takes an HTTP request and returns the CSRF token for that request or an empty string if the token does not exist.

Is there a problem with using comma, ok to attempt the assertion and return the empty string if it fails? Like:

func Token(req *http.Request) string {
	ctx, ok := req.Context().Value(nosurfKey).(*csrfContext)
        if !ok {
              return ""
        }
	return ctx.token
}

go version go1.10.2 linux/amd64

Or as an alternative, possibly add a func MockRequest(*http.Request) *http.Request which injects a csrfContext into a request? Just so there is a way to unit test handlers which call Token() without them panicing.

#49 has been merged and fixed this.

I'm testing nosurf in a new project, and after working well for a long time, this panic happened once:

2020/08/09 03:41:59 http: panic serving 127.0.0.1:54032: interface conversion: interface {} is nil, not *nosurf.csrfContext
goroutine 361 [running]:
net/http.(*conn).serve.func1(0xc0000c2280)
	/usr/local/Cellar/go/1.14.6/libexec/src/net/http/server.go:1800 +0x139
panic(0x1adf8c0, 0xc000130030)
	/usr/local/Cellar/go/1.14.6/libexec/src/runtime/panic.go:975 +0x3e3
github.com/justinas/nosurf.ctxSetToken(0xc000284800, 0xc0005aa240, 0x20, 0x20)
	/Users/henvic/projects/gocode/pkg/mod/github.com/justinas/nosurf@v1.1.0/context.go:52 +0x12c
github.com/justinas/nosurf.(*CSRFHandler).setTokenCookie(0xc000113560, 0x1e84800, 0xc00053e000, 0xc000284800, 0xc0005aa240, 0x20, 0x20)
	/Users/henvic/projects/gocode/pkg/mod/github.com/justinas/nosurf@v1.1.0/handler.go:206 +0x6a
github.com/justinas/nosurf.(*CSRFHandler).RegenerateToken(0xc000113560, 0x1e84800, 0xc00053e000, 0xc000284800, 0x56, 0x0)
	/Users/henvic/projects/gocode/pkg/mod/github.com/justinas/nosurf@v1.1.0/handler.go:199 +0x6f
github.com/plifk/market/internal/services.(*CSRFProtection).RegenerateToken(...)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/internal/services/security.go:32
github.com/plifk/market/internal/services.(*Security).RegenerateCSRFToken(...)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/internal/services/security.go:16
github.com/plifk/market/internal/webpages.(*LoginHandler).loginPostHandler(0xc0004baab0, 0x1e84800, 0xc00053e000, 0xc000284800)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/internal/webpages/authentication.go:123 +0x3bb
github.com/plifk/market/internal/webpages.(*LoginHandler).ServeHTTP(0xc0004baab0, 0x1e84800, 0xc00053e000, 0xc000284800)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/internal/webpages/authentication.go:47 +0x178
github.com/plifk/market/internal/webpages.(*Router).ServeHTTP(0xc0004fc390, 0x1e84800, 0xc00053e000, 0xc000284800)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/internal/webpages/webpages.go:114 +0x28a
github.com/plifk/market.(*System).ServeHTTP(0xc000364ab0, 0x1e84800, 0xc00053e000, 0xc000284700)
	/Users/henvic/projects/gocode/src/github.com/plifk/market/http.go:29 +0xc4
github.com/henvic/httpretty.httpHandler.ServeHTTP(0xc0004c9500, 0x1e77bc0, 0xc000364ab0, 0x1e84800, 0xc00053e000, 0xc000284700)
	/Users/henvic/projects/gocode/pkg/mod/github.com/henvic/httpretty@v0.0.5/httpretty.go:346 +0x41b
net/http.serverHandler.ServeHTTP(0xc000358620, 0x1e84800, 0xc00053e000, 0xc000284700)
	/usr/local/Cellar/go/1.14.6/libexec/src/net/http/server.go:2836 +0xa3
net/http.(*conn).serve(0xc0000c2280, 0x1e88ec0, 0xc000434080)
	/usr/local/Cellar/go/1.14.6/libexec/src/net/http/server.go:1924 +0x86c
created by net/http.(*Server).Serve
	/usr/local/Cellar/go/1.14.6/libexec/src/net/http/server.go:2962 +0x35c

Versions:

$ go version
go version go1.14.6 darwin/amd64

$ cat go.mod | grep nosurf
	github.com/justinas/nosurf v1.1.0

Update: this actually happened when I forgot to initialize the nosurf middleware and set it as the HTTP handler.