Skip to content

Commit

Permalink
net/http/httptest: don't panic on Close of user-constructed Server value
Browse files Browse the repository at this point in the history
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 golang#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>
  • Loading branch information
bradfitz authored and lparth committed Apr 13, 2017
1 parent 3e37806 commit 08ce1a2
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/net/http/httptest/server.go
Expand Up @@ -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()
Expand Down
17 changes: 17 additions & 0 deletions src/net/http/httptest/server_test.go
Expand Up @@ -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
}

0 comments on commit 08ce1a2

Please sign in to comment.