From 0c5409d1723b853540064945fb9997f37b1b7944 Mon Sep 17 00:00:00 2001 From: "R.B. Boyer" Date: Tue, 27 Aug 2019 10:45:05 -0500 Subject: [PATCH] test: fix TestAgent.Start() to not segfault if the DNSServer cannot ListenAndServe (#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. --- agent/agent.go | 6 ++++-- agent/testagent.go | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index c3427c3c0..063b120eb 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -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 diff --git a/agent/testagent.go b/agent/testagent.go index 648d91957..fc9c15a6c 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -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