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

net/http: StripPrefix drops context value set by down-stream handler #21784

Closed
reusee opened this issue Sep 7, 2017 · 6 comments
Closed

net/http: StripPrefix drops context value set by down-stream handler #21784

reusee opened this issue Sep 7, 2017 · 6 comments

Comments

@reusee
Copy link

reusee commented Sep 7, 2017

What did you do?

package main

import (
	"context"
	"fmt"
	"net/http"
)

func main() {
	http.Handle("/foo/", func(h http.Handler) http.Handler {
		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			h.ServeHTTP(w, r)
			// log user id
			fmt.Printf("UserID -> %#v\n", r.Context().Value("UserID"))
		})
	}(

		http.StripPrefix("/foo/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
			// set user id
			*r = *r.WithContext(context.WithValue(r.Context(), "UserID", "123"))
		})),

		//http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		//	// set user id
		//	*r = *r.WithContext(context.WithValue(r.Context(), "UserID", "123"))
		//}),
	))
	go func() {
		if err := http.ListenAndServe(":8888", nil); err != nil {
			panic(err)
		}
	}()
	resp, err := http.Get("http://localhost:8888/foo/")
	if err != nil {
		panic(err)
	}
	defer resp.Body.Close()
}

What did you expect to see?

UserID -> "123"

The commented out handler without StripPrefix outputs as above.

What did you see instead?

UserID -> <nil>

Does this issue reproduce with the latest release (go1.9)?

yes

System details

go version devel +d39649087b Thu Aug 31 03:30:43 2017 +0000 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/reus/go"
GORACE=""
GOROOT="/home/reus/gotip"
GOTOOLDIR="/home/reus/gotip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build879880846=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version devel +d39649087b Thu Aug 31 03:30:43 2017 +0000 linux/amd64
GOROOT/bin/go tool compile -V: compile version devel +d39649087b Thu Aug 31 03:30:43 2017 +0000
uname -sr: Linux 4.12.8-2-ARCH
LSB Version:	1.4
Distributor ID:	Arch
Description:	Arch Linux
Release:	rolling
Codename:	n/a
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.25, by Roland McGrath et al.
@reusee
Copy link
Author

reusee commented Sep 7, 2017

A quick fix would be

h.ServeHTTP(w, r2)
*r = *r.WithContext(r2.Context()) // set original request's context

@mvdan
Copy link
Member

mvdan commented Sep 7, 2017

Note that WithContext creates a request copy on purpose - context values are layered and passed on, not attached to the original request forever.

@reusee
Copy link
Author

reusee commented Sep 7, 2017

@mvdan Is there an idiomatic way to pass back context value to upstream handler?

@mvdan
Copy link
Member

mvdan commented Sep 7, 2017

Well, I guess what you're doing - modifying the request in-place - is as good as it gets. But it goes against the design of a request's context, so using the standard library will be painful - the request may be copied and the pointer changed, as you have seen.

The question to ask here is likely why you'd need a variable to go back to the upstream handler in the first place. I don't think using contexts is a good idea, as they're for the exact opposite.

I would suggest taking this to https://github.com/golang/go/wiki/Questions, as the issue tracker is used for bugs, not questions.

@reusee
Copy link
Author

reusee commented Sep 7, 2017

I've update the demo codes to reflect what the real codes used to be doing before 1.9. I can workaround the limits of StripPrefix by passing a setter function in context to downstream handler and call it to pass back something.

@tombergan
Copy link
Contributor

The question to ask here is likely why you'd need a variable to go back to the upstream handler in the first place.

I have the same question. StripPrefix is almost certainly not going to change. The composition seems backwards in your example. I would nest the handler within the StripPrefix handler, rather than visa-versa as your example does. e.g.:

http.Handle("/foo/", http.StripPrefix("/foo/", func(rw http.ResponseWriter, r *http.Request) {
  ... this code can modify r however it wants ...
}))

I'm closing this because I don't see a bug, but if you find a bug or have a more detailed feature request, please reopen.

@golang golang locked and limited conversation to collaborators Sep 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants