From: Brad Fitzpatrick Date: Thu, 12 Apr 2018 17:20:18 +0000 (+0000) Subject: net/http: don't crash if Server.Server is called with non-comparable Listener X-Git-Tag: go1.11beta1~856 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=740a209a2ed416fa0be306dca5c84e55954e6924;p=gostls13.git net/http: don't crash if Server.Server is called with non-comparable Listener Fixes #24812 Change-Id: If8d496d61b1120233e44c72d854e80cb06bab970 Reviewed-on: https://go-review.googlesource.com/106657 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 49239b3671..ba8c8f030d 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -5843,6 +5843,19 @@ func TestServerValidatesMethod(t *testing.T) { } } +// Listener for TestServerListenNotComparableListener. +type eofListenerNotComparable []int + +func (eofListenerNotComparable) Accept() (net.Conn, error) { return nil, io.EOF } +func (eofListenerNotComparable) Addr() net.Addr { return nil } +func (eofListenerNotComparable) Close() error { return nil } + +// Issue 24812: don't crash on non-comparable Listener +func TestServerListenNotComparableListener(t *testing.T) { + var s Server + s.Serve(make(eofListenerNotComparable, 1)) // used to panic +} + func BenchmarkResponseStatusLine(b *testing.B) { b.ReportAllocs() b.RunParallel(func(pb *testing.PB) { diff --git a/src/net/http/server.go b/src/net/http/server.go index 114a2263c3..1ae7e2dd43 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -2485,7 +2485,7 @@ type Server struct { nextProtoErr error // result of http2.ConfigureServer if used mu sync.Mutex - listeners map[net.Listener]struct{} + listeners map[*net.Listener]struct{} activeConn map[*conn]struct{} doneChan chan struct{} onShutdown []func() @@ -2621,7 +2621,7 @@ func (s *Server) closeIdleConns() bool { func (s *Server) closeListenersLocked() error { var err error for ln := range s.listeners { - if cerr := ln.Close(); cerr != nil && err == nil { + if cerr := (*ln).Close(); cerr != nil && err == nil { err = cerr } delete(s.listeners, ln) @@ -2765,8 +2765,8 @@ func (srv *Server) Serve(l net.Listener) error { return err } - srv.trackListener(l, true) - defer srv.trackListener(l, false) + srv.trackListener(&l, true) + defer srv.trackListener(&l, false) baseCtx := context.Background() // base is always background, per Issue 16220 ctx := context.WithValue(baseCtx, ServerContextKey, srv) @@ -2843,11 +2843,19 @@ func (srv *Server) ServeTLS(l net.Listener, certFile, keyFile string) error { return srv.Serve(tlsListener) } -func (s *Server) trackListener(ln net.Listener, add bool) { +// trackListener adds or removes a net.Listener to the set of tracked +// listeners. +// +// We store a pointer to interface in the map set, in case the +// net.Listener is not comparable. This is safe because we only call +// trackListener via Serve and can track+defer untrack the same +// pointer to local variable there. We never need to compare a +// Listener from another caller. +func (s *Server) trackListener(ln *net.Listener, add bool) { s.mu.Lock() defer s.mu.Unlock() if s.listeners == nil { - s.listeners = make(map[net.Listener]struct{}) + s.listeners = make(map[*net.Listener]struct{}) } if add { // If the *Server is being reused after a previous