middleware/file: correctly parse the stanza (#658)

* middleware/file: correctly parse the stanza

Parsing the file stanza would give precedence to 'transfer' and ignore
other bits if it wasn't specified.

This change fixes the parsing. The actually external CNAME retrieval is
working fine (once the upstream is correctly parsed).

This wasn't caught in tests, because we lack a parsing test for this.

Fixes #657

* Add tests

middleware/file/setup.go

@@ -92,12 +92,17 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 
 			noReload := false
 			prxy := proxy.Proxy{}
+			t := []string{}
+			var e error
+
 			for c.NextBlock() {
-				t, _, e := TransferParse(c, false)
-				if e != nil {
-					return Zones{}, e
-				}
 				switch c.Val() {
+				case "transfer":
+					t, _, e = TransferParse(c, false)
+					if e != nil {
+						return Zones{}, e
+					}
+
 				case "no_reload":
 					noReload = true
 
@@ -128,40 +133,37 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 
 // TransferParse parses transfer statements: 'transfer to [address...]'.
 func TransferParse(c *caddy.Controller, secondary bool) (tos, froms []string, err error) {
-	what := c.Val()
 	if !c.NextArg() {
 		return nil, nil, c.ArgErr()
 	}
 	value := c.Val()
-	switch what {
+	switch value {
-	case "transfer":
+	case "to":
-		if value == "to" {
+		tos = c.RemainingArgs()
-			tos = c.RemainingArgs()
+		for i := range tos {
-			for i := range tos {
+			if tos[i] != "*" {
-				if tos[i] != "*" {
+				normalized, err := dnsutil.ParseHostPort(tos[i], "53")
-					normalized, err := dnsutil.ParseHostPort(tos[i], "53")
+				if err != nil {
-					if err != nil {
+					return nil, nil, err
-						return nil, nil, err
-					}
-					tos[i] = normalized
 				}
+				tos[i] = normalized
 			}
 		}
-		if value == "from" {
+
-			if !secondary {
+	case "from":
-				return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
+		if !secondary {
-			}
+			return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
-			froms = c.RemainingArgs()
+		}
-			for i := range froms {
+		froms = c.RemainingArgs()
-				if froms[i] != "*" {
+		for i := range froms {
-					normalized, err := dnsutil.ParseHostPort(froms[i], "53")
+			if froms[i] != "*" {
-					if err != nil {
+				normalized, err := dnsutil.ParseHostPort(froms[i], "53")
-						return nil, nil, err
+				if err != nil {
-					}
+					return nil, nil, err
-					froms[i] = normalized
-				} else {
-					return nil, nil, fmt.Errorf("can't use '*' in transfer from")
 				}
+				froms[i] = normalized
+			} else {
+				return nil, nil, fmt.Errorf("can't use '*' in transfer from")
 			}
 		}
 	}

middleware/secondary/setup.go

@@ -48,6 +48,7 @@ func secondaryParse(c *caddy.Controller) (file.Zones, error) {
 	names := []string{}
 	origins := []string{}
 	for c.Next() {
+
 		if c.Val() == "secondary" {
 			// secondary [origin]
 			origins = make([]string, len(c.ServerBlockKeys))
@@ -63,10 +64,18 @@ func secondaryParse(c *caddy.Controller) (file.Zones, error) {
 			}
 
 			for c.NextBlock() {
-				t, f, e := file.TransferParse(c, true)
+
-				if e != nil {
+				t, f := []string{}, []string{}
-					return file.Zones{}, e
+				var e error
+
+				switch c.Val() {
+				case "transfer":
+					t, _, e = file.TransferParse(c, true)
+					if e != nil {
+						return file.Zones{}, e
+					}
 				}
+
 				for _, origin := range origins {
 					if t != nil {
 						z[origin].TransferTo = append(z[origin].TransferTo, t...)

test/file_cname_proxy_test.go

@@ -12,7 +12,7 @@ import (
 	"github.com/miekg/dns"
 )
 
-func TestZoneExternalCNAMELookup(t *testing.T) {
+func TestZoneExternalCNAMELookupWithoutProxy(t *testing.T) {
 	t.Parallel()
 	log.SetOutput(ioutil.Discard)
 
@@ -50,3 +50,45 @@ func TestZoneExternalCNAMELookup(t *testing.T) {
 		t.Fatalf("Expected 1 RR in answer section got %d", len(resp.Answer))
 	}
 }
+
+func TestZoneExternalCNAMELookupWithProxy(t *testing.T) {
+	t.Parallel()
+	log.SetOutput(ioutil.Discard)
+
+	name, rm, err := TempFile(".", exampleOrg)
+	if err != nil {
+		t.Fatalf("Failed to create zone: %s", err)
+	}
+	defer rm()
+
+	// Corefile with for example without proxy section.
+	corefile := `example.org:0 {
+       file ` + name + ` {
+	       upstream 8.8.8.8
+	}
+}
+`
+	i, err := CoreDNSServer(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+
+	udp, _ := CoreDNSServerPorts(i, 0)
+	if udp == "" {
+		t.Fatalf("Could not get UDP listening port")
+	}
+	defer i.Stop()
+
+	p := proxy.NewLookup([]string{udp})
+	state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)}
+
+	resp, err := p.Lookup(state, "cname.example.org.", dns.TypeA)
+	if err != nil {
+		t.Fatalf("Expected to receive reply, but didn't: %s", err)
+	}
+	// There should be a CNAME *and* an IP address in the answer section.
+	// For now, just check that we have 2 RRs
+	if len(resp.Answer) != 2 {
+		t.Fatalf("Expected 2 RRs in answer section got %d", len(resp.Answer))
+	}
+}