Fix zone parser error handling

coredns · May 13, 2024 · 6983186 · 6983186
1 parent e196de3
commit 6983186
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 13 deletions.
diff --git a/plugin/file/file.go b/plugin/file/file.go
@@ -136,10 +136,6 @@ func Parse(f io.Reader, origin, fileName string, serial int64) (*Zone, error) {
 	z := NewZone(origin, fileName)
 	seenSOA := false
 	for rr, ok := zp.Next(); ok; rr, ok = zp.Next() {
-		if err := zp.Err(); err != nil {
-			return nil, err
-		}
-
 		if !seenSOA {
 			if s, ok := rr.(*dns.SOA); ok {
 				seenSOA = true
@@ -160,5 +156,9 @@ func Parse(f io.Reader, origin, fileName string, serial int64) (*Zone, error) {
 		return nil, fmt.Errorf("file %q has no SOA record for origin %s", fileName, origin)
 	}
 
+	if err := zp.Err(); err != nil {
+		return nil, err
+	}
+
 	return z, nil
 }
diff --git a/plugin/file/file_test.go b/plugin/file/file_test.go
@@ -29,3 +29,31 @@ www          IN  A      192.168.0.14
 mail         IN  A      192.168.0.15
 imap         IN  CNAME  mail
 `
+
+func TestParseSyntaxError(t *testing.T) {
+	_, err := Parse(strings.NewReader(dbSyntaxError), "example.org.", "stdin", 0)
+	if err == nil {
+		t.Fatalf("Zone %q should have failed to load", "example.org.")
+	}
+	if !strings.Contains(err.Error(), "\"invalid\"") {
+		t.Fatalf("Zone %q should have failed with syntax error: %s", "example.org.", err)
+	}
+}
+
+const dbSyntaxError = `
+$TTL         1M
+$ORIGIN      example.org.
+
+@            IN  SOA    ns1.example.com. admin.example.com.  (
+                               2005011437 ; Serial
+                               1200       ; Refresh
+                               144        ; Retry
+                               1814400    ; Expire
+                               2h )       ; Minimum
+@            IN  NS     ns1.example.com.
+
+# invalid comment
+www          IN  A      192.168.0.14
+mail         IN  A      192.168.0.15
+imap         IN  CNAME  mail
+`
diff --git a/plugin/sign/file.go b/plugin/sign/file.go
@@ -66,10 +66,6 @@ func Parse(f io.Reader, origin, fileName string) (*file.Zone, error) {
 	seenSOA := false
 
 	for rr, ok := zp.Next(); ok; rr, ok = zp.Next() {
-		if err := zp.Err(); err != nil {
-			return nil, err
-		}
-
 		switch rr.(type) {
 		case *dns.DNSKEY, *dns.RRSIG, *dns.CDNSKEY, *dns.CDS:
 			continue
@@ -88,5 +84,9 @@ func Parse(f io.Reader, origin, fileName string) (*file.Zone, error) {
 		return nil, fmt.Errorf("file %q has no SOA record", fileName)
 	}
 
+	if err := zp.Err(); err != nil {
+		return nil, err
+	}
+
 	return z, nil
 }
diff --git a/plugin/sign/signer.go b/plugin/sign/signer.go
@@ -133,10 +133,6 @@ func resign(rd io.Reader, now time.Time) (why error) {
 	i := 0
 
 	for rr, ok := zp.Next(); ok; rr, ok = zp.Next() {
-		if err := zp.Err(); err != nil {
-			return err
-		}
-
 		switch x := rr.(type) {
 		case *dns.RRSIG:
 			if x.TypeCovered != dns.TypeSOA {
@@ -166,7 +162,7 @@ func resign(rd io.Reader, now time.Time) (why error) {
 		}
 	}
 
-	return nil
+	return zp.Err()
 }
 
 func signAndLog(s *Signer, why error) {