From 9cab33ca8e480d9a24efdd84433601a80f9c7fb5 Mon Sep 17 00:00:00 2001 From: Sameer Ajmani Date: Tue, 29 Jul 2014 22:32:12 -0400 Subject: go.blog/context: don't use the address of an empty struct as a context key, as this is not guaranteed to be unique. Fixes golang/go#8443. LGTM=campoy R=adg, campoy CC=dsymonds, golang-codereviews https://golang.org/cl/121890043 --- content/context.article | 12 ++++++------ content/context/gorilla/gorilla.go | 8 +++++--- content/context/userip/userip.go | 14 ++++++++++---- 3 files changed, 21 insertions(+), 13 deletions(-) (limited to 'content') diff --git a/content/context.article b/content/context.article index 73de06d..054a911 100644 --- a/content/context.article +++ b/content/context.article @@ -40,9 +40,9 @@ The `Err` method returns an error indicating why the `Context` was canceled. The [[/pipelines][Pipelines and Cancelation]] article discusses the `Done` channel idiom in more detail. -A `Context` does _not_ have a `Cancel` method for same reason the `Done` channel -is receive-only: the function receiving a cancelation signal is usually not the -one that sends the signal. +A `Context` does _not_ have a `Cancel` method for the same reason the `Done` +channel is receive-only: the function receiving a cancelation signal is usually +not the one that sends the signal. In particular, when a parent operation starts goroutines for sub-operations, those sub-operations should not be able to cancel the parent. Instead, the `WithCancel` function (described below) provides a way to cancel a @@ -140,10 +140,10 @@ multiple goroutines. Packages like `userip` hide the details of this mapping and provide strongly-typed access to a specific `Context` value. -To avoid key collisions, `userip` defines an unexported variable `key` and uses -its address as the context key: +To avoid key collisions, `userip` defines an unexported type `key` and uses +a value of this type as the context key: -.code context/userip/userip.go /var key/ +.code context/userip/userip.go /The key type/,/const userIPKey/ `FromRequest` extracts a `userIP` value from an `http.Request`: diff --git a/content/context/gorilla/gorilla.go b/content/context/gorilla/gorilla.go index 08267c3..c9b6474 100644 --- a/content/context/gorilla/gorilla.go +++ b/content/context/gorilla/gorilla.go @@ -22,12 +22,14 @@ type wrapper struct { req *http.Request } -var reqKey struct{} +type key int + +const reqKey key = 0 // Value returns Gorilla's context package's value for this Context's request // and key. It delegates to the parent Context if there is no such value. func (ctx *wrapper) Value(key interface{}) interface{} { - if key == &reqKey { + if key == reqKey { return ctx.req } if val, ok := gcontext.GetOk(ctx.req, key); ok { @@ -42,6 +44,6 @@ func HTTPRequest(ctx context.Context) (*http.Request, bool) { // We cannot use ctx.(*wrapper).req to get the request because ctx may // be a Context derived from a *wrapper. Instead, we use Value to // access the request if it is anywhere up the Context tree. - req, ok := ctx.Value(&reqKey).(*http.Request) + req, ok := ctx.Value(reqKey).(*http.Request) return req, ok } diff --git a/content/context/userip/userip.go b/content/context/userip/userip.go index 3001ade..ba3851e 100644 --- a/content/context/userip/userip.go +++ b/content/context/userip/userip.go @@ -21,18 +21,24 @@ func FromRequest(req *http.Request) (net.IP, error) { return userIP, nil } -// The address &key is the context key. -var key struct{} +// The key type is unexported to prevent collisions with context keys defined in +// other packages. +type key int + +// userIPkey is the context key for the user IP address. Its value of zero is +// arbitrary. If this package defined other context keys, they would have +// different integer values. +const userIPKey key = 0 // NewContext returns a new Context carrying userIP. func NewContext(ctx context.Context, userIP net.IP) context.Context { - return context.WithValue(ctx, &key, userIP) + return context.WithValue(ctx, userIPKey, userIP) } // FromContext extracts the user IP address from ctx, if present. func FromContext(ctx context.Context) (net.IP, bool) { // ctx.Value returns nil if ctx has no value for the key; // the net.IP type assertion returns ok=false for nil. - userIP, ok := ctx.Value(&key).(net.IP) + userIP, ok := ctx.Value(userIPKey).(net.IP) return userIP, ok } -- cgit v1.2.3