diff --git a/CHANGELOG.md b/CHANGELOG.md index 12ac29a646e..dfa362d5152 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,11 @@ and this project adheres to @@ -82,7 +82,8 @@ In this release, the schema version has changed from 20 to 21. ### Fixed -- Safe Search not working with `AAAA` queries for Yandex domains ([#5913]). +- Safe Search not working with `AAAA` queries for domains that don't have `AAAA` + records ([#5913]). [#951]: https://github.com/AdguardTeam/AdGuardHome/issues/951 [#1577]: https://github.com/AdguardTeam/AdGuardHome/issues/1577 diff --git a/internal/filtering/safesearch/safesearch.go b/internal/filtering/safesearch/safesearch.go index c08e57bb024..9d5b5121f84 100644 --- a/internal/filtering/safesearch/safesearch.go +++ b/internal/filtering/safesearch/safesearch.go @@ -259,7 +259,7 @@ func (ss *Default) newResult( 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) diff --git a/internal/filtering/safesearch/safesearch_test.go b/internal/filtering/safesearch/safesearch_test.go index b0775c60d2a..12860c5d3c5 100644 --- a/internal/filtering/safesearch/safesearch_test.go +++ b/internal/filtering/safesearch/safesearch_test.go @@ -1,6 +1,7 @@ package safesearch_test import ( + "context" "net" "testing" "time" @@ -80,6 +81,14 @@ func TestDefault_CheckHost_yandexAAAA(t *testing.T) { 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) { @@ -116,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)