all: imp err msgs

internal/aghnet/addr.go

@@ -48,32 +48,32 @@ const maxDomainLabelLen = 63
 // See https://stackoverflow.com/a/32294443/1892060.
 const maxDomainNameLen = 253
 
-const invalidCharMsg = "invalid char %q at index %d in %q"
-
 // ValidateDomainNameLabel returns an error if label is not a valid label of
 // a domain name.
 func ValidateDomainNameLabel(label string) (err error) {
+	defer agherr.Annotate("validating label %q: %w", &err, label)
+
 	l := len(label)
 	if l > maxDomainLabelLen {
-		return fmt.Errorf("%q is too long, max: %d", label, maxDomainLabelLen)
+		return fmt.Errorf("label is too long, max: %d", maxDomainLabelLen)
 	} else if l == 0 {
 		return agherr.Error("label is empty")
 	}
 
 	if r := label[0]; !IsValidHostOuterRune(rune(r)) {
-		return fmt.Errorf(invalidCharMsg, r, 0, label)
+		return fmt.Errorf("invalid char %q at index %d", r, 0)
 	} else if l == 1 {
 		return nil
 	}
 
 	for i, r := range label[1 : l-1] {
 		if !isValidHostRune(r) {
-			return fmt.Errorf(invalidCharMsg, r, i+1, label)
+			return fmt.Errorf("invalid char %q at index %d", r, i+1)
 		}
 	}
 
 	if r := label[l-1]; !IsValidHostOuterRune(rune(r)) {
-		return fmt.Errorf(invalidCharMsg, r, l-1, label)
+		return fmt.Errorf("invalid char %q at index %d", r, l-1)
 	}
 
 	return nil
@@ -87,7 +87,7 @@ func ValidateDomainNameLabel(label string) (err error) {
 // TODO(a.garipov): After making sure that this works correctly, port this into
 // module golibs.
 func ValidateDomainName(name string) (err error) {
-	defer agherr.Annotate("validating %q: %w", &err, name)
+	defer agherr.Annotate("validating domain name %q: %w", &err, name)
 
 	name, err = idna.ToASCII(name)
 	if err != nil {

internal/aghnet/addr_test.go

@@ -95,42 +95,46 @@ func TestValidateDomainName(t *testing.T) {
 	}, {
 		name:       "empty",
 		in:         "",
-		wantErrMsg: `validating "": domain name is empty`,
+		wantErrMsg: `validating domain name "": domain name is empty`,
 	}, {
 		name: "bad_symbol",
 		in:   "!!!",
-		wantErrMsg: `validating "!!!": invalid domain name label at index 0: ` +
-			`invalid char '!' at index 0 in "!!!"`,
+		wantErrMsg: `validating domain name "!!!": invalid domain name label at index 0: ` +
+			`validating label "!!!": invalid char '!' at index 0`,
 	}, {
 		name:       "bad_length",
 		in:         longDomainName,
-		wantErrMsg: `validating "` + longDomainName + `": too long, max: 253`,
+		wantErrMsg: `validating domain name "` + longDomainName + `": too long, max: 253`,
 	}, {
 		name: "bad_label_length",
 		in:   longLabelDomainName,
-		wantErrMsg: `validating "` + longLabelDomainName + `": ` +
-			`invalid domain name label at index 0: "` + longLabel +
-			`" is too long, max: 63`,
+		wantErrMsg: `validating domain name "` + longLabelDomainName + `": ` +
+			`invalid domain name label at index 0: validating label "` + longLabel +
+			`": label is too long, max: 63`,
 	}, {
 		name: "bad_label_empty",
 		in:   "example..com",
-		wantErrMsg: `validating "example..com": ` +
-			`invalid domain name label at index 1: label is empty`,
+		wantErrMsg: `validating domain name "example..com": ` +
+			`invalid domain name label at index 1: ` +
+			`validating label "": label is empty`,
 	}, {
 		name: "bad_label_first_symbol",
 		in:   "example.-aa.com",
-		wantErrMsg: `validating "example.-aa.com": invalid domain name label at index 1:` +
-			` invalid char '-' at index 0 in "-aa"`,
+		wantErrMsg: `validating domain name "example.-aa.com": ` +
+			`invalid domain name label at index 1: ` +
+			`validating label "-aa": invalid char '-' at index 0`,
 	}, {
 		name: "bad_label_last_symbol",
 		in:   "example-.aa.com",
-		wantErrMsg: `validating "example-.aa.com": invalid domain name label at index 0:` +
-			` invalid char '-' at index 7 in "example-"`,
+		wantErrMsg: `validating domain name "example-.aa.com": ` +
+			`invalid domain name label at index 0: ` +
+			`validating label "example-": invalid char '-' at index 7`,
 	}, {
 		name: "bad_label_symbol",
 		in:   "example.a!!!.com",
-		wantErrMsg: `validating "example.a!!!.com": invalid domain name label at index 1:` +
-			` invalid char '!' at index 1 in "a!!!"`,
+		wantErrMsg: `validating domain name "example.a!!!.com": ` +
+			`invalid domain name label at index 1: ` +
+			`validating label "a!!!": invalid char '!' at index 1`,
 	}}
 
 	for _, tc := range testCases {

internal/dhcpd/dhcpd.go

@@ -27,8 +27,8 @@ var webHandlersRegistered = false
 
 // Lease contains the necessary information about a DHCP lease
 type Lease struct {
-	// Expiry is the expiration time of the lease.  The unix timestamp of
-	// 1 means that this is a static lease.
+	// Expiry is the expiration time of the lease.  The unix timestamp value
+	// of 1 means that this is a static lease.
 	Expiry time.Time `json:"expires"`
 
 	Hostname string           `json:"hostname"`

internal/dnsforward/clientid.go

@@ -2,6 +2,7 @@ package dnsforward
 
 import (
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"path"
 	"strings"
@@ -15,7 +16,8 @@ import (
 func ValidateClientID(clientID string) (err error) {
 	err = aghnet.ValidateDomainNameLabel(clientID)
 	if err != nil {
-		return fmt.Errorf("invalid client id: %w", err)
+		// Replace the domain name label wrapper with our own.
+		return fmt.Errorf("invalid client id %q: %w", clientID, errors.Unwrap(err))
 	}
 
 	return nil

internal/dnsforward/clientid_test.go

@@ -117,8 +117,8 @@ func TestProcessClientID(t *testing.T) {
 		hostSrvName:  "example.com",
 		cliSrvName:   "!!!.example.com",
 		wantClientID: "",
-		wantErrMsg: `client id check: invalid client id: invalid char '!' ` +
-			`at index 0 in "!!!"`,
+		wantErrMsg: `client id check: invalid client id "!!!": ` +
+			`invalid char '!' at index 0`,
 		wantRes:   resultCodeError,
 		strictSNI: true,
 	}, {
@@ -128,9 +128,9 @@ func TestProcessClientID(t *testing.T) {
 		cliSrvName: `abcdefghijklmnopqrstuvwxyz0123456789abcdefghijklmno` +
 			`pqrstuvwxyz0123456789.example.com`,
 		wantClientID: "",
-		wantErrMsg: `client id check: invalid client id: "abcdefghijklmno` +
-			`pqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789" ` +
-			`is too long, max: 63`,
+		wantErrMsg: `client id check: invalid client id "abcdefghijklmno` +
+			`pqrstuvwxyz0123456789abcdefghijklmnopqrstuvwxyz0123456789": ` +
+			`label is too long, max: 63`,
 		wantRes:   resultCodeError,
 		strictSNI: true,
 	}, {
@@ -238,8 +238,8 @@ func TestProcessClientID_https(t *testing.T) {
 		name:         "invalid_client_id",
 		path:         "/dns-query/!!!",
 		wantClientID: "",
-		wantErrMsg: `client id check: invalid client id: invalid char '!' ` +
-			`at index 0 in "!!!"`,
+		wantErrMsg: `client id check: invalid client id "!!!": ` +
+			`invalid char '!' at index 0`,
 		wantRes: resultCodeError,
 	}}
 

internal/dnsforward/dnsforward_test.go

@@ -1166,9 +1166,9 @@ func TestNewServer(t *testing.T) {
 		in: DNSCreateParams{
 			LocalDomain: "!!!",
 		},
-		wantErrMsg: `local domain: validating "!!!": ` +
+		wantErrMsg: `local domain: validating domain name "!!!": ` +
 			`invalid domain name label at index 0: ` +
-			`invalid char '!' at index 0 in "!!!"`,
+			`validating label "!!!": invalid char '!' at index 0`,
 	}}
 
 	for _, tc := range testCases {