SplitDomainName handles non-FQDN labels correctly, doesn't return '.' in parts.

SplitDomainName would always include the '.' after each label segment. This was
inconsistent with the "." case (were it returned nil) and didn't seem too useful
since it required more processing to remove the '.' (like when building a
compression dictionary in Msg.Len() or Msg.Pack()). It also had issues with the
last segment, not including it in the split. It now returns all segments,
including the last, irrespective of the label being fully qualified.
A test has also been added to ensure no regressions.

labels.go

@@ -10,24 +10,35 @@ package dns
 // www.miek.nl. returns []string{"www", "miek", "nl"}
 // The root label (.) returns nil.
 func SplitDomainName(s string) []string {
-	idx := Split(s)
+	var (
+		labels  []string
+		fqdnEnd int // offset of the final '.' or the length of the name
+		idx     = Split(s)
+		begin   = 0
+	)
+
+	if s[len(s)-1] == '.' {
+		fqdnEnd = len(s) - 1
+	} else {
+		fqdnEnd = len(s)
+	}
+
 	switch len(idx) {
 	case 0:
 		return nil
 	case 1:
-		return []string{s}
+		// no-op
 	default:
-		begin := 0
 		end := 0
-		labels := make([]string, 0)
 		for i := 1; i < len(idx); i++ {
 			end = idx[i]
-			labels = append(labels, s[begin:end])
+			labels = append(labels, s[begin:end-1])
 			begin = end
 		}
-		return labels
 	}
-	panic("dns: not reached")
+
+	labels = append(labels, s[begin:fqdnEnd])
+	return labels
 }
 
 // CompareDomainName compares the names s1 and s2 and

labels_test.go

@@ -45,6 +45,7 @@ func TestSplit(t *testing.T) {
 	splitter := map[string]int{
 		"www.miek.nl.":   3,
 		"www.miek.nl":    3,
+		"www..miek.nl":   4,
 		`www\.miek.nl.`:  2,
 		`www\\.miek.nl.`: 3,
 		".":              0,
@@ -63,20 +64,30 @@ func TestSplit(t *testing.T) {
 	}
 }
 
-func TestCountLabel(t *testing.T) {
-	labels := map[string]int{
-		"miek.nl":       2,
-		".":             0,
-		"www.miek.nl.":  3,
-		"www.miek.nl":   3,
-		"www..miek.nl":  4,
-		`www\.miek.nl`:  2,
-		`www\\.miek.nl`: 3,
+func TestSplitDomainName(t *testing.T) {
+	labels := map[string][]string{
+		"miek.nl":       []string{"miek", "nl"},
+		".":             nil,
+		"www.miek.nl.":  []string{"www", "miek", "nl"},
+		"www.miek.nl":   []string{"www", "miek", "nl"},
+		"www..miek.nl":  []string{"www", "", "miek", "nl"},
+		`www\.miek.nl`:  []string{`www\.miek`, "nl"},
+		`www\\.miek.nl`: []string{`www\\`, "miek", "nl"},
 	}
-	for owner, lab := range labels {
-		if l := CountLabel(owner); l != lab {
-			t.Logf("%s should have %d labels, got %d\n", owner, lab, l)
+domainLoop:
+	for domain, splits := range labels {
+		parts := SplitDomainName(domain)
+		if len(parts) != len(splits) {
+			t.Logf("SplitDomainName returned %v for %s, expected %v", parts, domain, splits)
 			t.Fail()
+			continue domainLoop
+		}
+		for i := range parts {
+			if parts[i] != splits[i] {
+				t.Logf("SplitDomainName returned %v for %s, expected %v", parts, domain, splits)
+				t.Fail()
+				continue domainLoop
+			}
 		}
 	}
 }