Pull request 1916: 5990-root-ignore

Updates #5990.

Squashed commit of the following:

commit 1d5d3451c855681a631b85652417ee1bebadab01
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jul 11 20:11:45 2023 +0300

    all: allow ignoring root in querylog and stats

CHANGELOG.md

@@ -23,6 +23,11 @@ See also the [v0.107.34 GitHub milestone][ms-v0.107.34].
 NOTE: Add new changes BELOW THIS COMMENT.
 -->
 
+### Added
+
+- Ability to ignore queries for the root domain, such as `NS .` queries
+  ([#5990]).
+
 ### Changed
 
 - Improved CPU and RAM consumption during updates of filtering-rule lists.
@@ -83,6 +88,7 @@ In this release, the schema version has changed from 23 to 24.
 
 [#5896]: https://github.com/AdguardTeam/AdGuardHome/issues/5896
 [#5972]: https://github.com/AdguardTeam/AdGuardHome/issues/5972
+[#5990]: https://github.com/AdguardTeam/AdGuardHome/issues/5990
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

internal/aghnet/addr.go

@@ -0,0 +1,43 @@
+package aghnet
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/AdguardTeam/golibs/stringutil"
+)
+
+// NormalizeDomain returns a lowercased version of host without the final dot,
+// unless host is ".", in which case it returns it unchanged.  That is a special
+// case that to allow matching queries like:
+//
+//	dig IN NS '.'
+func NormalizeDomain(host string) (norm string) {
+	if host == "." {
+		return host
+	}
+
+	return strings.ToLower(strings.TrimSuffix(host, "."))
+}
+
+// NewDomainNameSet returns nil and error, if list has duplicate or empty domain
+// name.  Otherwise returns a set, which contains domain names normalized using
+// [NormalizeDomain].
+func NewDomainNameSet(list []string) (set *stringutil.Set, err error) {
+	set = stringutil.NewSet()
+
+	for i, host := range list {
+		if host == "" {
+			return nil, fmt.Errorf("at index %d: hostname is empty", i)
+		}
+
+		host = NormalizeDomain(host)
+		if set.Has(host) {
+			return nil, fmt.Errorf("duplicate hostname %q at index %d", host, i)
+		}
+
+		set.Add(host)
+	}
+
+	return set, nil
+}

internal/aghnet/addr_test.go

@@ -0,0 +1,59 @@
+package aghnet_test
+
+import (
+	"testing"
+
+	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
+	"github.com/AdguardTeam/golibs/testutil"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewDomainNameSet(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		name       string
+		wantErrMsg string
+		in         []string
+	}{{
+		name:       "nil",
+		wantErrMsg: "",
+		in:         nil,
+	}, {
+		name:       "success",
+		wantErrMsg: "",
+		in: []string{
+			"Domain.Example",
+			".",
+		},
+	}, {
+		name:       "dups",
+		wantErrMsg: `duplicate hostname "domain.example" at index 1`,
+		in: []string{
+			"Domain.Example",
+			"domain.example",
+		},
+	}, {
+		name:       "bad_domain",
+		wantErrMsg: "at index 0: hostname is empty",
+		in: []string{
+			"",
+		},
+	}}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			set, err := aghnet.NewDomainNameSet(tc.in)
+			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
+			if err != nil {
+				return
+			}
+
+			for _, host := range tc.in {
+				assert.Truef(t, set.Has(aghnet.NormalizeDomain(host)), "%q not matched", host)
+			}
+		})
+	}
+}

internal/aghnet/hostgen.go

@@ -1,12 +1,8 @@
 package aghnet
 
 import (
-	"fmt"
 	"net/netip"
 	"strings"
-
-	"github.com/AdguardTeam/golibs/errors"
-	"github.com/AdguardTeam/golibs/stringutil"
 )
 
 // GenerateHostname generates the hostname from ip.  In case of using IPv4 the
@@ -29,32 +25,8 @@ func GenerateHostname(ip netip.Addr) (hostname string) {
 	hostname = ip.StringExpanded()
 
 	if ip.Is4() {
-		return strings.Replace(hostname, ".", "-", -1)
+		return strings.ReplaceAll(hostname, ".", "-")
 	}
 
-	return strings.Replace(hostname, ":", "-", -1)
-}
-
-// NewDomainNameSet returns nil and error, if list has duplicate or empty
-// domain name.  Otherwise returns a set, which contains non-FQDN domain names,
-// and nil error.
-func NewDomainNameSet(list []string) (set *stringutil.Set, err error) {
-	set = stringutil.NewSet()
-
-	for i, v := range list {
-		host := strings.ToLower(strings.TrimSuffix(v, "."))
-		// TODO(a.garipov): Think about ignoring empty (".") names in the
-		// future.
-		if host == "" {
-			return nil, errors.Error("host name is empty")
-		}
-
-		if set.Has(host) {
-			return nil, fmt.Errorf("duplicate host name %q at index %d", host, i)
-		}
-
-		set.Add(host)
-	}
-
-	return set, nil
+	return strings.ReplaceAll(hostname, ":", "-")
 }

internal/dnsforward/stats.go

@@ -2,9 +2,9 @@ package dnsforward
 
 import (
 	"net"
-	"strings"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/AdGuardHome/internal/querylog"
 	"github.com/AdguardTeam/AdGuardHome/internal/stats"
@@ -24,7 +24,7 @@ func (s *Server) processQueryLogsAndStats(dctx *dnsContext) (rc resultCode) {
 	pctx := dctx.proxyCtx
 
 	q := pctx.Req.Question[0]
-	host := strings.ToLower(strings.TrimSuffix(q.Name, "."))
+	host := aghnet.NormalizeDomain(q.Name)
 
 	ip, _ := netutil.IPAndPortFromAddr(pctx.Addr)
 	ip = slices.Clone(ip)
@@ -139,11 +139,10 @@ func (s *Server) updateStats(
 	clientIP string,
 ) {
 	pctx := ctx.proxyCtx
-	e := stats.Entry{}
-	e.Domain = strings.ToLower(pctx.Req.Question[0].Name)
-	if e.Domain != "." {
-		// Remove last ".", but save the domain as is for "." queries.
-		e.Domain = e.Domain[:len(e.Domain)-1]
+	e := stats.Entry{
+		Domain: aghnet.NormalizeDomain(pctx.Req.Question[0].Name),
+		Result: stats.RNotFiltered,
+		Time:   uint32(elapsed / 1000),
 	}
 
 	if clientID := ctx.clientID; clientID != "" {
@@ -152,9 +151,6 @@ func (s *Server) updateStats(
 		e.Client = clientIP
 	}
 
-	e.Time = uint32(elapsed / 1000)
-	e.Result = stats.RNotFiltered
-
 	switch res.Reason {
 	case filtering.FilteredSafeBrowsing:
 		e.Result = stats.RSafeBrowsing
@@ -162,7 +158,8 @@ func (s *Server) updateStats(
 		e.Result = stats.RParental
 	case filtering.FilteredSafeSearch:
 		e.Result = stats.RSafeSearch
-	case filtering.FilteredBlockList,
+	case
+		filtering.FilteredBlockList,
 		filtering.FilteredInvalid,
 		filtering.FilteredBlockedService:
 		e.Result = stats.RFiltered

internal/home/config.go

@@ -240,6 +240,7 @@ type tlsConfigSettings struct {
 
 type queryLogConfig struct {
 	// Ignored is the list of host names, which should not be written to log.
+	// "." is considered to be the root domain.
 	Ignored []string `yaml:"ignored"`
 
 	// Interval is the interval for query log's files rotation.

internal/querylog/qlog.go

@@ -4,7 +4,6 @@ package querylog
 import (
 	"fmt"
 	"os"
-	"strings"
 	"sync"
 	"time"
 
@@ -161,10 +160,7 @@ func (l *queryLog) clear() {
 // newLogEntry creates an instance of logEntry from parameters.
 func newLogEntry(params *AddParams) (entry *logEntry) {
 	q := params.Question.Question[0]
-	qHost := q.Name
-	if qHost != "." {
-		qHost = strings.ToLower(q.Name[:len(q.Name)-1])
-	}
+	qHost := aghnet.NormalizeDomain(q.Name)
 
 	entry = &logEntry{
 		// TODO(d.kolyshev): Export this timestamp to func params.

internal/stats/http_test.go

@@ -86,7 +86,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: duplicate host name \"ignor.ed\" at index 1\n",
+		wantErr:  "ignored: duplicate hostname \"ignor.ed\" at index 1\n",
 	}, {
 		name: "ignored_empty",
 		body: getConfigResp{
@@ -97,7 +97,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: host name is empty\n",
+		wantErr:  "ignored: at index 0: hostname is empty\n",
 	}, {
 		name: "enabled_is_null",
 		body: getConfigResp{