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

Comments

Projects
None yet
4 participants
@reusee
Copy link

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

This comment has been minimized.

Copy link
Author

commented Sep 7, 2017

A quick fix would be

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

commented Sep 7, 2017

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

@mvdan

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

commented Sep 11, 2017

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.

@tombergan tombergan closed this Sep 11, 2017

@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.
You can’t perform that action at this time.