all: sync with master; upd chlog

internal/filtering/safesearch/safesearch.go

@@ -161,12 +161,8 @@ func (ss *Default) resetEngine(
 // type check
 var _ filtering.SafeSearch = (*Default)(nil)
 
-// CheckHost implements the [filtering.SafeSearch] interface for
-// *DefaultSafeSearch.
-func (ss *Default) CheckHost(
-	host string,
-	qtype rules.RRType,
-) (res filtering.Result, err error) {
+// CheckHost implements the [filtering.SafeSearch] interface for *Default.
+func (ss *Default) CheckHost(host string, qtype rules.RRType) (res filtering.Result, err error) {
 	start := time.Now()
 	defer func() {
 		ss.log(log.DEBUG, "lookup for %q finished in %s", host, time.Since(start))
@@ -196,14 +192,10 @@ func (ss *Default) CheckHost(
 		return filtering.Result{}, err
 	}
 
-	if fltRes != nil {
-		res = *fltRes
-		ss.setCacheResult(host, qtype, res)
+	res = *fltRes
+	ss.setCacheResult(host, qtype, res)
 
-		return res, nil
-	}
-
-	return filtering.Result{}, fmt.Errorf("no ipv4 addresses for %q", host)
+	return res, nil
 }
 
 // searchHost looks up DNS rewrites in the internal DNS filtering engine.
@@ -229,7 +221,11 @@ func (ss *Default) searchHost(host string, qtype rules.RRType) (res *rules.DNSRe
 }
 
 // newResult creates Result object from rewrite rule.  qtype must be either
-// [dns.TypeA] or [dns.TypeAAAA].
+// [dns.TypeA] or [dns.TypeAAAA].  If err is nil, res is never nil, so that the
+// empty result is converted into a NODATA response.
+//
+// TODO(a.garipov): Use the main rewrite result mechanism used in
+// [dnsforward.Server.filterDNSRequest].
 func (ss *Default) newResult(
 	rewrite *rules.DNSRewrite,
 	qtype rules.RRType,
@@ -243,9 +239,10 @@ func (ss *Default) newResult(
 	}
 
 	if rewrite.RRType == qtype {
-		ip, ok := rewrite.Value.(net.IP)
+		v := rewrite.Value
+		ip, ok := v.(net.IP)
 		if !ok || ip == nil {
-			return nil, nil
+			return nil, fmt.Errorf("expected ip rewrite value, got %T(%[1]v)", v)
 		}
 
 		res.Rules[0].IP = ip
@@ -255,14 +252,14 @@ func (ss *Default) newResult(
 
 	host := rewrite.NewCNAME
 	if host == "" {
-		return nil, nil
+		return res, nil
 	}
 
 	ss.log(log.DEBUG, "resolving %q", host)
 
 	ips, err := ss.resolver.LookupIP(context.Background(), qtypeToProto(qtype), host)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("resolving cname: %w", err)
 	}
 
 	ss.log(log.DEBUG, "resolved %s", ips)
@@ -276,11 +273,9 @@ func (ss *Default) newResult(
 		}
 
 		res.Rules[0].IP = ip
-
-		return res, nil
 	}
 
-	return nil, nil
+	return res, nil
 }
 
 // qtypeToProto returns "ip4" for [dns.TypeA] and "ip6" for [dns.TypeAAAA].

internal/filtering/safesearch/safesearch_test.go

@@ -1,6 +1,7 @@
 package safesearch_test
 
 import (
+	"context"
 	"net"
 	"testing"
 	"time"
@@ -71,6 +72,25 @@ func TestDefault_CheckHost_yandex(t *testing.T) {
 	}
 }
 
+func TestDefault_CheckHost_yandexAAAA(t *testing.T) {
+	conf := testConf
+	ss, err := safesearch.NewDefault(conf, "", testCacheSize, testCacheTTL)
+	require.NoError(t, err)
+
+	res, err := ss.CheckHost("www.yandex.ru", dns.TypeAAAA)
+	require.NoError(t, err)
+
+	assert.True(t, res.IsFiltered)
+
+	// TODO(a.garipov): Currently, the safe-search filter returns a single rule
+	// with a nil IP address.  This isn't really necessary and should be changed
+	// once the TODO in [safesearch.Default.newResult] is resolved.
+	require.Len(t, res.Rules, 1)
+
+	assert.Nil(t, res.Rules[0].IP)
+	assert.EqualValues(t, filtering.SafeSearchListID, res.Rules[0].FilterListID)
+}
+
 func TestDefault_CheckHost_google(t *testing.T) {
 	resolver := &aghtest.TestResolver{}
 	ip, _ := resolver.HostToIPs("forcesafesearch.google.com")
@@ -105,6 +125,56 @@ func TestDefault_CheckHost_google(t *testing.T) {
 	}
 }
 
+// testResolver is a [filtering.Resolver] for tests.
+//
+// TODO(a.garipov): Move to aghtest and use everywhere.
+type testResolver struct {
+	OnLookupIP func(ctx context.Context, network, host string) (ips []net.IP, err error)
+}
+
+// type check
+var _ filtering.Resolver = (*testResolver)(nil)
+
+// LookupIP implements the [filtering.Resolver] interface for *testResolver.
+func (r *testResolver) LookupIP(
+	ctx context.Context,
+	network string,
+	host string,
+) (ips []net.IP, err error) {
+	return r.OnLookupIP(ctx, network, host)
+}
+
+func TestDefault_CheckHost_duckduckgoAAAA(t *testing.T) {
+	conf := testConf
+	conf.CustomResolver = &testResolver{
+		OnLookupIP: func(_ context.Context, network, host string) (ips []net.IP, err error) {
+			assert.Equal(t, "ip6", network)
+			assert.Equal(t, "safe.duckduckgo.com", host)
+
+			return nil, nil
+		},
+	}
+
+	ss, err := safesearch.NewDefault(conf, "", testCacheSize, testCacheTTL)
+	require.NoError(t, err)
+
+	// The DuckDuckGo safe-search addresses are resolved through CNAMEs, but
+	// DuckDuckGo doesn't have a safe-search IPv6 address.  The result should be
+	// the same as the one for Yandex IPv6.  That is, a NODATA response.
+	res, err := ss.CheckHost("www.duckduckgo.com", dns.TypeAAAA)
+	require.NoError(t, err)
+
+	assert.True(t, res.IsFiltered)
+
+	// TODO(a.garipov): Currently, the safe-search filter returns a single rule
+	// with a nil IP address.  This isn't really necessary and should be changed
+	// once the TODO in [safesearch.Default.newResult] is resolved.
+	require.Len(t, res.Rules, 1)
+
+	assert.Nil(t, res.Rules[0].IP)
+	assert.EqualValues(t, filtering.SafeSearchListID, res.Rules[0].FilterListID)
+}
+
 func TestDefault_Update(t *testing.T) {
 	conf := testConf
 	ss, err := safesearch.NewDefault(conf, "", testCacheSize, testCacheTTL)