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: Server BaseContext & ConnContext not respected for HTTP2 #32476

Closed
tmthrgd opened this issue Jun 7, 2019 · 2 comments
Closed

net/http: Server BaseContext & ConnContext not respected for HTTP2 #32476

tmthrgd opened this issue Jun 7, 2019 · 2 comments

Comments

@tmthrgd
Copy link
Contributor

@tmthrgd tmthrgd commented Jun 7, 2019

What version of Go are you using (go version)?

$ go version
go version devel +eb2fabf2fa Fri Jun 7 13:56:20 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/tom/.cache/go-build"
GOENV="/home/tom/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/tom/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/tom/go/gotip-eb2fabf2fa"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/tom/go/gotip-eb2fabf2fa/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/tom/go/gotip-eb2fabf2fa/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build584953743=/tmp/go-build -gno-record-gcc-switches"

What did you do?

CL 167681 (for #30694) introduced the BaseContext & ConnContext fields on http.Server, but they were never wired up in x/net/http2.

The following is a slight modification of the TestServerContexts test in serve_test.go that uses HTTP2:

func TestServerContextsHTTP2(t *testing.T) {
	setParallel(t)
	defer afterTest(t)
	type baseKey struct{}
	type connKey struct{}
	ch := make(chan context.Context, 1)
	ts := httptest.NewUnstartedServer(HandlerFunc(func(rw ResponseWriter, r *Request) {
		if r.ProtoMajor != 2 {
			t.Errorf("unexpected HTTP/1.x request")
		}
		ch <- r.Context()
	}))
	ts.Config.BaseContext = func(ln net.Listener) context.Context {
		if strings.Contains(reflect.TypeOf(ln).String(), "onceClose") {
			t.Errorf("unexpected onceClose listener type %T", ln)
		}
		return context.WithValue(context.Background(), baseKey{}, "base")
	}
	ts.Config.ConnContext = func(ctx context.Context, c net.Conn) context.Context {
		if got, want := ctx.Value(baseKey{}), "base"; got != want {
			t.Errorf("in ConnContext, base context key = %#v; want %q", got, want)
		}
		return context.WithValue(ctx, connKey{}, "conn")
	}
	ts.TLS = &tls.Config{
		NextProtos: []string{"h2", "http/1.1"},
	}
	ts.StartTLS()
	defer ts.Close()
	ts.Client().Transport.(*Transport).ForceAttemptHTTP2 = true
	res, err := ts.Client().Get(ts.URL)
	if err != nil {
		t.Fatal(err)
	}
	res.Body.Close()
	ctx := <-ch
	if got, want := ctx.Value(baseKey{}), "base"; got != want {
		t.Errorf("base context key = %#v; want %q", got, want)
	}
	if got, want := ctx.Value(connKey{}), "conn"; got != want {
		t.Errorf("conn context key = %#v; want %q", got, want)
	}
}

Then go test:

$ go test -v -run TestServerContexts net/http

What did you expect to see?

=== RUN   TestServerContexts
--- PASS: TestServerContexts (0.00s)
=== RUN   TestServerContextsHTTP2
--- PASS: TestServerContextsHTTP2 (0.00s)
PASS
ok  	net/http	0.005s

What did you see instead?

=== RUN   TestServerContexts
--- PASS: TestServerContexts (0.00s)
=== RUN   TestServerContextsHTTP2
--- FAIL: TestServerContextsHTTP2 (0.00s)
    serve_test.go:6106: base context key = <nil>; want "base"
    serve_test.go:6109: conn context key = <nil>; want "conn"
FAIL
FAIL	net/http	0.005s

x/net/http2 doesn't use the http.Server context–as it isn't provided to the TLSNextProto handler–instead creating it's own in serverConnBaseContext. This code currently lacks the BaseContext & ConnContext calls.

Also while I'm here, the two panics introduced by CL 167681 (here and here) should have a http: prefix.

/cc @bradfitz

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 7, 2019

Change https://golang.org/cl/181259 mentions this issue: http2: support getting the Server connection's base context from net/http

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 7, 2019

Change https://golang.org/cl/181260 mentions this issue: net/http: support BaseContext & ConnContext for http2 Server

gopherbot pushed a commit to golang/net that referenced this issue Jun 7, 2019
…http

This is the x/net/http2 half of the fix. The net/http half is in CL 181260.

Updates golang/go#32476
Updates golang/go#30694

Change-Id: Ic25c678dad99acc4ae8d679384d9e9a38dc1291c
Reviewed-on: https://go-review.googlesource.com/c/net/+/181259
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot gopherbot closed this in 4c84d87 Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.