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

3 files changed, 87 insertions, 34 deletions

diff --git a/middleware/file/setup.go b/middleware/file/setup.go
index c48e4c089..9cbcf2c21 100644
--- a/middleware/file/setup.go
+++ b/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 {
-	case "transfer":
-		if value == "to" {
-			tos = c.RemainingArgs()
-			for i := range tos {
-				if tos[i] != "*" {
-					normalized, err := dnsutil.ParseHostPort(tos[i], "53")
-					if err != nil {
-						return nil, nil, err
-					}
-					tos[i] = normalized
+	switch value {
+	case "to":
+		tos = c.RemainingArgs()
+		for i := range tos {
+			if tos[i] != "*" {
+				normalized, err := dnsutil.ParseHostPort(tos[i], "53")
+				if err != nil {
+					return nil, nil, err
 				}
+				tos[i] = normalized
 			}
 		}
-		if value == "from" {
-			if !secondary {
-				return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
-			}
-			froms = c.RemainingArgs()
-			for i := range froms {
-				if froms[i] != "*" {
-					normalized, err := dnsutil.ParseHostPort(froms[i], "53")
-					if err != nil {
-						return nil, nil, err
-					}
-					froms[i] = normalized
-				} else {
-					return nil, nil, fmt.Errorf("can't use '*' in transfer from")
+
+	case "from":
+		if !secondary {
+			return nil, nil, fmt.Errorf("can't use `transfer from` when not being a secondary")
+		}
+		froms = c.RemainingArgs()
+		for i := range froms {
+			if froms[i] != "*" {
+				normalized, err := dnsutil.ParseHostPort(froms[i], "53")
+				if err != nil {
+					return nil, nil, err
 				}
+				froms[i] = normalized
+			} else {
+				return nil, nil, fmt.Errorf("can't use '*' in transfer from")
 			}
 		}
 	}
diff --git a/middleware/secondary/setup.go b/middleware/secondary/setup.go
index 5822f9e8f..ca665e79b 100644
--- a/middleware/secondary/setup.go
+++ b/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 {
-					return file.Zones{}, e
+
+				t, f := []string{}, []string{}
+				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...)
diff --git a/test/file_cname_proxy_test.go b/test/file_cname_proxy_test.go
index 61bd8764e..a6ab8e59e 100644
--- a/test/file_cname_proxy_test.go
+++ b/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))
+	}
+}