net: fix parsing literal IP address in builtin dns stub resolver

This CL fixes a bug introduced by CL 128820043 which is that
builtin dns stub resolver doesn't work well with literal IPv6
address namesever entries in /etc/resolv.conf.

Also simplifies resolv.conf parser and adds more test cases.

LGTM=iant
R=golang-codereviews, bradfitz, iant
CC=golang-codereviews
https://golang.org/cl/140040043

src/pkg/net/dnsclient_unix_test.go

@@ -204,7 +204,7 @@ func TestReloadResolvConfChange(t *testing.T) {
 	if _, err := goLookupIP("golang.org"); err != nil {
 		t.Fatalf("goLookupIP(good) failed: %v", err)
 	}
-	r.WantServers([]string{"[8.8.8.8]"})
+	r.WantServers([]string{"8.8.8.8"})
 
 	// Using a bad resolv.conf when we had a good one
 	// before should not update the config
@@ -215,7 +215,7 @@ func TestReloadResolvConfChange(t *testing.T) {
 
 	// A new good config should get picked up
 	r.SetConf("nameserver 8.8.4.4")
-	r.WantServers([]string{"[8.8.4.4]"})
+	r.WantServers([]string{"8.8.4.4"})
 }
 
 func BenchmarkGoLookupIP(b *testing.B) {

src/pkg/net/dnsconfig_unix.go

@@ -25,13 +25,12 @@ func dnsReadConfig(filename string) (*dnsConfig, error) {
 	if err != nil {
 		return nil, &DNSConfigError{err}
 	}
-	conf := new(dnsConfig)
-	conf.servers = make([]string, 0, 3) // small, but the standard limit
-	conf.search = make([]string, 0)
-	conf.ndots = 1
-	conf.timeout = 5
-	conf.attempts = 2
-	conf.rotate = false
+	defer file.close()
+	conf := &dnsConfig{
+		ndots:    1,
+		timeout:  5,
+		attempts: 2,
+	}
 	for line, ok := file.readLine(); ok; line, ok = file.readLine() {
 		f := getFields(line)
 		if len(f) < 1 {
@@ -39,30 +38,18 @@ func dnsReadConfig(filename string) (*dnsConfig, error) {
 		}
 		switch f[0] {
 		case "nameserver": // add one name server
-			a := conf.servers
-			n := len(a)
-			if len(f) > 1 && n < cap(a) {
+			if len(f) > 1 && len(conf.servers) < 3 { // small, but the standard limit
 				// One more check: make sure server name is
 				// just an IP address.  Otherwise we need DNS
 				// to look it up.
-				name := f[1]
-				switch len(ParseIP(name)) {
-				case 16:
-					name = "[" + name + "]"
-					fallthrough
-				case 4:
-					a = a[0 : n+1]
-					a[n] = name
-					conf.servers = a
+				if ParseIP(f[1]) != nil {
+					conf.servers = append(conf.servers, f[1])
 				}
 			}
 
 		case "domain": // set search path to just this domain
 			if len(f) > 1 {
-				conf.search = make([]string, 1)
-				conf.search[0] = f[1]
-			} else {
-				conf.search = make([]string, 0)
+				conf.search = []string{f[1]}
 			}
 
 		case "search": // set search path to given servers
@@ -99,8 +86,6 @@ func dnsReadConfig(filename string) (*dnsConfig, error) {
 			}
 		}
 	}
-	file.close()
-
 	return conf, nil
 }
 

src/pkg/net/dnsconfig_unix_test.go

@@ -6,41 +6,64 @@
 
 package net
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+)
 
-func TestDNSReadConfig(t *testing.T) {
-	dnsConfig, err := dnsReadConfig("testdata/resolv.conf")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if len(dnsConfig.servers) != 1 {
-		t.Errorf("len(dnsConfig.servers) = %d; want %d", len(dnsConfig.servers), 1)
-	}
-	if dnsConfig.servers[0] != "[192.168.1.1]" {
-		t.Errorf("dnsConfig.servers[0] = %s; want %s", dnsConfig.servers[0], "[192.168.1.1]")
-	}
-
-	if len(dnsConfig.search) != 1 {
-		t.Errorf("len(dnsConfig.search) = %d; want %d", len(dnsConfig.search), 1)
-	}
-	if dnsConfig.search[0] != "Home" {
-		t.Errorf("dnsConfig.search[0] = %s; want %s", dnsConfig.search[0], "Home")
-	}
-
-	if dnsConfig.ndots != 5 {
-		t.Errorf("dnsConfig.ndots = %d; want %d", dnsConfig.ndots, 5)
-	}
-
-	if dnsConfig.timeout != 10 {
-		t.Errorf("dnsConfig.timeout = %d; want %d", dnsConfig.timeout, 10)
-	}
-
-	if dnsConfig.attempts != 3 {
-		t.Errorf("dnsConfig.attempts = %d; want %d", dnsConfig.attempts, 3)
-	}
+var dnsReadConfigTests = []struct {
+	name string
+	conf dnsConfig
+}{
+	{
+		name: "testdata/resolv.conf",
+		conf: dnsConfig{
+			servers:  []string{"8.8.8.8", "2001:4860:4860::8888"},
+			search:   []string{"localdomain"},
+			ndots:    5,
+			timeout:  10,
+			attempts: 3,
+			rotate:   true,
+		},
+	},
+	{
+		name: "testdata/domain-resolv.conf",
+		conf: dnsConfig{
+			servers:  []string{"8.8.8.8"},
+			search:   []string{"localdomain"},
+			ndots:    1,
+			timeout:  5,
+			attempts: 2,
+		},
+	},
+	{
+		name: "testdata/search-resolv.conf",
+		conf: dnsConfig{
+			servers:  []string{"8.8.8.8"},
+			search:   []string{"test", "invalid"},
+			ndots:    1,
+			timeout:  5,
+			attempts: 2,
+		},
+	},
+	{
+		name: "testdata/empty-resolv.conf",
+		conf: dnsConfig{
+			ndots:    1,
+			timeout:  5,
+			attempts: 2,
+		},
+	},
+}
 
-	if dnsConfig.rotate != true {
-		t.Errorf("dnsConfig.rotate = %t; want %t", dnsConfig.rotate, true)
+func TestDNSReadConfig(t *testing.T) {
+	for _, tt := range dnsReadConfigTests {
+		conf, err := dnsReadConfig(tt.name)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !reflect.DeepEqual(conf, &tt.conf) {
+			t.Errorf("got %v; want %v", conf, &tt.conf)
+		}
 	}
 }

src/pkg/net/testdata/domain-resolv.conf

+# /etc/resolv.conf
+
+search test invalid
+domain localdomain
+nameserver 8.8.8.8

src/pkg/net/testdata/empty-resolv.conf

+# /etc/resolv.conf

src/pkg/net/testdata/resolv.conf

 # /etc/resolv.conf
 
-domain Home
-nameserver 192.168.1.1
+domain localdomain
+nameserver 8.8.8.8
+nameserver 2001:4860:4860::8888
 options ndots:5 timeout:10 attempts:3 rotate
 options attempts 3

src/pkg/net/testdata/search-resolv.conf

+# /etc/resolv.conf
+
+domain localdomain
+search test invalid
+nameserver 8.8.8.8