diff --git a/CHANGELOG.md b/CHANGELOG.md
index b23ecb4d..44474ab0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,11 +42,14 @@ See also the [v0.107.21 GitHub milestone][ms-v0.107.21].
### Fixed
+- Filters updating strategy, which could sometimes lead to use of broken or
+ incompletely downloaded lists ([#5258]).
- Errors popping up during updates of settings, which could sometimes cause the
server to stop responding ([#5251]).
[#5238]: https://github.com/AdguardTeam/AdGuardHome/issues/5238
[#5251]: https://github.com/AdguardTeam/AdGuardHome/issues/5251
+[#5258]: https://github.com/AdguardTeam/AdGuardHome/issues/5258
[ms-v0.107.21]: https://github.com/AdguardTeam/AdGuardHome/milestone/57?closed=1
diff --git a/client/src/actions/encryption.js b/client/src/actions/encryption.js
index 2f58abd3..670a9f4e 100644
--- a/client/src/actions/encryption.js
+++ b/client/src/actions/encryption.js
@@ -41,6 +41,8 @@ export const setTlsConfig = (config) => async (dispatch, getState) => {
response.certificate_chain = atob(response.certificate_chain);
response.private_key = atob(response.private_key);
+ redirectToCurrentProtocol(response, httpPort);
+
const dnsStatus = await apiClient.getGlobalStatus();
if (dnsStatus) {
dispatch(dnsStatusSuccess(dnsStatus));
@@ -48,7 +50,6 @@ export const setTlsConfig = (config) => async (dispatch, getState) => {
dispatch(setTlsConfigSuccess(response));
dispatch(addSuccessToast('encryption_config_saved'));
- redirectToCurrentProtocol(response, httpPort);
} catch (error) {
dispatch(addErrorToast({ error }));
dispatch(setTlsConfigFailure());
diff --git a/client/src/components/Logs/Filters/Form.js b/client/src/components/Logs/Filters/Form.js
index cf02ba5b..7b250ca5 100644
--- a/client/src/components/Logs/Filters/Form.js
+++ b/client/src/components/Logs/Filters/Form.js
@@ -155,7 +155,7 @@ const Form = (props) => {
name={FORM_NAMES.search}
component={renderFilterField}
type="text"
- className={classNames('form-control--search form-control--transparent', className)}
+ className={classNames('form-control form-control--search form-control--transparent', className)}
placeholder={t('domain_or_client')}
tooltip={t('query_log_strict_search')}
onClearInputClick={onInputClear}
diff --git a/client/src/components/Logs/Logs.css b/client/src/components/Logs/Logs.css
index d365478b..8df1d62b 100644
--- a/client/src/components/Logs/Logs.css
+++ b/client/src/components/Logs/Logs.css
@@ -103,14 +103,12 @@
}
.form-control--search {
- box-shadow: 0 1px 0 #ddd;
padding: 0 2.5rem;
height: 2.25rem;
flex-grow: 1;
}
.form-control--transparent {
- border: 0 solid transparent !important;
background-color: transparent !important;
}
@@ -174,10 +172,8 @@
display: inline-flex;
align-items: center;
justify-content: center;
-
- --size: 2.5rem;
- width: var(--size);
- height: var(--size);
+ width: 2.5rem;
+ height: 2.5rem;
padding: 0;
margin-left: 0.9375rem;
background-color: transparent;
@@ -474,7 +470,7 @@
.filteringRules__filter {
font-style: italic;
- font-weight: normal;
+ font-weight: 400;
margin-bottom: 1rem;
}
diff --git a/client/src/components/Settings/Clients/Form.js b/client/src/components/Settings/Clients/Form.js
index 35d9764d..f6b12d1c 100644
--- a/client/src/components/Settings/Clients/Form.js
+++ b/client/src/components/Settings/Clients/Form.js
@@ -11,12 +11,13 @@ import Select from 'react-select';
import i18n from '../../../i18n';
import Tabs from '../../ui/Tabs';
import Examples from '../Dns/Upstream/Examples';
-import { toggleAllServices } from '../../../helpers/helpers';
+import { toggleAllServices, trimLinesAndRemoveEmpty } from '../../../helpers/helpers';
import {
renderInputField,
renderGroupField,
CheckboxField,
renderServiceField,
+ renderTextareaField,
} from '../../../helpers/form';
import { validateClientId, validateRequiredValue } from '../../../helpers/validators';
import { CLIENT_ID_LINK, FORM_NAME } from '../../../helpers/constants';
@@ -230,10 +231,11 @@ let Form = (props) => {
,
diff --git a/internal/filtering/filter.go b/internal/filtering/filter.go
index 070ce73c..953d105b 100644
--- a/internal/filtering/filter.go
+++ b/internal/filtering/filter.go
@@ -2,6 +2,7 @@ package filtering
import (
"bufio"
+ "bytes"
"fmt"
"hash/crc32"
"io"
@@ -12,6 +13,7 @@ import (
"strings"
"time"
+ "github.com/AdguardTeam/AdGuardHome/internal/aghalg"
"github.com/AdguardTeam/golibs/errors"
"github.com/AdguardTeam/golibs/log"
"github.com/AdguardTeam/golibs/stringutil"
@@ -134,8 +136,8 @@ func (d *DNSFilter) filterSetProperties(
// TODO(e.burkov): The validation of the contents of the new URL is
// currently skipped if the rule list is disabled. This makes it
// possible to set a bad rules source, but the validation should still
- // kick in when the filter is enabled. Consider making changing this
- // behavior to be stricter.
+ // kick in when the filter is enabled. Consider changing this behavior
+ // to be stricter.
filt.unload()
}
@@ -269,10 +271,10 @@ func (d *DNSFilter) periodicallyRefreshFilters() {
// already going on.
//
// TODO(e.burkov): Get rid of the concurrency pattern which requires the
-// sync.Mutex.TryLock.
+// [sync.Mutex.TryLock].
func (d *DNSFilter) tryRefreshFilters(block, allow, force bool) (updated int, isNetworkErr, ok bool) {
if ok = d.refreshLock.TryLock(); !ok {
- return 0, false, ok
+ return 0, false, false
}
defer d.refreshLock.Unlock()
@@ -427,52 +429,124 @@ func (d *DNSFilter) refreshFiltersIntl(block, allow, force bool) (int, bool) {
return updNum, false
}
-// Allows printable UTF-8 text with CR, LF, TAB characters
-func isPrintableText(data []byte, len int) bool {
- for i := 0; i < len; i++ {
- c := data[i]
+// isPrintableText returns true if data is printable UTF-8 text with CR, LF, TAB
+// characters.
+//
+// TODO(e.burkov): Investigate the purpose of this and improve the
+// implementation. Perhaps, use something from the unicode package.
+func isPrintableText(data string) (ok bool) {
+ for _, c := range []byte(data) {
if (c >= ' ' && c != 0x7f) || c == '\n' || c == '\r' || c == '\t' {
continue
}
+
return false
}
+
return true
}
-// A helper function that parses filter contents and returns a number of rules and a filter name (if there's any)
-func (d *DNSFilter) parseFilterContents(file io.Reader) (int, uint32, string) {
- rulesCount := 0
- name := ""
- seenTitle := false
- r := bufio.NewReader(file)
- checksum := uint32(0)
+// scanLinesWithBreak is essentially a [bufio.ScanLines] which keeps trailing
+// line breaks.
+func scanLinesWithBreak(data []byte, atEOF bool) (advance int, token []byte, err error) {
+ if atEOF && len(data) == 0 {
+ return 0, nil, nil
+ }
- for {
- line, err := r.ReadString('\n')
- checksum = crc32.Update(checksum, crc32.IEEETable, []byte(line))
+ if i := bytes.IndexByte(data, '\n'); i >= 0 {
+ return i + 1, data[0 : i+1], nil
+ }
- line = strings.TrimSpace(line)
- if len(line) == 0 {
- //
- } else if line[0] == '!' {
- m := d.filterTitleRegexp.FindAllStringSubmatch(line, -1)
- if len(m) > 0 && len(m[0]) >= 2 && !seenTitle {
- name = m[0][1]
- seenTitle = true
- }
+ if atEOF {
+ return len(data), data, nil
+ }
- } else if line[0] == '#' {
- //
- } else {
- rulesCount++
+ // Request more data.
+ return 0, nil, nil
+}
+
+// parseFilter copies filter's content from src to dst and returns the number of
+// rules, name, number of bytes written, checksum, and title of the parsed list.
+// dst must not be nil.
+func (d *DNSFilter) parseFilter(
+ src io.Reader,
+ dst io.Writer,
+) (rulesNum, written int, checksum uint32, title string, err error) {
+ scanner := bufio.NewScanner(src)
+ scanner.Split(scanLinesWithBreak)
+
+ titleFound := false
+ for n := 0; scanner.Scan(); written += n {
+ line := scanner.Text()
+ var isRule bool
+ var likelyTitle string
+ isRule, likelyTitle, err = d.parseFilterLine(line, !titleFound, written == 0)
+ if err != nil {
+ return 0, written, 0, "", err
}
+ if isRule {
+ rulesNum++
+ } else if likelyTitle != "" {
+ title, titleFound = likelyTitle, true
+ }
+
+ checksum = crc32.Update(checksum, crc32.IEEETable, []byte(line))
+
+ n, err = dst.Write([]byte(line))
if err != nil {
- break
+ return 0, written, 0, "", fmt.Errorf("writing filter line: %w", err)
}
}
- return rulesCount, checksum, name
+ if err = scanner.Err(); err != nil {
+ return 0, written, 0, "", fmt.Errorf("scanning filter contents: %w", err)
+ }
+
+ return rulesNum, written, checksum, title, nil
+}
+
+// parseFilterLine returns true if the passed line is a rule. line is
+// considered a rule if it's not a comment and contains no title.
+func (d *DNSFilter) parseFilterLine(
+ line string,
+ lookForTitle bool,
+ testHTML bool,
+) (isRule bool, title string, err error) {
+ if !isPrintableText(line) {
+ return false, "", errors.Error("filter contains non-printable characters")
+ }
+
+ line = strings.TrimSpace(line)
+ if line == "" || line[0] == '#' {
+ return false, "", nil
+ }
+
+ if testHTML && isHTML(line) {
+ return false, "", errors.Error("data is HTML, not plain text")
+ }
+
+ if line[0] == '!' && lookForTitle {
+ match := d.filterTitleRegexp.FindStringSubmatch(line)
+ if len(match) > 1 {
+ title = match[1]
+ }
+
+ return false, title, nil
+ }
+
+ return true, "", nil
+}
+
+// isHTML returns true if the line contains HTML tags instead of plain text.
+// line shouldn have no leading space symbols.
+//
+// TODO(ameshkov): It actually gives too much false-positives. Perhaps, just
+// check if trimmed string begins with angle bracket.
+func isHTML(line string) (ok bool) {
+ line = strings.ToLower(line)
+
+ return strings.HasPrefix(line, "`),
}} {
- ipp := serveFiltersLocally(t, rulesSource.content)
- *rulesSource.endpoint = (&url.URL{
- Scheme: "http",
- Host: ipp.String(),
- }).String()
+ *rulesSource.endpoint = serveFiltersLocally(t, rulesSource.content)
}
testCases := []struct {