fix multiple problems with the validation function

- it is a host name validation function
- a trailing dot is valid
- without trailing dot, max len is 253
- with a trailing dot, max len is 254
- use table tests with error message checking
- clarify some error messages

README.md

@@ -7,13 +7,13 @@
 
 # domain.Check()
 
-This package contains a single function that checks the validity of a domain name. 
+This package contains a single function that checks the validity of a **host name** domain name.
 
 A domain name must respect rules defined in 
 - [section 3.5 of RFC 1034 ("Domain names - concepts and facilities")](https://tools.ietf.org/html/rfc1034#section-3.5)
 - [section 2 of RFC 1123 ("Requirements for Internet Hosts -- Application and Support")](https://tools.ietf.org/html/rfc1123#section-2)
 
-The `domain.Check` function ensures that the domain name respect those rules. If not, it returns an error explaining the detected problem. 
+The `domain.Check` function ensures that the given host name respect those rules. If not, it returns an error explaining the detected problem.
 
 ## Prerequisites
 
@@ -24,7 +24,7 @@ The package has no prerequisites and external dependencies.
 To install or update this package use the instruction:
 
 ```bash
-go get -u "github.com/chmike/domain"
+go get github.com/chmike/domain@latest
 ```
 
 ## Usage examples
@@ -34,7 +34,7 @@ The `Check` function can be used to check the validity of host or domain names.
 ```go
 name := "host.example.com"
 if err := domain.Check(name); if err != nil {
-    log.Fatalf("invalid domain name '%s': %v", name, err)
+    log.Fatalf("invalid host name '%s': %v", name, err)
 }
 ```
 

check.go

@@ -6,15 +6,23 @@ import (
 	"unicode/utf8"
 )
 
-// Check returns an error if the domain name is not valid.
+// Check returns an error if the host name is not valid.
 // See https://tools.ietf.org/html/rfc1034#section-3.5 and
 // https://tools.ietf.org/html/rfc1123#section-2.
 func Check(name string) error {
-	switch {
-	case len(name) == 0:
+	if len(name) == 0 {
 		return errors.New("domain name is empty")
-	case len(name) > 255:
-		return fmt.Errorf("domain name length is %d, can't exceed 255", len(name))
+	}
+	if name[len(name)-1] == '.' {
+		if len(name) > 254 {
+			return fmt.Errorf("domain name length is %d, can't exceed 254 with a trailing dot", len(name))
+		}
+		name = name[:len(name)-1] // drop valid trailing dot
+		if len(name) == 0 {
+			return errors.New("domain name is a single dot")
+		}
+	} else if len(name) > 253 {
+		return fmt.Errorf("domain name length is %d, can't exceed 253 without a trailing dot", len(name))
 	}
 	var l int
 	for i := 0; i < len(name); i++ {
@@ -23,7 +31,7 @@ func Check(name string) error {
 			// check domain labels validity
 			switch {
 			case i == l:
-				return fmt.Errorf("domain has invalid character '.' at offset %d, label can't begin with a period", i)
+				return fmt.Errorf("domain has an empty label at offset %d", l)
 			case i-l > 63:
 				return fmt.Errorf("domain byte length of label '%s' is %d, can't exceed 63", name[l:i], i-l)
 			case name[l] == '-':
@@ -46,8 +54,6 @@ func Check(name string) error {
 	}
 	// check top level domain validity
 	switch {
-	case l == len(name):
-		return fmt.Errorf("domain has missing top level domain, domain can't end with a period")
 	case len(name)-l > 63:
 		return fmt.Errorf("domain's top level domain '%s' has byte length %d, can't exceed 63", name[l:], len(name)-l)
 	case name[l] == '-':

check_test.go

@@ -5,68 +5,58 @@ import (
 	"testing"
 )
 
+var longName = strings.Repeat(strings.Repeat("a", 63)+".", 4)
+
 func TestCheck(t *testing.T) {
-	if err := Check(""); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.com"); err != nil {
-		t.Errorf("unexpected error: %s", err)
-	}
-	if err := Check("EXAMPLE.com"); err != nil {
-		t.Errorf("unexpected error: %s", err)
-	}
-	if err := Check("foo-bar.com"); err != nil {
-		t.Errorf("unexpected error: %s", err)
-	}
-	if err := Check("www1.foo-bar.com"); err != nil {
-		t.Errorf("unexpected error: %s", err)
-	}
-	if err := Check("192.168.1.1.example.com"); err != nil {
-		t.Errorf("unexpected error: %s", err)
-	}
-	if err := Check(strings.Repeat("a", 300)); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check(strings.Repeat("a", 70) + ".com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.com" + strings.Repeat("a", 70)); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("?"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("\t"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("exàmple.com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("www.\xbd\xb2.com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("-example.com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example-.com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.-com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.com-"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.1com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check(".example.com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example..com"); err == nil {
-		t.Errorf("unexpected nil error")
-	}
-	if err := Check("example.com."); err == nil {
-		t.Errorf("unexpected nil error")
+	tests := []struct {
+		n string
+		e string
+	}{
+		// 0
+		{n: "", e: "domain name is empty"},
+		{n: "example.com"},
+		{n: "EXAMPLE.com"},
+		{n: "foo-bar.com"},
+		{n: "www1.foo-bar.com"},
+		// 5
+		{n: "192.168.1.1.example.com"},
+		{n: strings.Repeat("a", 70) + ".com", e: "domain byte length of label 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' is 70, can't exceed 63"},
+		{n: "example.com" + strings.Repeat("a", 70), e: "domain's top level domain 'comaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' has byte length 73, can't exceed 63"},
+		{n: "?", e: "domain has invalid character '?' at offset 0"},
+		{n: "\t", e: "domain has invalid character '\t' at offset 0"},
+		// 10
+		{n: "exàmple.com", e: "domain has invalid character 'à' at offset 2"},
+		{n: "www.\xbd\xb2.com", e: "domain has invalid rune at offset 4"},
+		{n: "-example.com", e: "domain label '-example' at offset 0 begins with a hyphen"},
+		{n: "example-.com", e: "domain label 'example-' at offset 0 ends with a hyphen"},
+		{n: "example.-com", e: "domain's top level domain '-com' at offset 8 begin with a hyphen"},
+		// 15
+		{n: "example.com-", e: "domain's top level domain 'com-' at offset 8 ends with a hyphen"},
+		{n: "example.1com", e: "domain's top level domain '1com' at offset 8 begins with a digit"},
+		{n: ".example.com", e: "domain has an empty label at offset 0"},
+		{n: "example..com", e: "domain has an empty label at offset 8"},
+		{n: "example.com."},
+		// 20
+		{n: longName, e: "domain name length is 256, can't exceed 254 with a trailing dot"},
+		{n: longName[:253]},
+		{n: longName[:253] + "."},
+		{n: longName[:254], e: "domain name length is 254, can't exceed 253 without a trailing dot"},
+		{n: longName[:255] + ".", e: "domain name length is 256, can't exceed 254 with a trailing dot"},
+		// 25
+		{n: ".", e: "domain name is a single dot"},
+	}
+	for i, test := range tests {
+		err := Check(test.n)
+		if (err == nil) != (test.e == "") {
+			if err != nil {
+				t.Errorf("%2d unexpected error: %q", i, err)
+			} else {
+				t.Errorf("%2d unexpected nil error", i)
+			}
+			continue
+		}
+		if err != nil && err.Error() != test.e {
+			t.Errorf("%2d expect error %q, got %q", i, test.e, err)
+		}
 	}
 }