Repository: miekg/dns (go)

Issue:
RFC3597
Hi,
I was checking your library for parsing a zone with generic representation and it seems that or I'm doing something wrong, or something is broken.  If we have this code (called test-parser.go) that reads a zone file and print the RRs:

```
package main
import (
	"fmt"
	"os"
	"github.com/miekg/dns"
	)
func main () {

	zonefile := os.Args[1]
	fd, _ := os.Open(zonefile)
	defer fd.Close()

	zone := dns.NewZoneParser(fd,"","")

	for rr, ok := zone.Next(); ok; rr, ok = zone.Next() {
		fmt.Println(rr)
	}
}
```

and having this simple.zone file: 
```
example.        86400   IN      NS      ns.example.
example.        86400   IN      SOA     ns.example. admin.example. 2018031900 18
00 900 604800 86400
ns.example.     3600    IN      A       127.0.0.1
example.     3600    IN      A       127.0.0.2
```

It is printed correctly with `$ go run test-parser.go simple.zone`

But, if I change the last line for the Hex representation using:
`perl -MNet::DNS::RR -e '$rr = new Net::DNS::RR("example. 3600 IN A 127.0.0.2");print $rr->generic;'`

Which gives me the simple.zone file:
```
example.        86400   IN      NS      ns.example.
example.        86400   IN      SOA     ns.example. admin.example. 2018031900 18
00 900 604800 86400
ns.example.     3600    IN      A       127.0.0.1
example. 3600 CLASS1 TYPE1 \# 4 7f000002
```
Then, running the code i will have:
```
$ go run test-parser.go simple.zone 
example.	86400	IN	NS	ns.example.
example.	86400	IN	SOA	ns.example. admin.example. 2018031900 1800 900 604800 86400
ns.example.	3600	IN	A	127.0.0.1

```


Pull request:
Support parsing known RR types in RFC 3597 format
RFC 3597 says we should allow parsing known record types in the "Unknown DNS Resource Record Type" format. This makes sense, zone files shouldn't become invalid simply because we learn a new concrete RR type or because other software doesn't recognise an RR type we support. The RFC also mentions instances where it can be preferable to use this format even in widely known RR types.

The concrete RR type will be returned for known record types. This is true even of ANY, NULL, OPT and TSIG records which would previously return a `*RFC3597` record.

Fixes #1209

/cc @madestro

Patch:
diff --git a/dns.go b/dns.go
index ad83a27e..33c93b2b 100644
--- a/dns.go
+++ b/dns.go
@@ -1,6 +1,9 @@
 package dns
 
-import "strconv"
+import (
+	"encoding/hex"
+	"strconv"
+)
 
 const (
 	year68     = 1 << 31 // For RFC1982 (Serial Arithmetic) calculations in 32 bits.
@@ -111,7 +114,7 @@ func (h *RR_Header) parse(c *zlexer, origin string) *ParseError {
 
 // ToRFC3597 converts a known RR to the unknown RR representation from RFC 3597.
 func (rr *RFC3597) ToRFC3597(r RR) error {
-	buf := make([]byte, Len(r)*2)
+	buf := make([]byte, Len(r))
 	headerEnd, off, err := packRR(r, buf, 0, compressionMap{}, false)
 	if err != nil {
 		return err
@@ -126,9 +129,25 @@ func (rr *RFC3597) ToRFC3597(r RR) error {
 	}
 
 	_, err = rr.unpack(buf, headerEnd)
+	return err
+}
+
+// fromRFC3597 converts an unknown RR representation from RFC 3597 to the known RR type.
+func (rr *RFC3597) fromRFC3597(r RR) error {
+	*r.Header() = rr.Hdr
+
+	if len(rr.Rdata) == 0 {
+		// Dynamic update.
+		return nil
+	}
+
+	// rr.pack requires an extra allocation and a copy so we just decode Rdata
+	// manually, it's simpler anyway.
+	msg, err := hex.DecodeString(rr.Rdata)
 	if err != nil {
 		return err
 	}
 
-	return nil
+	_, err = r.unpack(msg, 0)
+	return err
 }
diff --git a/edns.go b/edns.go
index 04808d57..f3fb1c68 100644
--- a/edns.go
+++ b/edns.go
@@ -88,8 +88,8 @@ func (rr *OPT) len(off int, compression map[string]struct{}) int {
 	return l
 }
 
-func (rr *OPT) parse(c *zlexer, origin string) *ParseError {
-	panic("dns: internal error: parse should never be called on OPT")
+func (*OPT) parse(c *zlexer, origin string) *ParseError {
+	return &ParseError{err: "OPT records do not have a presentation format"}
 }
 
 func (r1 *OPT) isDuplicate(r2 RR) bool { return false }
diff --git a/generate.go b/generate.go
index f713074a..ac8df34d 100644
--- a/generate.go
+++ b/generate.go
@@ -75,10 +75,10 @@ func (zp *ZoneParser) generate(l lex) (RR, bool) {
 	r := &generateReader{
 		s: s,
 
-		cur:   int(start),
-		start: int(start),
-		end:   int(end),
-		step:  int(step),
+		cur:   start,
+		start: start,
+		end:   end,
+		step:  step,
 
 		file: zp.file,
 		lex:  &l,
@@ -94,10 +94,10 @@ type generateReader struct {
 	s  string
 	si int
 
-	cur   int
-	start int
-	end   int
-	step  int
+	cur   int64
+	start int64
+	end   int64
+	step  int64
 
 	mod bytes.Buffer
 
@@ -173,7 +173,7 @@ func (r *generateReader) ReadByte() (byte, error) {
 			return '$', nil
 		}
 
-		var offset int
+		var offset int64
 
 		// Search for { and }
 		if r.s[si+1] == '{' {
@@ -188,7 +188,7 @@ func (r *generateReader) ReadByte() (byte, error) {
 			if errMsg != "" {
 				return 0, r.parseError(errMsg, si+3+sep)
 			}
-			if r.start+offset < 0 || int64(r.end) + int64(offset) > 1<<31-1 {
+			if r.start+offset < 0 || r.end+offset > 1<<31-1 {
 				return 0, r.parseError("bad offset in $GENERATE", si+3+sep)
 			}
 
@@ -208,7 +208,7 @@ func (r *generateReader) ReadByte() (byte, error) {
 }
 
 // Convert a $GENERATE modifier 0,0,d to something Printf can deal with.
-func modToPrintf(s string) (string, int, string) {
+func modToPrintf(s string) (string, int64, string) {
 	// Modifier is { offset [ ,width [ ,base ] ] } - provide default
 	// values for optional width and type, if necessary.
 	var offStr, widthStr, base string
@@ -240,8 +240,8 @@ func modToPrintf(s string) (string, int, string) {
 	}
 
 	if width == 0 {
-		return "%" + base, int(offset), ""
+		return "%" + base, offset, ""
 	}
 
-	return "%0" + widthStr + base, int(offset), ""
+	return "%0" + widthStr + base, offset, ""
 }
diff --git a/scan.go b/scan.go
index aa2840ef..67161de2 100644
--- a/scan.go
+++ b/scan.go
@@ -577,10 +577,23 @@ func (zp *ZoneParser) Next() (RR, bool) {
 
 			st = zExpectRdata
 		case zExpectRdata:
-			var rr RR
-			if newFn, ok := TypeToRR[h.Rrtype]; ok && canParseAsRR(h.Rrtype) {
+			var (
+				rr             RR
+				parseAsRFC3597 bool
+			)
+			if newFn, ok := TypeToRR[h.Rrtype]; ok {
 				rr = newFn()
 				*rr.Header() = *h
+
+				// We may be parsing a known RR type using the RFC3597 format.
+				// If so, we handle that here in a generic way.
+				//
+				// This is also true for PrivateRR types which will have the
+				// RFC3597 parsing done for them and the Unpack method called
+				// to populate the RR instead of simply deferring to Parse.
+				if zp.c.Peek().token == "\\#" {
+					parseAsRFC3597 = true
+				}
 			} else {
 				rr = &RFC3597{Hdr: *h}
 			}
@@ -600,13 +613,18 @@ func (zp *ZoneParser) Next() (RR, bool) {
 				return zp.setParseError("unexpected newline", l)
 			}
 
-			if err := rr.parse(zp.c, zp.origin); err != nil {
+			parseAsRR := rr
+			if parseAsRFC3597 {
+				parseAsRR = &RFC3597{Hdr: *h}
+			}
+
+			if err := parseAsRR.parse(zp.c, zp.origin); err != nil {
 				// err is a concrete *ParseError without the file field set.
 				// The setParseError call below will construct a new
 				// *ParseError with file set to zp.file.
 
-				// If err.lex is nil than we have encounter an unknown RR type
-				// in that case we substitute our current lex token.
+				// err.lex may be nil in which case we substitute our current
+				// lex token.
 				if err.lex == (lex{}) {
 					return zp.setParseError(err.err, l)
 				}
@@ -614,6 +632,13 @@ func (zp *ZoneParser) Next() (RR, bool) {
 				return zp.setParseError(err.err, err.lex)
 			}
 
+			if parseAsRFC3597 {
+				err := parseAsRR.(*RFC3597).fromRFC3597(rr)
+				if err != nil {
+					return zp.setParseError(err.Error(), l)
+				}
+			}
+
 			return rr, true
 		}
 	}
@@ -623,18 +648,6 @@ func (zp *ZoneParser) Next() (RR, bool) {
 	return nil, false
 }
 
-// canParseAsRR returns true if the record type can be parsed as a
-// concrete RR. It blacklists certain record types that must be parsed
-// according to RFC 3597 because they lack a presentation format.
-func canParseAsRR(rrtype uint16) bool {
-	switch rrtype {
-	case TypeANY, TypeNULL, TypeOPT, TypeTSIG:
-		return false
-	default:
-		return true
-	}
-}
-
 type zlexer struct {
 	br io.ByteReader
 
@@ -1290,7 +1303,7 @@ func appendOrigin(name, origin string) string {
 
 // LOC record helper function
 func locCheckNorth(token string, latitude uint32) (uint32, bool) {
-	if latitude > 90 * 1000 * 60 * 60 {
+	if latitude > 90*1000*60*60 {
 		return latitude, false
 	}
 	switch token {
@@ -1304,7 +1317,7 @@ func locCheckNorth(token string, latitude uint32) (uint32, bool) {
 
 // LOC record helper function
 func locCheckEast(token string, longitude uint32) (uint32, bool) {
-	if longitude > 180 * 1000 * 60 * 60 {
+	if longitude > 180*1000*60*60 {
 		return longitude, false
 	}
 	switch token {
diff --git a/tsig.go b/tsig.go
index 5b52e1c4..b49562d8 100644
--- a/tsig.go
+++ b/tsig.go
@@ -106,8 +106,8 @@ func (rr *TSIG) String() string {
 	return s
 }
 
-func (rr *TSIG) parse(c *zlexer, origin string) *ParseError {
-	panic("dns: internal error: parse should never be called on TSIG")
+func (*TSIG) parse(c *zlexer, origin string) *ParseError {
+	return &ParseError{err: "TSIG records do not have a presentation format"}
 }
 
 // The following values must be put in wireformat, so that the MAC can be calculated.
diff --git a/types.go b/types.go
index 1f385bd2..9e379eb3 100644
--- a/types.go
+++ b/types.go
@@ -245,8 +245,8 @@ type ANY struct {
 
 func (rr *ANY) String() string { return rr.Hdr.String() }
 
-func (rr *ANY) parse(c *zlexer, origin string) *ParseError {
-	panic("dns: internal error: parse should never be called on ANY")
+func (*ANY) parse(c *zlexer, origin string) *ParseError {
+	return &ParseError{err: "ANY records do not have a presentation format"}
 }
 
 // NULL RR. See RFC 1035.
@@ -260,8 +260,8 @@ func (rr *NULL) String() string {
 	return ";" + rr.Hdr.String() + rr.Data
 }
 
-func (rr *NULL) parse(c *zlexer, origin string) *ParseError {
-	panic("dns: internal error: parse should never be called on NULL")
+func (*NULL) parse(c *zlexer, origin string) *ParseError {
+	return &ParseError{err: "NULL records do not have a presentation format"}
 }
 
 // CNAME RR. See RFC 1034.


Test patch:
diff --git a/generate_test.go b/generate_test.go
index cd161c1f..f0feb86a 100644
--- a/generate_test.go
+++ b/generate_test.go
@@ -162,7 +162,7 @@ func TestGenerateModToPrintf(t *testing.T) {
 	tests := []struct {
 		mod        string
 		wantFmt    string
-		wantOffset int
+		wantOffset int64
 		wantErr    bool
 	}{
 		{"0,0,d", "%d", 0, false},
diff --git a/parse_test.go b/parse_test.go
index d839562a..886f46cc 100644
--- a/parse_test.go
+++ b/parse_test.go
@@ -1208,8 +1208,8 @@ func TestTypeXXXX(t *testing.T) {
 		t.Errorf("this should not work, for TYPE655341")
 	}
 	_, err = NewRR("example.com IN TYPE1 \\# 4 0a000001")
-	if err == nil {
-		t.Errorf("this should not work")
+	if err != nil {
+		t.Errorf("failed to parse TYPE1 RR: %v", err)
 	}
 }
 
diff --git a/scan_test.go b/scan_test.go
index 578deba2..08424e09 100644
--- a/scan_test.go
+++ b/scan_test.go
@@ -145,10 +145,10 @@ func TestZoneParserAddressAAAA(t *testing.T) {
 		}
 		aaaa, ok := got.(*AAAA)
 		if !ok {
-			t.Fatalf("expected *AAAA RR, but got %T", aaaa)
+			t.Fatalf("expected *AAAA RR, but got %T", got)
 		}
-		if g, w := aaaa.AAAA, tc.want.AAAA; !g.Equal(w) {
-			t.Fatalf("expected AAAA with IP %v, but got %v", g, w)
+		if !aaaa.AAAA.Equal(tc.want.AAAA) {
+			t.Fatalf("expected AAAA with IP %v, but got %v", tc.want.AAAA, aaaa.AAAA)
 		}
 	}
 }
@@ -229,6 +229,48 @@ example.com. 60 PX (
 	}
 }
 
+func TestParseKnownRRAsRFC3597(t *testing.T) {
+	t.Run("with RDATA", func(t *testing.T) {
+		rr, err := NewRR("example. 3600 CLASS1 TYPE1 \\# 4 7f000001")
+		if err != nil {
+			t.Fatalf("failed to parse RFC3579 format: %v", err)
+		}
+
+		if rr.Header().Rrtype != TypeA {
+			t.Errorf("expected TypeA (1) Rrtype, but got %v", rr.Header().Rrtype)
+		}
+
+		a, ok := rr.(*A)
+		if !ok {
+			t.Fatalf("expected *A RR, but got %T", rr)
+		}
+
+		localhost := net.IPv4(127, 0, 0, 1)
+		if !a.A.Equal(localhost) {
+			t.Fatalf("expected A with IP %v, but got %v", localhost, a.A)
+		}
+	})
+	t.Run("without RDATA", func(t *testing.T) {
+		rr, err := NewRR("example. 3600 CLASS1 TYPE1 \\# 0")
+		if err != nil {
+			t.Fatalf("failed to parse RFC3579 format: %v", err)
+		}
+
+		if rr.Header().Rrtype != TypeA {
+			t.Errorf("expected TypeA (1) Rrtype, but got %v", rr.Header().Rrtype)
+		}
+
+		a, ok := rr.(*A)
+		if !ok {
+			t.Fatalf("expected *A RR, but got %T", rr)
+		}
+
+		if len(a.A) != 0 {
+			t.Fatalf("expected A with empty IP, but got %v", a.A)
+		}
+	})
+}
+
 func BenchmarkNewRR(b *testing.B) {
 	const name1 = "12345678901234567890123456789012345.12345678.123."
 	const s = name1 + " 3600 IN MX 10 " + name1
diff --git a/update_test.go b/update_test.go
index 5dba413a..380376c9 100644
--- a/update_test.go
+++ b/update_test.go
@@ -6,15 +6,28 @@ import (
 )
 
 func TestDynamicUpdateParsing(t *testing.T) {
-	prefix := "example.com. IN "
-	for _, typ := range TypeToString {
-		if typ == "OPT" || typ == "AXFR" || typ == "IXFR" || typ == "ANY" || typ == "TKEY" ||
-			typ == "TSIG" || typ == "ISDN" || typ == "UNSPEC" || typ == "NULL" || typ == "ATMA" ||
-			typ == "Reserved" || typ == "None" || typ == "NXT" || typ == "MAILB" || typ == "MAILA" {
+	const prefix = "example.com. IN "
+
+	for typ, name := range TypeToString {
+		switch typ {
+		case TypeNone, TypeReserved:
+			continue
+		case TypeANY:
+			// ANY is ambiguous here and ends up parsed as a CLASS.
+			//
+			// TODO(tmthrgd): Using TYPE255 here doesn't seem to work and also
+			//   seems to fail for some other record types. Investigate.
 			continue
 		}
-		if _, err := NewRR(prefix + typ); err != nil {
-			t.Errorf("failure to parse: %s %s: %v", prefix, typ, err)
+
+		s := prefix + name
+		if _, err := NewRR(s); err != nil {
+			t.Errorf("failure to parse: %s: %v", s, err)
+		}
+
+		s += " \\# 0"
+		if _, err := NewRR(s); err != nil {
+			t.Errorf("failure to parse: %s: %v", s, err)
 		}
 	}
 }