From c54635e8a166f0c7f9d50d12f96f981c2a2a15f6 Mon Sep 17 00:00:00 2001
From: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Fri, 11 Aug 2023 13:55:49 +0300
Subject: [PATCH] Pull request: 6020-rulelist-name
Updates #6020.
Squashed commit of the following:
commit fedb9415fb40d103261ca9b966c3d634692f899d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Fri Aug 11 12:45:06 2023 +0300
filtering: imp tests
commit d85d193ca7808e9089fa8ac3b26652f9c88c44ad
Merge: f1c1eddc1 94cf50a53
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Fri Aug 11 11:07:39 2023 +0300
Merge remote-tracking branch 'origin/master' into 6020-rulelist-name
commit f1c1eddc113d2659adb666d7849ce0830eaf71f0
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Fri Aug 11 10:59:07 2023 +0300
filtering: imp tests
commit 39e9d546dc2438409607ffebe414e9d656275504
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Fri Aug 11 10:02:48 2023 +0300
filtering: imp code
commit 230f15ddad95c670e93c58db6d9928c3d0e0b79b
Merge: 1940fb397 111005b8d
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu Aug 10 15:03:08 2023 +0300
Merge remote-tracking branch 'origin/master' into 6020-rulelist-name
commit 1940fb3973344a7d1ab8acfdc9401ed41fe0e666
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu Aug 10 15:01:57 2023 +0300
all: docs
commit 810f6d17968873ce489b2e24f496d31179675e37
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu Aug 10 15:00:59 2023 +0300
filtering: imp code
commit f310dd2281dc81cd816701696cf1bb289b4fb708
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu Aug 10 12:19:55 2023 +0300
client: flt name
commit 9494771c57c464dbe5117315efdb3104b977bac4
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu Aug 10 12:18:57 2023 +0300
filtering: flt name
---
CHANGELOG.md | 7 +-
client/src/components/Filters/Form.js | 1 -
internal/filtering/filter.go | 20 +++-
internal/filtering/filter_test.go | 129 ++++++++++++++++++--------
4 files changed, 114 insertions(+), 43 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cf360a41..82d58a69 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,7 +25,9 @@ NOTE: Add new changes BELOW THIS COMMENT.
### Added
-- The ability to filter DNS HTTPS records including IPv4/v6 hints. ([#6053]).
+- While adding or updating blocklists, the title can now be parsed from
+ `! Title:` definition of the blocklist's source ([#6020]).
+- The ability to filter DNS HTTPS records including IPv4/v6 hints ([#6053]).
- Two new metrics showing total number of responses from each upstream DNS
server and their average processing time in the Web UI ([#1453]).
- The ability to set the port for the `pprof` debug API, see configuration
@@ -35,7 +37,7 @@ NOTE: Add new changes BELOW THIS COMMENT.
- For non-A and non-AAAA requests, which has been filtered, the NODATA response
is returned if the blocking mode isn't set to `Null IP`. In previous versions
- it returned NXDOMAIN response in such cases.
+ it returned NXDOMAIN response in such cases.
#### Configuration Changes
@@ -68,6 +70,7 @@ In this release, the schema version has changed from 24 to 25.
[#1453]: https://github.com/AdguardTeam/AdGuardHome/issues/1453
[#5948]: https://github.com/AdguardTeam/AdGuardHome/issues/5948
+[#6020]: https://github.com/AdguardTeam/AdGuardHome/issues/6020
[#6053]: https://github.com/AdguardTeam/AdGuardHome/issues/6053
<!--
diff --git a/client/src/components/Filters/Form.js b/client/src/components/Filters/Form.js
index f4e902f5..5619580b 100644
--- a/client/src/components/Filters/Form.js
+++ b/client/src/components/Filters/Form.js
@@ -134,7 +134,6 @@ const Form = (props) => {
component={renderInputField}
className="form-control"
placeholder={t('enter_name_hint')}
- validate={[validateRequiredValue]}
normalizeOnBlur={(data) => data.trim()}
/>
</div>
diff --git a/internal/filtering/filter.go b/internal/filtering/filter.go
index 88e8a0fc..0d476802 100644
--- a/internal/filtering/filter.go
+++ b/internal/filtering/filter.go
@@ -10,7 +10,6 @@ import (
"strings"
"time"
- "github.com/AdguardTeam/AdGuardHome/internal/aghalg"
"github.com/AdguardTeam/AdGuardHome/internal/aghrenameio"
"github.com/AdguardTeam/AdGuardHome/internal/filtering/rulelist"
"github.com/AdguardTeam/golibs/errors"
@@ -54,6 +53,22 @@ func (filter *FilterYAML) Path(dataDir string) string {
return filepath.Join(dataDir, filterDir, strconv.FormatInt(filter.ID, 10)+".txt")
}
+// ensureName sets provided title or default name for the filter if it doesn't
+// have name already.
+func (filter *FilterYAML) ensureName(title string) {
+ if filter.Name != "" {
+ return
+ }
+
+ if title != "" {
+ filter.Name = title
+
+ return
+ }
+
+ filter.Name = fmt.Sprintf("List %d", filter.ID)
+}
+
const (
// errFilterNotExist is returned from [filterSetProperties] when there are
// no lists with the desired URL to update.
@@ -527,7 +542,7 @@ func (d *DNSFilter) finalizeUpdate(
rulesCount := res.RulesCount
log.Info("filtering: updated filter %d: %d bytes, %d rules", id, res.BytesWritten, rulesCount)
- flt.Name = aghalg.Coalesce(flt.Name, res.Title)
+ flt.ensureName(res.Title)
flt.checksum = res.Checksum
flt.RulesCount = rulesCount
@@ -601,6 +616,7 @@ func (d *DNSFilter) load(flt *FilterYAML) (err error) {
return fmt.Errorf("parsing filter file: %w", err)
}
+ flt.ensureName(res.Title)
flt.RulesCount, flt.checksum, flt.LastUpdated = res.RulesCount, res.Checksum, st.ModTime()
return nil
diff --git a/internal/filtering/filter_test.go b/internal/filtering/filter_test.go
index 53e846fc..e613f1b3 100644
--- a/internal/filtering/filter_test.go
+++ b/internal/filtering/filter_test.go
@@ -1,7 +1,6 @@
package filtering
import (
- "io/fs"
"net"
"net/http"
"net/url"
@@ -16,6 +15,9 @@ import (
"github.com/stretchr/testify/require"
)
+// testTimeout is the common timeout for tests.
+const testTimeout = 5 * time.Second
+
// serveHTTPLocally starts a new HTTP server, that handles its index with h. It
// also gracefully closes the listener when the test under t finishes.
func serveHTTPLocally(t *testing.T, h http.Handler) (urlStr string) {
@@ -50,7 +52,49 @@ func serveFiltersLocally(t *testing.T, fltContent []byte) (urlStr string) {
}))
}
-func TestFilters(t *testing.T) {
+// updateAndAssert loads filter content from its URL and then asserts rules
+// count.
+func updateAndAssert(
+ t *testing.T,
+ dnsFilter *DNSFilter,
+ f *FilterYAML,
+ wantUpd require.BoolAssertionFunc,
+ wantRulesCount int,
+) {
+ t.Helper()
+
+ ok, err := dnsFilter.update(f)
+ require.NoError(t, err)
+ wantUpd(t, ok)
+
+ assert.Equal(t, wantRulesCount, f.RulesCount)
+
+ dir, err := os.ReadDir(filepath.Join(dnsFilter.DataDir, filterDir))
+ require.NoError(t, err)
+ require.FileExists(t, f.Path(dnsFilter.DataDir))
+
+ assert.Len(t, dir, 1)
+
+ err = dnsFilter.load(f)
+ require.NoError(t, err)
+}
+
+// newDNSFilter returns a new properly initialized DNS filter instance.
+func newDNSFilter(t *testing.T) (d *DNSFilter) {
+ t.Helper()
+
+ dnsFilter, err := New(&Config{
+ DataDir: t.TempDir(),
+ HTTPClient: &http.Client{
+ Timeout: testTimeout,
+ },
+ }, nil)
+ require.NoError(t, err)
+
+ return dnsFilter
+}
+
+func TestDNSFilter_Update(t *testing.T) {
const content = `||example.org^$third-party
# Inline comment example
||example.com^$third-party
@@ -58,49 +102,20 @@ func TestFilters(t *testing.T) {
`
fltContent := []byte(content)
-
addr := serveFiltersLocally(t, fltContent)
-
- tempDir := t.TempDir()
-
- filters, err := New(&Config{
- DataDir: tempDir,
- HTTPClient: &http.Client{
- Timeout: 5 * time.Second,
- },
- }, nil)
- require.NoError(t, err)
-
f := &FilterYAML{
- URL: addr,
+ URL: addr,
+ Name: "test-filter",
}
- updateAndAssert := func(t *testing.T, want require.BoolAssertionFunc, wantRulesCount int) {
- var ok bool
- ok, err = filters.update(f)
- require.NoError(t, err)
- want(t, ok)
-
- assert.Equal(t, wantRulesCount, f.RulesCount)
-
- var dir []fs.DirEntry
- dir, err = os.ReadDir(filepath.Join(tempDir, filterDir))
- require.NoError(t, err)
-
- assert.Len(t, dir, 1)
-
- require.FileExists(t, f.Path(tempDir))
-
- err = filters.load(f)
- require.NoError(t, err)
- }
+ dnsFilter := newDNSFilter(t)
t.Run("download", func(t *testing.T) {
- updateAndAssert(t, require.True, 3)
+ updateAndAssert(t, dnsFilter, f, require.True, 3)
})
t.Run("refresh_idle", func(t *testing.T) {
- updateAndAssert(t, require.False, 3)
+ updateAndAssert(t, dnsFilter, f, require.False, 3)
})
t.Run("refresh_actually", func(t *testing.T) {
@@ -110,13 +125,51 @@ func TestFilters(t *testing.T) {
f.URL = serveFiltersLocally(t, anotherContent)
t.Cleanup(func() { f.URL = oldURL })
- updateAndAssert(t, require.True, 1)
+ updateAndAssert(t, dnsFilter, f, require.True, 1)
})
t.Run("load_unload", func(t *testing.T) {
- err = filters.load(f)
+ err := dnsFilter.load(f)
require.NoError(t, err)
f.unload()
})
}
+
+func TestFilterYAML_EnsureName(t *testing.T) {
+ dnsFilter := newDNSFilter(t)
+
+ t.Run("title_custom", func(t *testing.T) {
+ content := []byte("! Title: src-title\n||example.com^")
+
+ f := &FilterYAML{
+ URL: serveFiltersLocally(t, content),
+ Name: "user-custom",
+ }
+
+ updateAndAssert(t, dnsFilter, f, require.True, 1)
+ assert.Equal(t, "user-custom", f.Name)
+ })
+
+ t.Run("title_from_src", func(t *testing.T) {
+ content := []byte("! Title: src-title\n||example.com^")
+
+ f := &FilterYAML{
+ URL: serveFiltersLocally(t, content),
+ }
+
+ updateAndAssert(t, dnsFilter, f, require.True, 1)
+ assert.Equal(t, "src-title", f.Name)
+ })
+
+ t.Run("title_default", func(t *testing.T) {
+ content := []byte("||example.com^")
+
+ f := &FilterYAML{
+ URL: serveFiltersLocally(t, content),
+ }
+
+ updateAndAssert(t, dnsFilter, f, require.True, 1)
+ assert.Equal(t, "List 0", f.Name)
+ })
+}