From db52f7a3aca54dcea286a0b151fcca38b0fb8e8b Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Wed, 3 May 2023 19:52:06 +0300 Subject: [PATCH] Pull request 1841: AG-21462-safebrowsing-parental-http-tests Merge in DNS/adguard-home from AG-21462-safebrowsing-parental-http-tests to master Squashed commit of the following: commit 22a83ebad08a27939a443530137a7c195f512ee4 Author: Stanislav Chzhen Date: Wed May 3 17:39:46 2023 +0300 filtering: fix test commit c3ca8b4987245cdd552f6f09759804e716bcae80 Author: Stanislav Chzhen Date: Wed May 3 16:43:35 2023 +0300 filtering: imp tests even more commit 7643bfae350373b5b6dfb61b64e57da66c6ab952 Author: Stanislav Chzhen Date: Wed May 3 16:17:42 2023 +0300 filtering: imp tests more commit 399c05ee4d479a727b61378b7a07158a568d0181 Author: Stanislav Chzhen Date: Wed May 3 14:45:41 2023 +0300 filtering: imp tests commit f361df39e784ec9c5191666736a6c64b332928e8 Author: Stanislav Chzhen Date: Tue May 2 12:49:26 2023 +0300 filtering: add tests --- internal/filtering/filtering.go | 66 +++++++++++ internal/filtering/http.go | 75 +++++++++++++ internal/filtering/http_test.go | 169 +++++++++++++++++++++++++++++ internal/filtering/safebrowsing.go | 137 ----------------------- 4 files changed, 310 insertions(+), 137 deletions(-) delete mode 100644 internal/filtering/safebrowsing.go diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go index 7b8ddfa9..5c30c645 100644 --- a/internal/filtering/filtering.go +++ b/internal/filtering/filtering.go @@ -1035,3 +1035,69 @@ func (d *DNSFilter) Start() { // So for now we just start this periodic task from here. go d.periodicallyRefreshFilters() } + +// Safe browsing and parental control methods. + +// TODO(a.garipov): Unify with checkParental. +func (d *DNSFilter) checkSafeBrowsing( + host string, + _ uint16, + setts *Settings, +) (res Result, err error) { + if !setts.ProtectionEnabled || !setts.SafeBrowsingEnabled { + return Result{}, nil + } + + if log.GetLevel() >= log.DEBUG { + timer := log.StartTimer() + defer timer.LogElapsed("safebrowsing lookup for %q", host) + } + + res = Result{ + Rules: []*ResultRule{{ + Text: "adguard-malware-shavar", + FilterListID: SafeBrowsingListID, + }}, + Reason: FilteredSafeBrowsing, + IsFiltered: true, + } + + block, err := d.safeBrowsingChecker.Check(host) + if !block || err != nil { + return Result{}, err + } + + return res, nil +} + +// TODO(a.garipov): Unify with checkSafeBrowsing. +func (d *DNSFilter) checkParental( + host string, + _ uint16, + setts *Settings, +) (res Result, err error) { + if !setts.ProtectionEnabled || !setts.ParentalEnabled { + return Result{}, nil + } + + if log.GetLevel() >= log.DEBUG { + timer := log.StartTimer() + defer timer.LogElapsed("parental lookup for %q", host) + } + + res = Result{ + Rules: []*ResultRule{{ + Text: "parental CATEGORY_BLACKLISTED", + FilterListID: ParentalListID, + }}, + Reason: FilteredParental, + IsFiltered: true, + } + + block, err := d.parentalControlChecker.Check(host) + if !block || err != nil { + return Result{}, err + } + + return res, nil +} diff --git a/internal/filtering/http.go b/internal/filtering/http.go index 11f64283..e114be33 100644 --- a/internal/filtering/http.go +++ b/internal/filtering/http.go @@ -8,6 +8,7 @@ import ( "net/url" "os" "path/filepath" + "sync" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" @@ -458,6 +459,80 @@ func (d *DNSFilter) handleCheckHost(w http.ResponseWriter, r *http.Request) { _ = aghhttp.WriteJSONResponse(w, r, resp) } +// setProtectedBool sets the value of a boolean pointer under a lock. l must +// protect the value under ptr. +// +// TODO(e.burkov): Make it generic? +func setProtectedBool(mu *sync.RWMutex, ptr *bool, val bool) { + mu.Lock() + defer mu.Unlock() + + *ptr = val +} + +// protectedBool gets the value of a boolean pointer under a read lock. l must +// protect the value under ptr. +// +// TODO(e.burkov): Make it generic? +func protectedBool(mu *sync.RWMutex, ptr *bool) (val bool) { + mu.RLock() + defer mu.RUnlock() + + return *ptr +} + +// handleSafeBrowsingEnable is the handler for the POST +// /control/safebrowsing/enable HTTP API. +func (d *DNSFilter) handleSafeBrowsingEnable(w http.ResponseWriter, r *http.Request) { + setProtectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled, true) + d.Config.ConfigModified() +} + +// handleSafeBrowsingDisable is the handler for the POST +// /control/safebrowsing/disable HTTP API. +func (d *DNSFilter) handleSafeBrowsingDisable(w http.ResponseWriter, r *http.Request) { + setProtectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled, false) + d.Config.ConfigModified() +} + +// handleSafeBrowsingStatus is the handler for the GET +// /control/safebrowsing/status HTTP API. +func (d *DNSFilter) handleSafeBrowsingStatus(w http.ResponseWriter, r *http.Request) { + resp := &struct { + Enabled bool `json:"enabled"` + }{ + Enabled: protectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled), + } + + _ = aghhttp.WriteJSONResponse(w, r, resp) +} + +// handleParentalEnable is the handler for the POST /control/parental/enable +// HTTP API. +func (d *DNSFilter) handleParentalEnable(w http.ResponseWriter, r *http.Request) { + setProtectedBool(&d.confLock, &d.Config.ParentalEnabled, true) + d.Config.ConfigModified() +} + +// handleParentalDisable is the handler for the POST /control/parental/disable +// HTTP API. +func (d *DNSFilter) handleParentalDisable(w http.ResponseWriter, r *http.Request) { + setProtectedBool(&d.confLock, &d.Config.ParentalEnabled, false) + d.Config.ConfigModified() +} + +// handleParentalStatus is the handler for the GET /control/parental/status +// HTTP API. +func (d *DNSFilter) handleParentalStatus(w http.ResponseWriter, r *http.Request) { + resp := &struct { + Enabled bool `json:"enabled"` + }{ + Enabled: protectedBool(&d.confLock, &d.Config.ParentalEnabled), + } + + _ = aghhttp.WriteJSONResponse(w, r, resp) +} + // RegisterFilteringHandlers - register handlers func (d *DNSFilter) RegisterFilteringHandlers() { registerHTTP := d.HTTPRegister diff --git a/internal/filtering/http_test.go b/internal/filtering/http_test.go index df09c3f9..8330dac6 100644 --- a/internal/filtering/http_test.go +++ b/internal/filtering/http_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -136,3 +137,171 @@ func TestDNSFilter_handleFilteringSetURL(t *testing.T) { }) } } + +func TestDNSFilter_handleSafeBrowsingStatus(t *testing.T) { + const ( + testTimeout = time.Second + statusURL = "/control/safebrowsing/status" + ) + + confModCh := make(chan struct{}) + filtersDir := t.TempDir() + + testCases := []struct { + name string + url string + enabled bool + wantStatus assert.BoolAssertionFunc + }{{ + name: "enable_off", + url: "/control/safebrowsing/enable", + enabled: false, + wantStatus: assert.True, + }, { + name: "enable_on", + url: "/control/safebrowsing/enable", + enabled: true, + wantStatus: assert.True, + }, { + name: "disable_on", + url: "/control/safebrowsing/disable", + enabled: true, + wantStatus: assert.False, + }, { + name: "disable_off", + url: "/control/safebrowsing/disable", + enabled: false, + wantStatus: assert.False, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + handlers := make(map[string]http.Handler) + + d, err := New(&Config{ + ConfigModified: func() { + testutil.RequireSend(testutil.PanicT{}, confModCh, struct{}{}, testTimeout) + }, + DataDir: filtersDir, + HTTPRegister: func(_, url string, handler http.HandlerFunc) { + handlers[url] = handler + }, + SafeBrowsingEnabled: tc.enabled, + }, nil) + require.NoError(t, err) + t.Cleanup(d.Close) + + d.RegisterFilteringHandlers() + require.NotEmpty(t, handlers) + require.Contains(t, handlers, statusURL) + + r := httptest.NewRequest(http.MethodPost, tc.url, nil) + w := httptest.NewRecorder() + + go handlers[tc.url].ServeHTTP(w, r) + + testutil.RequireReceive(t, confModCh, testTimeout) + + r = httptest.NewRequest(http.MethodGet, statusURL, nil) + w = httptest.NewRecorder() + + handlers[statusURL].ServeHTTP(w, r) + require.Equal(t, http.StatusOK, w.Code) + + status := struct { + Enabled bool `json:"enabled"` + }{ + Enabled: false, + } + + err = json.NewDecoder(w.Body).Decode(&status) + require.NoError(t, err) + + tc.wantStatus(t, status.Enabled) + }) + } +} + +func TestDNSFilter_handleParentalStatus(t *testing.T) { + const ( + testTimeout = time.Second + statusURL = "/control/parental/status" + ) + + confModCh := make(chan struct{}) + filtersDir := t.TempDir() + + testCases := []struct { + name string + url string + enabled bool + wantStatus assert.BoolAssertionFunc + }{{ + name: "enable_off", + url: "/control/parental/enable", + enabled: false, + wantStatus: assert.True, + }, { + name: "enable_on", + url: "/control/parental/enable", + enabled: true, + wantStatus: assert.True, + }, { + name: "disable_on", + url: "/control/parental/disable", + enabled: true, + wantStatus: assert.False, + }, { + name: "disable_off", + url: "/control/parental/disable", + enabled: false, + wantStatus: assert.False, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + handlers := make(map[string]http.Handler) + + d, err := New(&Config{ + ConfigModified: func() { + testutil.RequireSend(testutil.PanicT{}, confModCh, struct{}{}, testTimeout) + }, + DataDir: filtersDir, + HTTPRegister: func(_, url string, handler http.HandlerFunc) { + handlers[url] = handler + }, + ParentalEnabled: tc.enabled, + }, nil) + require.NoError(t, err) + t.Cleanup(d.Close) + + d.RegisterFilteringHandlers() + require.NotEmpty(t, handlers) + require.Contains(t, handlers, statusURL) + + r := httptest.NewRequest(http.MethodPost, tc.url, nil) + w := httptest.NewRecorder() + + go handlers[tc.url].ServeHTTP(w, r) + + testutil.RequireReceive(t, confModCh, testTimeout) + + r = httptest.NewRequest(http.MethodGet, statusURL, nil) + w = httptest.NewRecorder() + + handlers[statusURL].ServeHTTP(w, r) + require.Equal(t, http.StatusOK, w.Code) + + status := struct { + Enabled bool `json:"enabled"` + }{ + Enabled: false, + } + + err = json.NewDecoder(w.Body).Decode(&status) + require.NoError(t, err) + + tc.wantStatus(t, status.Enabled) + }) + } +} diff --git a/internal/filtering/safebrowsing.go b/internal/filtering/safebrowsing.go deleted file mode 100644 index 5c291159..00000000 --- a/internal/filtering/safebrowsing.go +++ /dev/null @@ -1,137 +0,0 @@ -package filtering - -import ( - "net/http" - "sync" - - "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" - "github.com/AdguardTeam/golibs/log" -) - -// Safe browsing and parental control methods. - -// TODO(a.garipov): Unify with checkParental. -func (d *DNSFilter) checkSafeBrowsing( - host string, - _ uint16, - setts *Settings, -) (res Result, err error) { - if !setts.ProtectionEnabled || !setts.SafeBrowsingEnabled { - return Result{}, nil - } - - if log.GetLevel() >= log.DEBUG { - timer := log.StartTimer() - defer timer.LogElapsed("safebrowsing lookup for %q", host) - } - - res = Result{ - Rules: []*ResultRule{{ - Text: "adguard-malware-shavar", - FilterListID: SafeBrowsingListID, - }}, - Reason: FilteredSafeBrowsing, - IsFiltered: true, - } - - block, err := d.safeBrowsingChecker.Check(host) - if !block || err != nil { - return Result{}, err - } - - return res, nil -} - -// TODO(a.garipov): Unify with checkSafeBrowsing. -func (d *DNSFilter) checkParental( - host string, - _ uint16, - setts *Settings, -) (res Result, err error) { - if !setts.ProtectionEnabled || !setts.ParentalEnabled { - return Result{}, nil - } - - if log.GetLevel() >= log.DEBUG { - timer := log.StartTimer() - defer timer.LogElapsed("parental lookup for %q", host) - } - - res = Result{ - Rules: []*ResultRule{{ - Text: "parental CATEGORY_BLACKLISTED", - FilterListID: ParentalListID, - }}, - Reason: FilteredParental, - IsFiltered: true, - } - - block, err := d.parentalControlChecker.Check(host) - if !block || err != nil { - return Result{}, err - } - - return res, nil -} - -// setProtectedBool sets the value of a boolean pointer under a lock. l must -// protect the value under ptr. -// -// TODO(e.burkov): Make it generic? -func setProtectedBool(mu *sync.RWMutex, ptr *bool, val bool) { - mu.Lock() - defer mu.Unlock() - - *ptr = val -} - -// protectedBool gets the value of a boolean pointer under a read lock. l must -// protect the value under ptr. -// -// TODO(e.burkov): Make it generic? -func protectedBool(mu *sync.RWMutex, ptr *bool) (val bool) { - mu.RLock() - defer mu.RUnlock() - - return *ptr -} - -func (d *DNSFilter) handleSafeBrowsingEnable(w http.ResponseWriter, r *http.Request) { - setProtectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled, true) - d.Config.ConfigModified() -} - -func (d *DNSFilter) handleSafeBrowsingDisable(w http.ResponseWriter, r *http.Request) { - setProtectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled, false) - d.Config.ConfigModified() -} - -func (d *DNSFilter) handleSafeBrowsingStatus(w http.ResponseWriter, r *http.Request) { - resp := &struct { - Enabled bool `json:"enabled"` - }{ - Enabled: protectedBool(&d.confLock, &d.Config.SafeBrowsingEnabled), - } - - _ = aghhttp.WriteJSONResponse(w, r, resp) -} - -func (d *DNSFilter) handleParentalEnable(w http.ResponseWriter, r *http.Request) { - setProtectedBool(&d.confLock, &d.Config.ParentalEnabled, true) - d.Config.ConfigModified() -} - -func (d *DNSFilter) handleParentalDisable(w http.ResponseWriter, r *http.Request) { - setProtectedBool(&d.confLock, &d.Config.ParentalEnabled, false) - d.Config.ConfigModified() -} - -func (d *DNSFilter) handleParentalStatus(w http.ResponseWriter, r *http.Request) { - resp := &struct { - Enabled bool `json:"enabled"` - }{ - Enabled: protectedBool(&d.confLock, &d.Config.ParentalEnabled), - } - - _ = aghhttp.WriteJSONResponse(w, r, resp) -}