Pull request: dnsforward: stat empty queries

Merge in DNS/adguard-home from 5910-stats-malformed-entry to master

Squashed commit of the following:

commit ea6718daf0d65dc4b13cd08430da7fdeb6f7fd5b
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 17:34:50 2023 +0400

    all: imp docs

commit 5341c5b351c87556b91a7f692960edc76f44b21a
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 16:47:13 2023 +0400

    all: docs

commit 1703c421f501484a80ddcf4ca1ba24f71fa5d225
Merge: f56b62490 d26c480d0
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 16:46:13 2023 +0400

    Merge remote-tracking branch 'origin/master' into 5910-stats-malformed-entry

    # Conflicts:
    #	CHANGELOG.md

commit f56b62490c7033e1d0b620a9cb4ba8de64862de0
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 14:49:43 2023 +0400

    all: docs

commit be954a8cab950d038c8cf08b95158069fa0747ba
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 14:44:05 2023 +0400

    querylog: empty queries

commit 0cda309d402156e750c7387e471da6795aa43bd6
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Jun 19 14:26:41 2023 +0400

    dnsforward: stat empty queries

CHANGELOG.md

@@ -82,10 +82,13 @@ In this release, the schema version has changed from 20 to 21.
 
 ### Fixed
 
+- Queries with the question-section target `.`, for example `NS .`, are now
+  counted in the statistics and correctly shown in the query log ([#5910]).
 - Safe Search not working with `AAAA` queries for Yandex domains ([#5913]).
 
 [#951]:  https://github.com/AdguardTeam/AdGuardHome/issues/951
 [#1577]: https://github.com/AdguardTeam/AdGuardHome/issues/1577
+[#5910]: https://github.com/AdguardTeam/AdGuardHome/issues/5910
 [#5913]: https://github.com/AdguardTeam/AdGuardHome/issues/5913
 
 <!--

internal/dnsforward/stats.go

@@ -123,7 +123,10 @@ func (s *Server) updateStats(
 	pctx := ctx.proxyCtx
 	e := stats.Entry{}
 	e.Domain = strings.ToLower(pctx.Req.Question[0].Name)
-	e.Domain = e.Domain[:len(e.Domain)-1] // remove last "."
+	if e.Domain != "." {
+		// Remove last ".", but save the domain as is for "." queries.
+		e.Domain = e.Domain[:len(e.Domain)-1]
+	}
 
 	if clientID := ctx.clientID; clientID != "" {
 		e.Client = clientID

internal/dnsforward/stats_test.go

@@ -46,6 +46,10 @@ type testStats struct {
 
 // Update implements the [stats.Interface] interface for *testStats.
 func (l *testStats) Update(e stats.Entry) {
+	if e.Domain == "" {
+		return
+	}
+
 	l.lastEntry = e
 }
 
@@ -54,9 +58,12 @@ func (l *testStats) ShouldCount(string, uint16, uint16, []string) bool {
 	return true
 }
 
-func TestProcessQueryLogsAndStats(t *testing.T) {
+func TestServer_ProcessQueryLogsAndStats(t *testing.T) {
+	const domain = "example.com."
+
 	testCases := []struct {
 		name           string
+		domain         string
 		proto          proxy.Proto
 		addr           net.Addr
 		clientID       string
@@ -67,6 +74,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult stats.Result
 	}{{
 		name:           "success_udp",
+		domain:         domain,
 		proto:          proxy.ProtoUDP,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -77,6 +85,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_tls_clientid",
+		domain:         domain,
 		proto:          proxy.ProtoTLS,
 		addr:           &net.TCPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "cli42",
@@ -87,6 +96,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_tls",
+		domain:         domain,
 		proto:          proxy.ProtoTLS,
 		addr:           &net.TCPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -97,6 +107,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_quic",
+		domain:         domain,
 		proto:          proxy.ProtoQUIC,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -107,6 +118,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_https",
+		domain:         domain,
 		proto:          proxy.ProtoHTTPS,
 		addr:           &net.TCPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -117,6 +129,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_dnscrypt",
+		domain:         domain,
 		proto:          proxy.ProtoDNSCrypt,
 		addr:           &net.TCPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -127,6 +140,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RNotFiltered,
 	}, {
 		name:           "success_udp_filtered",
+		domain:         domain,
 		proto:          proxy.ProtoUDP,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -137,6 +151,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RFiltered,
 	}, {
 		name:           "success_udp_sb",
+		domain:         domain,
 		proto:          proxy.ProtoUDP,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -147,6 +162,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RSafeBrowsing,
 	}, {
 		name:           "success_udp_ss",
+		domain:         domain,
 		proto:          proxy.ProtoUDP,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -157,6 +173,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantStatResult: stats.RSafeSearch,
 	}, {
 		name:           "success_udp_pc",
+		domain:         domain,
 		proto:          proxy.ProtoUDP,
 		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 4}, Port: 1234},
 		clientID:       "",
@@ -165,6 +182,17 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		wantCode:       resultCodeSuccess,
 		reason:         filtering.FilteredParental,
 		wantStatResult: stats.RParental,
+	}, {
+		name:           "success_udp_pc_empty_fqdn",
+		domain:         ".",
+		proto:          proxy.ProtoUDP,
+		addr:           &net.UDPAddr{IP: net.IP{1, 2, 3, 5}, Port: 1234},
+		clientID:       "",
+		wantLogProto:   "",
+		wantStatClient: "1.2.3.5",
+		wantCode:       resultCodeSuccess,
+		reason:         filtering.FilteredParental,
+		wantStatResult: stats.RParental,
 	}}
 
 	ups, err := upstream.AddressToUpstream("1.1.1.1", nil)
@@ -181,7 +209,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			req := &dns.Msg{
 				Question: []dns.Question{{
-					Name: "example.com.",
+					Name: tc.domain,
 				}},
 			}
 			pctx := &proxy.DNSContext{

internal/querylog/qlog.go

@@ -161,12 +161,15 @@ 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])
+	}
 
 	entry = &logEntry{
 		// TODO(d.kolyshev): Export this timestamp to func params.
-		Time: time.Now(),
+		Time:   time.Now(),
-
+		QHost:  qHost,
-		QHost:  strings.ToLower(q.Name[:len(q.Name)-1]),
 		QType:  dns.Type(q.Qtype).String(),
 		QClass: dns.Class(q.Qclass).String(),
 

internal/querylog/qlog_test.go

@@ -43,6 +43,7 @@ func TestQueryLog(t *testing.T) {
 	// Add memory entries.
 	addEntry(l, "test.example.org", net.IPv4(1, 1, 1, 3), net.IPv4(2, 2, 2, 3))
 	addEntry(l, "example.com", net.IPv4(1, 1, 1, 4), net.IPv4(2, 2, 2, 4))
+	addEntry(l, "", net.IPv4(1, 1, 1, 5), net.IPv4(2, 2, 2, 5))
 
 	type tcAssertion struct {
 		host   string
@@ -59,10 +60,11 @@ func TestQueryLog(t *testing.T) {
 		name: "all",
 		sCr:  []searchCriterion{},
 		want: []tcAssertion{
-			{num: 0, host: "example.com", answer: net.IPv4(1, 1, 1, 4), client: net.IPv4(2, 2, 2, 4)},
+			{num: 0, host: ".", answer: net.IPv4(1, 1, 1, 5), client: net.IPv4(2, 2, 2, 5)},
-			{num: 1, host: "test.example.org", answer: net.IPv4(1, 1, 1, 3), client: net.IPv4(2, 2, 2, 3)},
+			{num: 1, host: "example.com", answer: net.IPv4(1, 1, 1, 4), client: net.IPv4(2, 2, 2, 4)},
-			{num: 2, host: "example.org", answer: net.IPv4(1, 1, 1, 2), client: net.IPv4(2, 2, 2, 2)},
+			{num: 2, host: "test.example.org", answer: net.IPv4(1, 1, 1, 3), client: net.IPv4(2, 2, 2, 3)},
-			{num: 3, host: "example.org", answer: net.IPv4(1, 1, 1, 1), client: net.IPv4(2, 2, 2, 1)},
+			{num: 3, host: "example.org", answer: net.IPv4(1, 1, 1, 2), client: net.IPv4(2, 2, 2, 2)},
+			{num: 4, host: "example.org", answer: net.IPv4(1, 1, 1, 1), client: net.IPv4(2, 2, 2, 1)},
 		},
 	}, {
 		name: "by_domain_strict",
@@ -104,10 +106,11 @@ func TestQueryLog(t *testing.T) {
 			value:         "2.2.2",
 		}},
 		want: []tcAssertion{
-			{num: 0, host: "example.com", answer: net.IPv4(1, 1, 1, 4), client: net.IPv4(2, 2, 2, 4)},
+			{num: 0, host: ".", answer: net.IPv4(1, 1, 1, 5), client: net.IPv4(2, 2, 2, 5)},
-			{num: 1, host: "test.example.org", answer: net.IPv4(1, 1, 1, 3), client: net.IPv4(2, 2, 2, 3)},
+			{num: 1, host: "example.com", answer: net.IPv4(1, 1, 1, 4), client: net.IPv4(2, 2, 2, 4)},
-			{num: 2, host: "example.org", answer: net.IPv4(1, 1, 1, 2), client: net.IPv4(2, 2, 2, 2)},
+			{num: 2, host: "test.example.org", answer: net.IPv4(1, 1, 1, 3), client: net.IPv4(2, 2, 2, 3)},
-			{num: 3, host: "example.org", answer: net.IPv4(1, 1, 1, 1), client: net.IPv4(2, 2, 2, 1)},
+			{num: 3, host: "example.org", answer: net.IPv4(1, 1, 1, 2), client: net.IPv4(2, 2, 2, 2)},
+			{num: 4, host: "example.org", answer: net.IPv4(1, 1, 1, 1), client: net.IPv4(2, 2, 2, 1)},
 		},
 	}}