net: actually re-check resolv.conf when no explicit nameservers detected

Follow-up of CL 739620. I think it is wrong, since there is no
way dc.servers == 0, since we always set it to defaultNS.

Change-Id: Ibb76bfeb2b7974301d3515d90517d7766a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/741140
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Mark Freeman <markfreeman@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

3 files changed, 51 insertions, 4 deletions

diff --git a/src/net/dnsclient_unix.go b/src/net/dnsclient_unix.go
index fc671053f5..53c98f7874 100644
--- a/src/net/dnsclient_unix.go
+++ b/src/net/dnsclient_unix.go
@@ -366,7 +366,11 @@ type resolverConfig struct {
 var resolvConf resolverConfig
 
 func getSystemDNSConfig() *dnsConfig {
-	resolvConf.tryUpdate("/etc/resolv.conf")
+	return getSystemDNSConfigNamed("/etc/resolv.conf")
+}
+
+func getSystemDNSConfigNamed(path string) *dnsConfig {
+	resolvConf.tryUpdate(path)
 	return resolvConf.dnsConfig.Load()
 }
 
@@ -414,8 +418,19 @@ func (conf *resolverConfig) tryUpdate(name string) {
 	defer conf.releaseSema()
 
 	now := time.Now()
-	if (len(dc.servers) > 0 && conf.lastChecked.After(now.Add(-5*time.Second))) ||
-		conf.lastChecked == distantFuture {
+
+	// Only recheck the resolv.conf when:
+	// - expired (last re-check was more that 5 seconds ago)
+	// - the default nameservers are used (the last parse could not load
+	//   resolv.conf, or it did not contain any nameservers)
+	// - rechecks are not disabled (only possible in case of testing)
+	//
+	// Note: We only do one check at a time. Other concurrent requests might
+	// still use the previous (outdated) version of the configuration file.
+	expired := now.After(conf.lastChecked.Add(5 * time.Second))
+	rechecksEnabled := conf.lastChecked != distantFuture // for testing purposes
+	recheck := (expired || dc.isDefaultNS()) && rechecksEnabled
+	if !recheck {
 		return
 	}
 	conf.lastChecked = now
diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go
index fc1d40f18b..fc2eeb6acb 100644
--- a/src/net/dnsclient_unix_test.go
+++ b/src/net/dnsclient_unix_test.go
@@ -331,7 +331,7 @@ func (conf *resolvConfTest) write(lines []string) error {
 }
 
 func (conf *resolvConfTest) writeAndUpdate(lines []string) error {
-	return conf.writeAndUpdateWithLastCheckedTime(lines, time.Now().Add(time.Hour))
+	return conf.writeAndUpdateWithLastCheckedTime(lines, distantFuture)
 }
 
 func (conf *resolvConfTest) writeAndUpdateWithLastCheckedTime(lines []string, lastChecked time.Time) error {
@@ -2855,3 +2855,29 @@ func TestExtendedRCode(t *testing.T) {
 		t.Fatalf("r.tryOneName(): unexpected error: %v", err)
 	}
 }
+
+// This test makes sure that we always re-check the resolv.conf no matter
+// the elapsed time in case the default nameservers are used.
+func TestEmptyResolvConfReplacedWithConfHaingNameservers(t *testing.T) {
+	conf, err := newResolvConfTest()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conf.teardown()
+
+	if err := conf.writeAndUpdateWithLastCheckedTime([]string{"# empty resolv.conf file"}, time.Now()); err != nil {
+		t.Fatal(err)
+	}
+
+	if !getSystemDNSConfigNamed(conf.path).isDefaultNS() {
+		t.Fatal("resolv.conf was not re-loaded")
+	}
+
+	if err := conf.writeAndUpdateWithLastCheckedTime([]string{"nameserver 192.0.2.1"}, time.Now()); err != nil {
+		t.Fatal(err)
+	}
+
+	if getSystemDNSConfigNamed(conf.path).isDefaultNS() {
+		t.Fatal("resolv.conf was not re-loaded")
+	}
+}
diff --git a/src/net/dnsconfig.go b/src/net/dnsconfig.go
index 5a156ea0e4..647509b028 100644
--- a/src/net/dnsconfig.go
+++ b/src/net/dnsconfig.go
@@ -25,6 +25,12 @@ import (
 //go:linkname defaultNS
 var defaultNS = []string{"127.0.0.1:53", "[::1]:53"}
 
+// isDefaultNS reports whether the c.servers field is set to [defaultNS], meaning that
+// no nameservers were specified in /etc/resolv.conf, thus the default ones are used.
+func (c *dnsConfig) isDefaultNS() bool {
+	return len(c.servers) == len(defaultNS) && &c.servers[0] == &defaultNS[0]
+}
+
 var getHostname = os.Hostname // variable for testing
 
 type dnsConfig struct {