net/http/httptest: don't panic on Close of user-constructed Server value

If the user created an httptest.Server directly without using a
constructor it won't have the new unexported 'client' field. So don't
assume it's non-nil.

Fixes #19729

Change-Id: Ie92e5da66cf4e7fb8d95f3ad0f4e3987d3ae8b77
Reviewed-on: https://go-review.googlesource.com/38710
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Kevin Burke <kev@inburke.com>

diff --git a/src/net/http/httptest/server.go b/src/net/http/httptest/server.go
index b5b18c747d9b2a1597e9979b73af7fe21a0e9fc4..549ef0462389c93afcd3cd2eca3bdc8b11cce00a 100644 (file)
--- a/src/net/http/httptest/server.go
+++ b/src/net/http/httptest/server.go
@@ -209,8 +209,10 @@ func (s *Server) Close() {
        }
 
        // Also close the client idle connections.
-       if t, ok := s.client.Transport.(closeIdleTransport); ok {
-               t.CloseIdleConnections()
+       if s.client != nil {
+               if t, ok := s.client.Transport.(closeIdleTransport); ok {
+                       t.CloseIdleConnections()
+               }
        }
 
        s.wg.Wait()

diff --git a/src/net/http/httptest/server_test.go b/src/net/http/httptest/server_test.go
index 62846de02cd582a329b267c9655bc5a293be6f1f..d97cec5fdd5329fd6bfb32e0ec7f49887db5f0fc 100644 (file)
--- a/src/net/http/httptest/server_test.go
+++ b/src/net/http/httptest/server_test.go
@@ -145,3 +145,20 @@ func TestTLSServerClientTransportType(t *testing.T) {
                t.Errorf("got %T, want *http.Transport", client.Transport)
        }
 }
+
+type onlyCloseListener struct {
+       net.Listener
+}
+
+func (onlyCloseListener) Close() error { return nil }
+
+// Issue 19729: panic in Server.Close for values created directly
+// without a constructor (so the unexported client field is nil).
+func TestServerZeroValueClose(t *testing.T) {
+       ts := &Server{
+               Listener: onlyCloseListener{},
+               Config:   &http.Server{},
+       }
+
+       ts.Close() // tests that it doesn't panic
+}