Repository: miekg/dns (go)

Issue:
sprintName disagrees with UnpackDomainName
`sprintName` escapes `'` while `UnpackDomainName` does not, which is surprising. It also fails to preserve escaping for decimal-escaped `.`s (i.e., `\046`), which is incorrect, and removes unnecessary escape sequences only _after_ a necessary escape sequence, which is bizarre.
```go
fromDisplayFormat := dns.Name(`\065'\066\046\067.example`).String()
fromWireFormat, _, _ := dns.UnpackDomainName([]byte{5, 65, '\'', 66, 46, 67, 7, 'e', 'x', 'a', 'm', 'p', 'l', 'e', 0}, 0)
fmt.Println("from text: " + fromDisplayFormat)
fmt.Println("from wire: " + fromWireFormat)
fmt.Println("expected same from both:")
fmt.Printf("           %s OR\n           %s\n",
	`A'B\.C.example.`,
	`A\'B\.C.example.`)
```
```
from text: \065\'B.C.example
from wire: A'B\.C.example.
expected same from both:
           A'B\.C.example. OR
           A\'B\.C.example.
```

Pull request:
Be consistent about domain name label character escaping
Fixes #1121

Patch:
diff --git a/generate.go b/generate.go
index f7e91a23..f713074a 100644
--- a/generate.go
+++ b/generate.go
@@ -20,13 +20,13 @@ import (
 // of $ after that are interpreted.
 func (zp *ZoneParser) generate(l lex) (RR, bool) {
 	token := l.token
-	step := 1
+	step := int64(1)
 	if i := strings.IndexByte(token, '/'); i >= 0 {
 		if i+1 == len(token) {
 			return zp.setParseError("bad step in $GENERATE range", l)
 		}
 
-		s, err := strconv.Atoi(token[i+1:])
+		s, err := strconv.ParseInt(token[i+1:], 10, 64)
 		if err != nil || s <= 0 {
 			return zp.setParseError("bad step in $GENERATE range", l)
 		}
@@ -40,12 +40,12 @@ func (zp *ZoneParser) generate(l lex) (RR, bool) {
 		return zp.setParseError("bad start-stop in $GENERATE range", l)
 	}
 
-	start, err := strconv.Atoi(sx[0])
+	start, err := strconv.ParseInt(sx[0], 10, 64)
 	if err != nil {
 		return zp.setParseError("bad start in $GENERATE range", l)
 	}
 
-	end, err := strconv.Atoi(sx[1])
+	end, err := strconv.ParseInt(sx[1], 10, 64)
 	if err != nil {
 		return zp.setParseError("bad stop in $GENERATE range", l)
 	}
@@ -75,10 +75,10 @@ func (zp *ZoneParser) generate(l lex) (RR, bool) {
 	r := &generateReader{
 		s: s,
 
-		cur:   start,
-		start: start,
-		end:   end,
-		step:  step,
+		cur:   int(start),
+		start: int(start),
+		end:   int(end),
+		step:  int(step),
 
 		file: zp.file,
 		lex:  &l,
@@ -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 || r.end+offset > 1<<31-1 {
+			if r.start+offset < 0 || int64(r.end) + int64(offset) > 1<<31-1 {
 				return 0, r.parseError("bad offset in $GENERATE", si+3+sep)
 			}
 
@@ -229,19 +229,19 @@ func modToPrintf(s string) (string, int, string) {
 		return "", 0, "bad base in $GENERATE"
 	}
 
-	offset, err := strconv.Atoi(offStr)
+	offset, err := strconv.ParseInt(offStr, 10, 64)
 	if err != nil {
 		return "", 0, "bad offset in $GENERATE"
 	}
 
-	width, err := strconv.Atoi(widthStr)
+	width, err := strconv.ParseInt(widthStr, 10, 64)
 	if err != nil || width < 0 || width > 255 {
 		return "", 0, "bad width in $GENERATE"
 	}
 
 	if width == 0 {
-		return "%" + base, offset, ""
+		return "%" + base, int(offset), ""
 	}
 
-	return "%0" + widthStr + base, offset, ""
+	return "%0" + widthStr + base, int(offset), ""
 }
diff --git a/msg.go b/msg.go
index 63656873..7001f6da 100644
--- a/msg.go
+++ b/msg.go
@@ -398,17 +398,12 @@ Loop:
 				return "", lenmsg, ErrLongDomain
 			}
 			for _, b := range msg[off : off+c] {
-				switch b {
-				case '.', '(', ')', ';', ' ', '@':
-					fallthrough
-				case '"', '\\':
+				if isDomainNameLabelSpecial(b) {
 					s = append(s, '\\', b)
-				default:
-					if b < ' ' || b > '~' { // unprintable, use \DDD
-						s = append(s, escapeByte(b)...)
-					} else {
-						s = append(s, b)
-					}
+				} else if b < ' ' || b > '~' {
+					s = append(s, escapeByte(b)...)
+				} else {
+					s = append(s, b)
 				}
 			}
 			s = append(s, '.')
diff --git a/types.go b/types.go
index ff32c3a1..7776b4f0 100644
--- a/types.go
+++ b/types.go
@@ -445,45 +445,38 @@ func sprintName(s string) string {
 	var dst strings.Builder
 
 	for i := 0; i < len(s); {
-		if i+1 < len(s) && s[i] == '\\' && s[i+1] == '.' {
+		if s[i] == '.' {
 			if dst.Len() != 0 {
-				dst.WriteString(s[i : i+2])
+				dst.WriteByte('.')
 			}
-			i += 2
+			i++
 			continue
 		}
 
 		b, n := nextByte(s, i)
 		if n == 0 {
-			i++
-			continue
-		}
-		if b == '.' {
-			if dst.Len() != 0 {
-				dst.WriteByte('.')
+			// Drop "dangling" incomplete escapes.
+			if dst.Len() == 0 {
+				return s[:i]
 			}
-			i += n
-			continue
+			break
 		}
-		switch b {
-		case ' ', '\'', '@', ';', '(', ')', '"', '\\': // additional chars to escape
+		if isDomainNameLabelSpecial(b) {
 			if dst.Len() == 0 {
 				dst.Grow(len(s) * 2)
 				dst.WriteString(s[:i])
 			}
 			dst.WriteByte('\\')
 			dst.WriteByte(b)
-		default:
-			if ' ' <= b && b <= '~' {
-				if dst.Len() != 0 {
-					dst.WriteByte(b)
-				}
-			} else {
-				if dst.Len() == 0 {
-					dst.Grow(len(s) * 2)
-					dst.WriteString(s[:i])
-				}
-				dst.WriteString(escapeByte(b))
+		} else if b < ' ' || b > '~' { // unprintable, use \DDD
+			if dst.Len() == 0 {
+				dst.Grow(len(s) * 2)
+				dst.WriteString(s[:i])
+			}
+			dst.WriteString(escapeByte(b))
+		} else {
+			if dst.Len() != 0 {
+				dst.WriteByte(b)
 			}
 		}
 		i += n
@@ -585,6 +578,17 @@ func escapeByte(b byte) string {
 	return escapedByteLarge[int(b)*4 : int(b)*4+4]
 }
 
+// isDomainNameLabelSpecial returns true if
+// a domain name label byte should be prefixed
+// with an escaping backslash.
+func isDomainNameLabelSpecial(b byte) bool {
+	switch b {
+	case '.', ' ', '\'', '@', ';', '(', ')', '"', '\\':
+		return true
+	}
+	return false
+}
+
 func nextByte(s string, offset int) (byte, int) {
 	if offset >= len(s) {
 		return 0, 0


Test patch:
diff --git a/msg_test.go b/msg_test.go
index 08a61950..cacc1ec4 100644
--- a/msg_test.go
+++ b/msg_test.go
@@ -202,6 +202,7 @@ func TestUnpackDomainName(t *testing.T) {
 			"\x03foo" + "\x03bar" + "\x07example" + "\xC0\x04",
 			"",
 			ErrLongDomain.Error()},
+		{"forward compression pointer", "\x02\xC0\xFF\xC0\x01", "", ErrBuf.Error()},
 		{"reserved compression pointer 0b10", "\x07example\x80", "", "dns: bad rdata"},
 		{"reserved compression pointer 0b01", "\x07example\x40", "", "dns: bad rdata"},
 	}
diff --git a/types_test.go b/types_test.go
index 5f3dece0..b3f5583f 100644
--- a/types_test.go
+++ b/types_test.go
@@ -74,10 +74,33 @@ func TestSplitN(t *testing.T) {
 }
 
 func TestSprintName(t *testing.T) {
-	got := sprintName("abc\\.def\007\"\127@\255\x05\xef\\")
-
-	if want := "abc\\.def\\007\\\"W\\@\\173\\005\\239"; got != want {
-		t.Errorf("expected %q, got %q", want, got)
+	tests := map[string]string{
+		// Non-numeric escaping of special printable characters.
+		" '@;()\"\\..example": `\ \'\@\;\(\)\"\..example`,
+		"\\032\\039\\064\\059\\040\\041\\034\\046\\092.example": `\ \'\@\;\(\)\"\.\\.example`,
+
+		// Numeric escaping of nonprintable characters.
+		"\x00\x07\x09\x0a\x1f.\x7f\x80\xad\xef\xff":           `\000\007\009\010\031.\127\128\173\239\255`,
+		"\\000\\007\\009\\010\\031.\\127\\128\\173\\239\\255": `\000\007\009\010\031.\127\128\173\239\255`,
+
+		// No escaping of other printable characters, at least after a prior escape.
+		";[a-zA-Z0-9_]+/*.~": `\;[a-zA-Z0-9_]+/*.~`,
+		";\\091\\097\\045\\122\\065\\045\\090\\048\\045\\057\\095\\093\\043\\047\\042.\\126": `\;[a-zA-Z0-9_]+/*.~`,
+		// "\\091\\097\\045\\122\\065\\045\\090\\048\\045\\057\\095\\093\\043\\047\\042.\\126": `[a-zA-Z0-9_]+/*.~`,
+
+		// Incomplete "dangling" escapes are dropped regardless of prior escaping.
+		"a\\": `a`,
+		";\\": `\;`,
+
+		// Escaped dots stay escaped regardless of prior escaping.
+		"a\\.\\046.\\.\\046": `a\.\..\.\.`,
+		"a\\046\\..\\046\\.": `a\.\..\.\.`,
+	}
+	for input, want := range tests {
+		got := sprintName(input)
+		if got != want {
+			t.Errorf("input %q: expected %q, got %q", input, want, got)
+		}
 	}
 }