all: cover rdns with tests, imp aghnet functionality

internal/aghnet/net.go

@@ -320,29 +320,91 @@ func ipReverse(ip net.IP) (rev net.IP) {
 	return rev
 }
 
+// ARPA addresses' suffixes.
+const (
+	arpaV4Suffix = ".in-addr.arpa"
+	arpaV6Suffix = ".ip6.arpa"
+)
+
 // UnreverseAddr tries to convert reversed ARPA to a normal IP address.
 func UnreverseAddr(arpa string) (unreversed net.IP) {
-	const arpaV4 = ".in-addr.arpa"
-	const arpaV6 = ".ip6.arpa"
-
 	// Unify the input data.
 	arpa = strings.TrimSuffix(arpa, ".")
 	arpa = strings.ToLower(arpa)
 
-	if strings.HasSuffix(arpa, arpaV4) {
-		ip := strings.TrimSuffix(arpa, arpaV4)
+	if strings.HasSuffix(arpa, arpaV4Suffix) {
+		ip := strings.TrimSuffix(arpa, arpaV4Suffix)
 		ip4 := net.ParseIP(ip).To4()
 		if ip4 == nil {
 			return nil
 		}
 
 		return ipReverse(ip4)
 
-	} else if strings.HasSuffix(arpa, arpaV6) {
-		ip := strings.TrimSuffix(arpa, arpaV6)
+	} else if strings.HasSuffix(arpa, arpaV6Suffix) {
+		ip := strings.TrimSuffix(arpa, arpaV6Suffix)
 		return ipParseARPA6(ip)
 	}
 
 	// The suffix unrecognizable.
 	return nil
 }
+
+// The length of extreme cases of arpa formatted addresses.
+//
+// The example of IPv4 with maximum length:
+//
+//   49.91.20.104.in-addr.arpa
+//
+// The example of IPv6 with maximum length:
+//
+//   1.3.b.5.4.1.8.6.0.0.0.0.0.0.0.0.0.0.0.0.0.1.0.0.0.0.7.4.6.0.6.2.ip6.arpa
+//
+const (
+	arpaV4MaxLen = len("000.000.000.000") + len(arpaV4Suffix)
+	arpaV6MaxLen = len("0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0") +
+		len(arpaV6Suffix)
+)
+
+// ReverseAddr returns the ARPA hostname of the ip suitable for reverse DNS
+// (PTR) record lookups.  This is the modified version of ReverseAddr from
+// github.com/miekg/dns package with no error among returned values.
+func ReverseAddr(ip net.IP) (arpa string) {
+	var strLen int
+	var suffix string
+	// Don't handle errors in implementations since strings.WriteString
+	// never returns non-nil errors.
+	var writeByte func(val byte)
+	b := &strings.Builder{}
+	if ip4 := ip.To4(); ip4 != nil {
+		strLen, suffix = arpaV4MaxLen, arpaV4Suffix[1:]
+		ip = ip4
+		writeByte = func(val byte) {
+			_, _ = b.WriteString(strconv.Itoa(int(val)))
+			_, _ = b.WriteRune('.')
+		}
+
+	} else if ip6 := ip.To16(); ip6 != nil {
+		strLen, suffix = arpaV6MaxLen, arpaV6Suffix[1:]
+		ip = ip6
+		writeByte = func(val byte) {
+			lByte, rByte := val&0xF, val>>4
+
+			_, _ = b.WriteString(strconv.FormatUint(uint64(lByte), 16))
+			_, _ = b.WriteRune('.')
+			_, _ = b.WriteString(strconv.FormatUint(uint64(rByte), 16))
+			_, _ = b.WriteRune('.')
+		}
+
+	} else {
+		return ""
+	}
+
+	b.Grow(strLen)
+	for i := len(ip) - 1; i >= 0; i-- {
+		writeByte(ip[i])
+	}
+	_, _ = b.WriteString(suffix)
+
+	return b.String()
+}

internal/aghnet/net_test.go

@@ -17,7 +17,7 @@ func TestGetValidNetInterfacesForWeb(t *testing.T) {
 	}
 }
 
-func TestDNSReverseAddr(t *testing.T) {
+func TestUnreverseAddr(t *testing.T) {
 	testCases := []struct {
 		name string
 		have string
@@ -67,3 +67,41 @@ func TestDNSReverseAddr(t *testing.T) {
 		})
 	}
 }
+
+func TestReverseAddr(t *testing.T) {
+	testCases := []struct {
+		name string
+		want string
+		ip   net.IP
+	}{{
+		name: "valid_ipv4",
+		want: "4.3.2.1.in-addr.arpa",
+		ip:   net.IP{1, 2, 3, 4},
+	}, {
+		name: "valid_ipv6",
+		want: "1.3.b.5.4.1.8.6.0.0.0.0.0.0.0.0.0.0.0.0.0.1.0.0.0.0.7.4.6.0.6.2.ip6.arpa",
+		ip:   net.ParseIP("2606:4700:10::6814:5b31"),
+	}, {
+		name: "nil_ip",
+		want: "",
+		ip:   nil,
+	}, {
+		name: "unspecified_ipv6",
+		want: "0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa",
+		ip:   net.IPv6unspecified,
+	}, {
+		name: "unspecified_ipv4",
+		want: "0.0.0.0.in-addr.arpa",
+		ip:   net.IPv4zero,
+	}, {
+		name: "wrong_length_ip",
+		want: "",
+		ip:   net.IP{1, 2, 3, 4, 5},
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert.Equal(t, tc.want, ReverseAddr(tc.ip))
+		})
+	}
+}

internal/aghtest/upstream.go

@@ -3,7 +3,6 @@ package aghtest
 import (
 	"crypto/sha256"
 	"encoding/hex"
-	"errors"
 	"fmt"
 	"net"
 	"strings"
@@ -162,14 +161,17 @@ func (u *TestBlockUpstream) RequestsCount() int {
 
 // TestErrUpstream implements upstream.Upstream interface for replacing real
 // upstream in tests.
-type TestErrUpstream struct{}
+type TestErrUpstream struct {
+	// The error returned by Exchange may be unwraped to the Err.
+	Err error
+}
 
 // Exchange always returns nil Msg and non-nil error.
 func (u *TestErrUpstream) Exchange(*dns.Msg) (*dns.Msg, error) {
 	// We don't use an agherr.Error to avoid the import cycle since aghtests
 	// used to provide the utilities for testing which agherr (and any other
 	// testable package) should be able to use.
-	return nil, errors.New("bad")
+	return nil, fmt.Errorf("errupstream: %w", u.Err)
 }
 
 // Address always returns an empty string.

internal/home/clients.go

@@ -591,8 +591,9 @@ func (clients *clientsContainer) SetWhoisInfo(ip string, info [][]string) {
 // taken into account.  ok is true if the pairing was added.
 func (clients *clientsContainer) AddHost(ip, host string, src clientSource) (ok bool, err error) {
 	clients.lock.Lock()
+	defer clients.lock.Unlock()
+
 	ok = clients.addHostLocked(ip, host, src)
-	clients.lock.Unlock()
 
 	return ok, nil
 }

internal/home/rdns.go

@@ -106,16 +106,13 @@ const (
 func (r *RDNS) resolve(ip net.IP) (host string, err error) {
 	log.Tracef("rdns: resolving host for %q", ip)
 
-	var arpa string
-	arpa, err = dns.ReverseAddr(ip.String())
-	if err != nil {
-		return "", fmt.Errorf("reversing %q: %w", ip, err)
-	}
+	arpa := dns.Fqdn(aghnet.ReverseAddr(ip))
 	msg := &dns.Msg{
 		MsgHdr: dns.MsgHdr{
 			Id:               dns.Id(),
 			RecursionDesired: true,
 		},
+		Compress: true,
 		Question: []dns.Question{{
 			Name:   arpa,
 			Qtype:  dns.TypePTR,
@@ -130,11 +127,11 @@ func (r *RDNS) resolve(ip net.IP) (host string, err error) {
 		resp, err = r.dnsServer.Exchange(msg)
 	}
 	if err != nil {
-		return "", fmt.Errorf("performing lookup for %q: %w", ip, err)
+		return "", fmt.Errorf("performing lookup for %q: %w", arpa, err)
 	}
 
 	if len(resp.Answer) == 0 {
-		return "", fmt.Errorf("lookup for %q: %w", ip, rDNSEmptyAnswerErr)
+		return "", fmt.Errorf("lookup for %q: %w", arpa, rDNSEmptyAnswerErr)
 	}
 
 	ptr, ok := resp.Answer[0].(*dns.PTR)
@@ -160,11 +157,8 @@ func (r *RDNS) workerLoop() {
 			continue
 		}
 
-		_, err = r.clients.AddHost(ip.String(), host, ClientSourceRDNS)
-		// AddHost always returns nil error for now but may begin to
-		// return some non-nil errors in the future.
-		if err != nil {
-			log.Error("rdns: adding %q into clients: %s", ip, err)
-		}
+		// Don't handle any errors since AddHost doesn't return non-nil
+		// errors for now.
+		_, _ = r.clients.AddHost(ip.String(), host, ClientSourceRDNS)
 	}
 }