Skip to content

Commit

Permalink
test: fix TestAgent.Start() to not segfault if the DNSServer cannot L…
Browse files Browse the repository at this point in the history
…istenAndServe (hashicorp#6409)

The embedded `Server` field on a `DNSServer` is only set inside of the
`ListenAndServe` method. If that method fails for reasons like the
address being in use and is not bindable, then the `Server` field will
not be set and the overall `Agent.Start()` will fail.

This will trigger the inner loop of `TestAgent.Start()` to invoke
`ShutdownEndpoints` which will attempt to pretty print the DNS servers
using fields on that inner `Server` field. Because it was never set,
this causes a nil pointer dereference and crashes the test.
  • Loading branch information
rboyer authored Aug 27, 2019
1 parent 0be1531 commit 91da908
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
6 changes: 4 additions & 2 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1654,8 +1654,10 @@ func (a *Agent) ShutdownEndpoints() {
}

for _, srv := range a.dnsServers {
a.logger.Printf("[INFO] agent: Stopping DNS server %s (%s)", srv.Server.Addr, srv.Server.Net)
srv.Shutdown()
if srv.Server != nil {
a.logger.Printf("[INFO] agent: Stopping DNS server %s (%s)", srv.Server.Addr, srv.Server.Net)
srv.Shutdown()
}
}
a.dnsServers = nil

Expand Down
2 changes: 1 addition & 1 deletion agent/testagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (a *TestAgent) Start(t *testing.T) *TestAgent {
a.Agent = agent
break
} else if i == 0 {
require.Fail("%s %s Error starting agent: %s", id, a.Name, err)
require.Failf("%s %s Error starting agent: %s", id, a.Name, err)
} else if a.ExpectConfigError {
// Panic the error since this can be caught if needed. Pretty gross way to
// detect errors but enough for now and this is a tiny edge case that I'd
Expand Down

0 comments on commit 91da908

Please sign in to comment.