Stop using packDomainName in IsDomainName (#873)

* Fork packDomainName for IsDomainName

* Eliminate msg buffer from packDomainName2

* Eliminate compression code from packDomainName2

* Remove off argument and return from packDomainName2

* Remove bs buffer from packDomainName2

* Merge packDomainName2 into IsDomainName

* Eliminate root label special case from IsDomainName

* Remove ls variable from IsDomainName

* Fixup comments in IsDomainName

* Remove msg == nil special cases from packDomainName

* Eliminate lenmsg variable from packDomainName

* Eliminate label counting from packDomainName

* Change off length check in IsDomainName

* Fix IsDomainName for escaped names

* Use strings.HasSuffix for IsFqdn

* Revert "Use strings.HasSuffix for IsFqdn"

I'll submit this as a seperate PR.

This reverts commit 80bf8c83700d121ea45edac0f00db52817498166.

* Cross reference IsDomainName and packDomainName

* Correct IsDomainName max length comment

defaults.go

@@ -164,11 +164,74 @@ func (dns *Msg) IsEdns0() *OPT {
 // the number of labels.  When false is returned the number of labels is not
 // defined.  Also note that this function is extremely liberal; almost any
 // string is a valid domain name as the DNS is 8 bit protocol. It checks if each
-// label fits in 63 characters, but there is no length check for the entire
+// label fits in 63 characters and that the entire name will fit into the 255
-// string s. I.e.  a domain name longer than 255 characters is considered valid.
+// octet wire format limit.
 func IsDomainName(s string) (labels int, ok bool) {
-	_, labels, err := packDomainName(s, nil, 0, compressionMap{}, false)
+	// XXX: The logic in this function was copied from packDomainName and
-	return labels, err == nil
+	// should be kept in sync with that function.
+
+	const lenmsg = 256
+
+	if len(s) == 0 { // Ok, for instance when dealing with update RR without any rdata.
+		return 0, false
+	}
+
+	if !strings.HasSuffix(s, ".") {
+		s += "."
+	}
+
+	// Each dot ends a segment of the name. Except for escaped dots (\.), which
+	// are normal dots.
+
+	var (
+		off    int
+		begin  int
+		wasDot bool
+	)
+	for i := 0; i < len(s); i++ {
+		switch s[i] {
+		case '\\':
+			if off+1 > lenmsg {
+				return labels, false
+			}
+
+			// check for \DDD
+			if i+3 < len(s) && isDigit(s[i+1]) && isDigit(s[i+2]) && isDigit(s[i+3]) {
+				i += 3
+				begin += 3
+			} else {
+				i++
+				begin++
+			}
+
+			wasDot = false
+		case '.':
+			if wasDot {
+				// two dots back to back is not legal
+				return labels, false
+			}
+			wasDot = true
+
+			labelLen := i - begin
+			if labelLen >= 1<<6 { // top two bits of length must be clear
+				return labels, false
+			}
+
+			// off can already (we're in a loop) be bigger than lenmsg
+			// this happens when a name isn't fully qualified
+			off += 1 + labelLen
+			if off > lenmsg {
+				return labels, false
+			}
+
+			labels++
+			begin = i + 1
+		default:
+			wasDot = false
+		}
+	}
+
+	return labels, true
 }
 
 // IsSubDomain checks if child is indeed a child of the parent. If child and parent

labels_test.go

@@ -162,6 +162,8 @@ func TestIsDomainName(t *testing.T) {
 		"www.example.com.":       {true, 3},
 		"mi\\k.nl.":              {true, 2},
 		"mi\\k.nl":               {true, 2},
+		longestDomain:            {true, 4},
+		longestUnprintableDomain: {true, 4},
 	}
 	for d, ok := range names {
 		l, k := IsDomainName(d)

msg.go

@@ -231,29 +231,21 @@ func (m compressionMap) find(s string) (int, bool) {
 // map needs to hold a mapping between domain names and offsets
 // pointing into msg.
 func PackDomainName(s string, msg []byte, off int, compression map[string]int, compress bool) (off1 int, err error) {
-	off1, _, err = packDomainName(s, msg, off, compressionMap{ext: compression}, compress)
+	return packDomainName(s, msg, off, compressionMap{ext: compression}, compress)
-	return
 }
 
-func packDomainName(s string, msg []byte, off int, compression compressionMap, compress bool) (off1 int, labels int, err error) {
+func packDomainName(s string, msg []byte, off int, compression compressionMap, compress bool) (off1 int, err error) {
-	// special case if msg == nil
+	// XXX: A logical copy of this function exists in IsDomainName and
-	lenmsg := 256
+	// should be kept in sync with this function.
-	if msg != nil {
-		lenmsg = len(msg)
-	}
 
 	ls := len(s)
 	if ls == 0 { // Ok, for instance when dealing with update RR without any rdata.
-		return off, 0, nil
+		return off, nil
 	}
 
-	// If not fully qualified, error out, but only if msg != nil #ugly
+	// If not fully qualified, error out.
 	if s[ls-1] != '.' {
-		if msg != nil {
+		return len(msg), ErrFqdn
-			return lenmsg, 0, ErrFqdn
-		}
-		s += "."
-		ls++
 	}
 
 	// Each dot ends a segment of the name.
@@ -283,8 +275,8 @@ loop:
 
 		switch c {
 		case '\\':
-			if off+1 > lenmsg {
+			if off+1 > len(msg) {
-				return lenmsg, labels, ErrBuf
+				return len(msg), ErrBuf
 			}
 
 			if bs == nil {
@@ -307,19 +299,19 @@ loop:
 		case '.':
 			if wasDot {
 				// two dots back to back is not legal
-				return lenmsg, labels, ErrRdata
+				return len(msg), ErrRdata
 			}
 			wasDot = true
 
 			labelLen := i - begin
 			if labelLen >= 1<<6 { // top two bits of length must be clear
-				return lenmsg, labels, ErrRdata
+				return len(msg), ErrRdata
 			}
 
 			// off can already (we're in a loop) be bigger than len(msg)
 			// this happens when a name isn't fully qualified
-			if off+1+labelLen > lenmsg {
+			if off+1+labelLen > len(msg) {
-				return lenmsg, labels, ErrBuf
+				return len(msg), ErrBuf
 			}
 
 			// Don't try to compress '.'
@@ -344,7 +336,6 @@ loop:
 			}
 
 			// The following is covered by the length check above.
-			if msg != nil {
 			msg[off] = byte(labelLen)
 
 			if bs == nil {
@@ -352,10 +343,8 @@ loop:
 			} else {
 				copy(msg[off+1:], bs[begin:i])
 			}
-			}
 			off += 1 + labelLen
 
-			labels++
 			begin = i + 1
 			compBegin = begin + compOff
 		default:
@@ -365,22 +354,21 @@ loop:
 
 	// Root label is special
 	if isRootLabel(s, bs, 0, ls) {
-		return off, labels, nil
+		return off, nil
 	}
 
 	// If we did compression and we find something add the pointer here
 	if pointer != -1 {
 		// We have two bytes (14 bits) to put the pointer in
-		// if msg == nil, we will never do compression
 		binary.BigEndian.PutUint16(msg[off:], uint16(pointer^0xC000))
-		return off + 2, labels, nil
+		return off + 2, nil
 	}
 
-	if msg != nil && off < lenmsg {
+	if off < len(msg) {
 		msg[off] = 0
 	}
 
-	return off + 1, labels, nil
+	return off + 1, nil
 }
 
 // isRootLabel returns whether s or bs, from off to end, is the root
@@ -1131,7 +1119,7 @@ func (dns *Msg) CopyTo(r1 *Msg) *Msg {
 }
 
 func (q *Question) pack(msg []byte, off int, compression compressionMap, compress bool) (int, error) {
-	off, _, err := packDomainName(q.Name, msg, off, compression, compress)
+	off, err := packDomainName(q.Name, msg, off, compression, compress)
 	if err != nil {
 		return off, err
 	}

msg_generate.go

@@ -115,9 +115,9 @@ return headerEnd, off, err
 			switch {
 			case st.Tag(i) == `dns:"-"`: // ignored
 			case st.Tag(i) == `dns:"cdomain-name"`:
-				o("off, _, err = packDomainName(rr.%s, msg, off, compression, compress)\n")
+				o("off, err = packDomainName(rr.%s, msg, off, compression, compress)\n")
 			case st.Tag(i) == `dns:"domain-name"`:
-				o("off, _, err = packDomainName(rr.%s, msg, off, compression, false)\n")
+				o("off, err = packDomainName(rr.%s, msg, off, compression, false)\n")
 			case st.Tag(i) == `dns:"a"`:
 				o("off, err = packDataA(rr.%s, msg, off)\n")
 			case st.Tag(i) == `dns:"aaaa"`:

msg_helpers.go

@@ -106,7 +106,7 @@ func (hdr RR_Header) pack(msg []byte, off int, compression compressionMap, compr
 		return off, off, nil
 	}
 
-	off, _, err := packDomainName(hdr.Name, msg, off, compression, compress)
+	off, err := packDomainName(hdr.Name, msg, off, compression, compress)
 	if err != nil {
 		return off, len(msg), err
 	}
@@ -640,7 +640,7 @@ func unpackDataDomainNames(msg []byte, off, end int) ([]string, int, error) {
 func packDataDomainNames(names []string, msg []byte, off int, compression compressionMap, compress bool) (int, error) {
 	var err error
 	for j := 0; j < len(names); j++ {
-		off, _, err = packDomainName(names[j], msg, off, compression, compress)
+		off, err = packDomainName(names[j], msg, off, compression, compress)
 		if err != nil {
 			return len(msg), err
 		}

zmsg.go

@@ -37,7 +37,7 @@ func (rr *AFSDB) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Hostname, msg, off, compression, false)
+	off, err = packDomainName(rr.Hostname, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -161,7 +161,7 @@ func (rr *CNAME) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Target, msg, off, compression, compress)
+	off, err = packDomainName(rr.Target, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -229,7 +229,7 @@ func (rr *DNAME) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Target, msg, off, compression, false)
+	off, err = packDomainName(rr.Target, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -433,7 +433,7 @@ func (rr *KX) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Exchanger, msg, off, compression, false)
+	off, err = packDomainName(rr.Exchanger, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -517,7 +517,7 @@ func (rr *LP) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Fqdn, msg, off, compression, false)
+	off, err = packDomainName(rr.Fqdn, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -529,7 +529,7 @@ func (rr *MB) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mb, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mb, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -541,7 +541,7 @@ func (rr *MD) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Md, msg, off, compression, compress)
+	off, err = packDomainName(rr.Md, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -553,7 +553,7 @@ func (rr *MF) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mf, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mf, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -565,7 +565,7 @@ func (rr *MG) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mg, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mg, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -577,11 +577,11 @@ func (rr *MINFO) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Rmail, msg, off, compression, compress)
+	off, err = packDomainName(rr.Rmail, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Email, msg, off, compression, compress)
+	off, err = packDomainName(rr.Email, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -593,7 +593,7 @@ func (rr *MR) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mr, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mr, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -609,7 +609,7 @@ func (rr *MX) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mx, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mx, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -641,7 +641,7 @@ func (rr *NAPTR) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Replacement, msg, off, compression, false)
+	off, err = packDomainName(rr.Replacement, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -693,7 +693,7 @@ func (rr *NS) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Ns, msg, off, compression, compress)
+	off, err = packDomainName(rr.Ns, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -705,7 +705,7 @@ func (rr *NSAPPTR) pack(msg []byte, off int, compression compressionMap, compres
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Ptr, msg, off, compression, false)
+	off, err = packDomainName(rr.Ptr, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -717,7 +717,7 @@ func (rr *NSEC) pack(msg []byte, off int, compression compressionMap, compress b
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.NextDomain, msg, off, compression, false)
+	off, err = packDomainName(rr.NextDomain, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -843,7 +843,7 @@ func (rr *PTR) pack(msg []byte, off int, compression compressionMap, compress bo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Ptr, msg, off, compression, compress)
+	off, err = packDomainName(rr.Ptr, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -859,11 +859,11 @@ func (rr *PX) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Map822, msg, off, compression, false)
+	off, err = packDomainName(rr.Map822, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mapx400, msg, off, compression, false)
+	off, err = packDomainName(rr.Mapx400, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -911,11 +911,11 @@ func (rr *RP) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mbox, msg, off, compression, false)
+	off, err = packDomainName(rr.Mbox, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Txt, msg, off, compression, false)
+	off, err = packDomainName(rr.Txt, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -955,7 +955,7 @@ func (rr *RRSIG) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.SignerName, msg, off, compression, false)
+	off, err = packDomainName(rr.SignerName, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -975,7 +975,7 @@ func (rr *RT) pack(msg []byte, off int, compression compressionMap, compress boo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Host, msg, off, compression, false)
+	off, err = packDomainName(rr.Host, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1015,7 +1015,7 @@ func (rr *SIG) pack(msg []byte, off int, compression compressionMap, compress bo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.SignerName, msg, off, compression, false)
+	off, err = packDomainName(rr.SignerName, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1055,11 +1055,11 @@ func (rr *SOA) pack(msg []byte, off int, compression compressionMap, compress bo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Ns, msg, off, compression, compress)
+	off, err = packDomainName(rr.Ns, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Mbox, msg, off, compression, compress)
+	off, err = packDomainName(rr.Mbox, msg, off, compression, compress)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1115,7 +1115,7 @@ func (rr *SRV) pack(msg []byte, off int, compression compressionMap, compress bo
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Target, msg, off, compression, false)
+	off, err = packDomainName(rr.Target, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1171,11 +1171,11 @@ func (rr *TALINK) pack(msg []byte, off int, compression compressionMap, compress
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.PreviousName, msg, off, compression, false)
+	off, err = packDomainName(rr.PreviousName, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.NextName, msg, off, compression, false)
+	off, err = packDomainName(rr.NextName, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1187,7 +1187,7 @@ func (rr *TKEY) pack(msg []byte, off int, compression compressionMap, compress b
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Algorithm, msg, off, compression, false)
+	off, err = packDomainName(rr.Algorithm, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}
@@ -1255,7 +1255,7 @@ func (rr *TSIG) pack(msg []byte, off int, compression compressionMap, compress b
 	if err != nil {
 		return headerEnd, off, err
 	}
-	off, _, err = packDomainName(rr.Algorithm, msg, off, compression, false)
+	off, err = packDomainName(rr.Algorithm, msg, off, compression, false)
 	if err != nil {
 		return headerEnd, off, err
 	}