From 2566e21f243387156e8e7f2acad0ce14d9712bbc Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 4 Nov 2019 04:00:29 +0000 Subject: [PATCH] net/http: support disabling built-in HTTP/2 with a new build tag Fixes #35082 Updates #6853 Change-Id: I4eeb0e15f534cff57fefb6039cd33fadf15b946e Reviewed-on: https://go-review.googlesource.com/c/go/+/205139 Reviewed-by: Emmanuel Odeke Reviewed-by: David Crawshaw --- src/net/http/clientserver_test.go | 3 ++ src/net/http/export_test.go | 6 +++ src/net/http/h2_bundle.go | 4 +- src/net/http/http.go | 7 +++ src/net/http/http_test.go | 26 +++++++++++ src/net/http/main_test.go | 3 ++ src/net/http/omithttp2.go | 71 +++++++++++++++++++++++++++++++ src/net/http/serve_test.go | 2 + src/net/http/server.go | 2 +- src/net/http/transport.go | 7 ++- src/net/http/transport_test.go | 3 ++ 11 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 src/net/http/omithttp2.go diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index c3877d7071..def5c424f0 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -83,6 +83,9 @@ func optWithServerLog(lg *log.Logger) func(*httptest.Server) { } func newClientServerTest(t *testing.T, h2 bool, h Handler, opts ...interface{}) *clientServerTest { + if h2 { + CondSkipHTTP2(t) + } cst := &clientServerTest{ t: t, h2: h2, diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go index e5c06a8903..657ff9dba4 100644 --- a/src/net/http/export_test.go +++ b/src/net/http/export_test.go @@ -60,6 +60,12 @@ func init() { } } +func CondSkipHTTP2(t *testing.T) { + if omitBundledHTTP2 { + t.Skip("skipping HTTP/2 test when nethttpomithttp2 build tag in use") + } +} + var ( SetEnterRoundTripHook = hookSetter(&testHookEnterRoundTrip) SetRoundTripRetried = hookSetter(&testHookRoundTripRetried) diff --git a/src/net/http/h2_bundle.go b/src/net/http/h2_bundle.go index ad00f0611b..a583a0d6cb 100644 --- a/src/net/http/h2_bundle.go +++ b/src/net/http/h2_bundle.go @@ -1,5 +1,7 @@ +// +build !nethttpomithttp2 + // Code generated by golang.org/x/tools/cmd/bundle. DO NOT EDIT. -//go:generate bundle -o h2_bundle.go -prefix http2 golang.org/x/net/http2 +// $ bundle -o=h2_bundle.go -prefix=http2 -tags=!nethttpomithttp2 golang.org/x/net/http2 // Package http2 implements the HTTP/2 protocol. // diff --git a/src/net/http/http.go b/src/net/http/http.go index 3510fe604d..89e86d80e8 100644 --- a/src/net/http/http.go +++ b/src/net/http/http.go @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. +//go:generate bundle -o=h2_bundle.go -prefix=http2 -tags=!nethttpomithttp2 golang.org/x/net/http2 + package http import ( @@ -22,6 +24,11 @@ const maxInt64 = 1<<63 - 1 // immediate cancellation of network operations. var aLongTimeAgo = time.Unix(1, 0) +// omitBundledHTTP2 is set by omithttp2.go when the nethttpomithttp2 +// build tag is set. That means h2_bundle.go isn't compiled in and we +// shouldn't try to use it. +var omitBundledHTTP2 bool + // TODO(bradfitz): move common stuff here. The other files have accumulated // generic http stuff in random places. diff --git a/src/net/http/http_test.go b/src/net/http/http_test.go index 224b46c796..f4ea52db3b 100644 --- a/src/net/http/http_test.go +++ b/src/net/http/http_test.go @@ -111,6 +111,32 @@ func TestCmdGoNoHTTPServer(t *testing.T) { } } +// Tests that the nethttpomithttp2 build tag doesn't rot too much, +// even if there's not a regular builder on it. +func TestOmitHTTP2(t *testing.T) { + if testing.Short() { + t.Skip("skipping in short mode") + } + t.Parallel() + goTool := testenv.GoToolPath(t) + out, err := exec.Command(goTool, "test", "-short", "-tags=nethttpomithttp2", "net/http").CombinedOutput() + if err != nil { + t.Fatalf("go test -short failed: %v, %s", err, out) + } +} + +// Tests that the nethttpomithttp2 build tag at least type checks +// in short mode. +// The TestOmitHTTP2 test above actually runs tests (in long mode). +func TestOmitHTTP2Vet(t *testing.T) { + t.Parallel() + goTool := testenv.GoToolPath(t) + out, err := exec.Command(goTool, "vet", "-tags=nethttpomithttp2", "net/http").CombinedOutput() + if err != nil { + t.Fatalf("go vet failed: %v, %s", err, out) + } +} + var valuesCount int func BenchmarkCopyValues(b *testing.B) { diff --git a/src/net/http/main_test.go b/src/net/http/main_test.go index 85aa9096c3..35cc80977c 100644 --- a/src/net/http/main_test.go +++ b/src/net/http/main_test.go @@ -90,6 +90,9 @@ func goroutineLeaked() bool { // (all.bash), but as a serial test otherwise. Using t.Parallel isn't // compatible with the afterTest func in non-short mode. func setParallel(t *testing.T) { + if strings.Contains(t.Name(), "HTTP2") { + http.CondSkipHTTP2(t) + } if testing.Short() { t.Parallel() } diff --git a/src/net/http/omithttp2.go b/src/net/http/omithttp2.go new file mode 100644 index 0000000000..a0b33e9aad --- /dev/null +++ b/src/net/http/omithttp2.go @@ -0,0 +1,71 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build nethttpomithttp2 + +package http + +import ( + "errors" + "sync" + "time" +) + +func init() { + omitBundledHTTP2 = true +} + +const noHTTP2 = "no bundled HTTP/2" // should never see this + +var http2errRequestCanceled = errors.New("net/http: request canceled") + +var http2goAwayTimeout = 1 * time.Second + +const http2NextProtoTLS = "h2" + +type http2Transport struct { + MaxHeaderListSize uint32 + ConnPool interface{} +} + +func (*http2Transport) RoundTrip(*Request) (*Response, error) { panic(noHTTP2) } +func (*http2Transport) CloseIdleConnections() {} + +type http2erringRoundTripper struct{} + +func (http2erringRoundTripper) RoundTrip(*Request) (*Response, error) { panic(noHTTP2) } + +type http2noDialClientConnPool struct { + http2clientConnPool http2clientConnPool +} + +type http2clientConnPool struct { + mu *sync.Mutex + conns map[string][]struct{} +} + +func http2configureTransport(*Transport) (*http2Transport, error) { panic(noHTTP2) } + +func http2isNoCachedConnError(err error) bool { + _, ok := err.(interface{ IsHTTP2NoCachedConnError() }) + return ok +} + +type http2Server struct { + NewWriteScheduler func() http2WriteScheduler +} + +type http2WriteScheduler interface{} + +func http2NewPriorityWriteScheduler(interface{}) http2WriteScheduler { panic(noHTTP2) } + +func http2ConfigureServer(s *Server, conf *http2Server) error { panic(noHTTP2) } + +var http2ErrNoCachedConn = http2noCachedConnError{} + +type http2noCachedConnError struct{} + +func (http2noCachedConnError) IsHTTP2NoCachedConnError() {} + +func (http2noCachedConnError) Error() string { return "http2: no cached connection was available" } diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 4c53c95eda..af43421fce 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -1507,6 +1507,7 @@ func TestTLSServer(t *testing.T) { } func TestServeTLS(t *testing.T) { + CondSkipHTTP2(t) // Not parallel: uses global test hooks. defer afterTest(t) defer SetTestHookServerServe(nil) @@ -1657,6 +1658,7 @@ func TestAutomaticHTTP2_ListenAndServe_GetCertificate(t *testing.T) { } func testAutomaticHTTP2_ListenAndServe(t *testing.T, tlsConf *tls.Config) { + CondSkipHTTP2(t) // Not parallel: uses global test hooks. defer afterTest(t) defer SetTestHookServerServe(nil) diff --git a/src/net/http/server.go b/src/net/http/server.go index b2c071fc21..4f1c73dbdf 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -3160,7 +3160,7 @@ func (srv *Server) onceSetNextProtoDefaults_Serve() { // configured otherwise. (by setting srv.TLSNextProto non-nil) // It must only be called via srv.nextProtoOnce (use srv.setupHTTP2_*). func (srv *Server) onceSetNextProtoDefaults() { - if strings.Contains(os.Getenv("GODEBUG"), "http2server=0") { + if omitBundledHTTP2 || strings.Contains(os.Getenv("GODEBUG"), "http2server=0") { return } // Enable HTTP/2 by default if the user hasn't otherwise diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 8989f65f25..6fade795ab 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -286,7 +286,6 @@ func (t *Transport) Clone() *Transport { DialContext: t.DialContext, Dial: t.Dial, DialTLS: t.DialTLS, - TLSClientConfig: t.TLSClientConfig.Clone(), TLSHandshakeTimeout: t.TLSHandshakeTimeout, DisableKeepAlives: t.DisableKeepAlives, DisableCompression: t.DisableCompression, @@ -302,6 +301,9 @@ func (t *Transport) Clone() *Transport { WriteBufferSize: t.WriteBufferSize, ReadBufferSize: t.ReadBufferSize, } + if t.TLSClientConfig != nil { + t2.TLSClientConfig = t.TLSClientConfig.Clone() + } if !t.tlsNextProtoWasNil { npm := map[string]func(authority string, c *tls.Conn) RoundTripper{} for k, v := range t.TLSNextProto { @@ -359,6 +361,9 @@ func (t *Transport) onceSetNextProtoDefaults() { // However, if ForceAttemptHTTP2 is true, it overrides the above checks. return } + if omitBundledHTTP2 { + return + } t2, err := http2configureTransport(t) if err != nil { log.Printf("Error enabling Transport HTTP/2 support: %v", err) diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 00d6b2608b..692868094c 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -591,6 +591,7 @@ func TestTransportMaxConnsPerHostIncludeDialInProgress(t *testing.T) { func TestTransportMaxConnsPerHost(t *testing.T) { defer afterTest(t) + CondSkipHTTP2(t) h := HandlerFunc(func(w ResponseWriter, r *Request) { _, err := w.Write([]byte("foo")) @@ -3994,6 +3995,7 @@ func TestTransportAutomaticHTTP2_DialTLS(t *testing.T) { } func testTransportAutoHTTP(t *testing.T, tr *Transport, wantH2 bool) { + CondSkipHTTP2(t) _, err := tr.RoundTrip(new(Request)) if err == nil { t.Error("expected error from RoundTrip") @@ -5896,6 +5898,7 @@ func TestDontCacheBrokenHTTP2Conn(t *testing.T) { // only be one decrement regardless of the number of failures. func TestTransportDecrementConnWhenIdleConnRemoved(t *testing.T) { defer afterTest(t) + CondSkipHTTP2(t) h := HandlerFunc(func(w ResponseWriter, r *Request) { _, err := w.Write([]byte("foo")) -- 2.50.0