From b5f0aff49503e31002b33198e06708e263c445a7 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 27 Jun 2016 16:39:40 -0700 Subject: net/http: conditionally configure HTTP/2 in Server.Serve(Listener) Don't configure HTTP/2 in http.Server.Serve(net.Listener) if the Server's TLSConfig is set and doesn't include the "h2" NextProto value. This avoids mutating a *tls.Config already in use if previously passed to tls.NewListener. Also document this. (it's come up a few times now) Fixes #15908 Change-Id: I283eed82fdb29a791f80d801aadd9f75db244de0 Reviewed-on: https://go-review.googlesource.com/24508 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/net/http/server.go | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) (limited to 'src/net/http/server.go') diff --git a/src/net/http/server.go b/src/net/http/server.go index a1c48272fd..7c3237c4cd 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2222,9 +2222,37 @@ func (srv *Server) ListenAndServe() error { var testHookServerServe func(*Server, net.Listener) // used if non-nil +// shouldDoServeHTTP2 reports whether Server.Serve should configure +// automatic HTTP/2. (which sets up the srv.TLSNextProto map) +func (srv *Server) shouldConfigureHTTP2ForServe() bool { + if srv.TLSConfig == nil { + // Compatibility with Go 1.6: + // If there's no TLSConfig, it's possible that the user just + // didn't set it on the http.Server, but did pass it to + // tls.NewListener and passed that listener to Serve. + // So we should configure HTTP/2 (to set up srv.TLSNextProto) + // in case the listener returns an "h2" *tls.Conn. + return true + } + // The user specified a TLSConfig on their http.Server. + // In this, case, only configure HTTP/2 if their tls.Config + // explicitly mentions "h2". Otherwise http2.ConfigureServer + // would modify the tls.Config to add it, but they probably already + // passed this tls.Config to tls.NewListener. And if they did, + // it's too late anyway to fix it. It would only be potentially racy. + // See Issue 15908. + return strSliceContains(srv.TLSConfig.NextProtos, http2NextProtoTLS) +} + // Serve accepts incoming connections on the Listener l, creating a // new service goroutine for each. The service goroutines read requests and // then call srv.Handler to reply to them. +// +// For HTTP/2 support, srv.TLSConfig should be initialized to the +// provided listener's TLS Config before calling Serve. If +// srv.TLSConfig is non-nil and doesn't include the string "h2" in +// Config.NextProtos, HTTP/2 support is not enabled. +// // Serve always returns a non-nil error. func (srv *Server) Serve(l net.Listener) error { defer l.Close() @@ -2232,9 +2260,13 @@ func (srv *Server) Serve(l net.Listener) error { fn(srv, l) } var tempDelay time.Duration // how long to sleep on accept failure - if err := srv.setupHTTP2(); err != nil { - return err + + if srv.shouldConfigureHTTP2ForServe() { + if err := srv.setupHTTP2(); err != nil { + return err + } } + // TODO: allow changing base context? can't imagine concrete // use cases yet. baseCtx := context.Background() -- cgit v1.3-6-g1900