From 16e5e09c2ec3e45a88973731732984ccd020ab41 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Tue, 29 Jun 2021 13:36:52 +0300 Subject: [PATCH] Pull request: 3013 querylog idna Merge in DNS/adguard-home from 3013-idna to master Closes #3013. Squashed commit of the following: commit 567d4c3beef3cf3ee995ad9d8c3aba6616c74c6c Author: Eugene Burkov Date: Tue Jun 29 13:11:10 2021 +0300 client: mv punycode label commit 6585dcaece9f590d7f02afb5aa25953ab0c2555b Author: Ildar Kamalov Date: Tue Jun 29 12:32:40 2021 +0300 client: handle unicode name commit c0f61bfbb9bdf919be7b07c112c4b7a52f3ad6a1 Author: Eugene Burkov Date: Mon Jun 28 20:00:57 2021 +0300 all: imp log of changes commit 41388abc8770ce164bcba327fcf0013133b5e6f7 Author: Eugene Burkov Date: Mon Jun 28 19:52:23 2021 +0300 scripts: imp hooks commit 9c4ba933fbd9340e1de061d4f451218238650c0f Author: Eugene Burkov Date: Mon Jun 28 19:47:27 2021 +0300 all: imp code, docs commit 61bd6d6f926480cb8c2f9bd3cd2b61e1532f71cf Author: Eugene Burkov Date: Mon Jun 28 16:09:25 2021 +0300 querylog: add ascii hostname, convert to unicode --- CHANGELOG.md | 3 + client/src/__locales/en.json | 1 + .../src/components/Logs/Cells/DomainCell.js | 119 +++++++++++++----- client/src/helpers/helpers.js | 3 +- internal/querylog/http.go | 4 +- internal/querylog/json.go | 21 +++- internal/querylog/qlog_test.go | 8 +- internal/querylog/searchcriterion.go | 12 +- openapi/CHANGELOG.md | 11 ++ openapi/openapi.yaml | 7 +- scripts/hooks/pre-commit | 2 +- 11 files changed, 139 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f49e0e22..5607b6e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to ### Changed +- Internationalized domains are now shown decoded in the query log with the + original encoded version shown in request details. ([#3013]). - When /etc/hosts-type rules have several IPs for one host, all IPs are now returned instead of only the first one ([#1381]). - The setting `rlimit_nofile` is now in the `os` block of the configuration @@ -79,6 +81,7 @@ released by then. [#2441]: https://github.com/AdguardTeam/AdGuardHome/issues/2441 [#2443]: https://github.com/AdguardTeam/AdGuardHome/issues/2443 [#2763]: https://github.com/AdguardTeam/AdGuardHome/issues/2763 +[#3013]: https://github.com/AdguardTeam/AdGuardHome/issues/3013 [#3136]: https://github.com/AdguardTeam/AdGuardHome/issues/3136 [#3166]: https://github.com/AdguardTeam/AdGuardHome/issues/3166 [#3172]: https://github.com/AdguardTeam/AdGuardHome/issues/3172 diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index a515f532..48cfe096 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -488,6 +488,7 @@ "interval_days": "{{count}} day", "interval_days_plural": "{{count}} days", "domain": "Domain", + "punycode": "Punycode", "answer": "Answer", "filter_added_successfully": "The list has been successfully added", "filter_removed_successfully": "The list has been successfully removed", diff --git a/client/src/components/Logs/Cells/DomainCell.js b/client/src/components/Logs/Cells/DomainCell.js index 4333089c..5ab18280 100644 --- a/client/src/components/Logs/Cells/DomainCell.js +++ b/client/src/components/Logs/Cells/DomainCell.js @@ -16,6 +16,7 @@ const DomainCell = ({ answer_dnssec, client_proto, domain, + unicodeName, time, tracker, type, @@ -41,10 +42,22 @@ const DomainCell = ({ const protocol = t(SCHEME_TO_PROTOCOL_MAP[client_proto]) || ''; const ip = type ? `${t('type_table_header')}: ${type}` : ''; - const requestDetailsObj = { + let requestDetailsObj = { time_table_header: formatTime(time, LONG_TIME_FORMAT), date: formatDateTime(time, DEFAULT_SHORT_DATE_FORMAT_OPTIONS), domain, + }; + + if (domain && unicodeName) { + requestDetailsObj = { + ...requestDetailsObj, + domain: unicodeName, + punycode: domain, + }; + } + + requestDetailsObj = { + ...requestDetailsObj, type_table_header: type, protocol, }; @@ -54,23 +67,40 @@ const DomainCell = ({ const knownTrackerDataObj = { name_table_header: tracker?.name, category_label: hasTracker && captitalizeWords(tracker.category), - source_label: sourceData - && {sourceData.name}, + source_label: sourceData && ( + + {sourceData.name} + + ), }; const renderGrid = (content, idx) => { const preparedContent = typeof content === 'string' ? t(content) : content; - const className = classNames('text-truncate o-hidden', { - 'overflow-break': preparedContent.length > 100, - }); - return
{preparedContent}
; + + const className = classNames( + 'text-truncate o-hidden', + { 'overflow-break': preparedContent?.length > 100 }, + ); + + return ( +
+ {preparedContent} +
+ ); }; const getGrid = (contentObj, title, className) => [ -
{t(title)}
, -
{React.Children.map(Object.entries(contentObj), renderGrid)}
, +
+ {t(title)} +
, +
+ {React.Children.map(Object.entries(contentObj), renderGrid)} +
, ]; const requestDetails = getGrid(requestDetailsObj, 'request_details'); @@ -81,35 +111,60 @@ const DomainCell = ({ 'px-2 d-flex justify-content-center flex-column': isDetailed, }); - const details = [ip, protocol].filter(Boolean) - .join(', '); + const details = [ip, protocol].filter(Boolean).join(', '); - return
- {dnssec_enabled && } - -
-
{domain}
- {details && isDetailed - &&
{details}
} + return ( +
+ {dnssec_enabled && ( + + )} + +
+ {unicodeName ? ( +
+ {unicodeName} +
+ ) : ( +
+ {domain} +
+ )} + {details && isDetailed && ( +
+ {details} +
+ )} +
-
; + ); }; DomainCell.propTypes = { answer_dnssec: propTypes.bool.isRequired, client_proto: propTypes.string.isRequired, domain: propTypes.string.isRequired, + unicodeName: propTypes.string, time: propTypes.string.isRequired, type: propTypes.string.isRequired, tracker: propTypes.object, diff --git a/client/src/helpers/helpers.js b/client/src/helpers/helpers.js index f7e20fcc..19ed0b70 100644 --- a/client/src/helpers/helpers.js +++ b/client/src/helpers/helpers.js @@ -77,7 +77,7 @@ export const normalizeLogs = (logs) => logs.map((log) => { upstream, } = log; - const { host: domain, type } = question; + const { name: domain, unicode_name: unicodeName, type } = question; const processResponse = (data) => (data ? data.map((response) => { const { value, type, ttl } = response; @@ -96,6 +96,7 @@ export const normalizeLogs = (logs) => logs.map((log) => { return { time, domain, + unicodeName, type, response: processResponse(answer), reason, diff --git a/internal/querylog/http.go b/internal/querylog/http.go index cc26f8ab..8bf5ee27 100644 --- a/internal/querylog/http.go +++ b/internal/querylog/http.go @@ -127,7 +127,7 @@ func getDoubleQuotesEnclosedValue(s *string) bool { } // parseSearchCriterion parses a search criterion from the query parameter. -func (l *queryLog) parseSearchCriterion(q url.Values, name string, ct criterionType) (bool, searchCriterion, error) { +func (l *queryLog) parseSearchCriterion(q url.Values, name string, ct criterionType) (ok bool, sc searchCriterion, err error) { val := q.Get(name) if len(val) == 0 { return false, searchCriterion{}, nil @@ -176,7 +176,7 @@ func (l *queryLog) parseSearchParams(r *http.Request) (p *searchParams, err erro } paramNames := map[string]criterionType{ - "search": ctDomainOrClient, + "search": ctTerm, "response_status": ctFilteringStatus, } diff --git a/internal/querylog/json.go b/internal/querylog/json.go index b2f6f515..2dfaf68b 100644 --- a/internal/querylog/json.go +++ b/internal/querylog/json.go @@ -10,6 +10,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/log" "github.com/miekg/dns" + "golang.org/x/net/idna" ) // TODO(a.garipov): Use a proper structured approach here. @@ -66,6 +67,20 @@ func (l *queryLog) logEntryToJSONEntry(entry *logEntry) (jsonEntry jobject) { } } + hostname := entry.QHost + question := jobject{ + "type": entry.QType, + "class": entry.QClass, + "name": hostname, + } + if qhost, err := idna.ToUnicode(hostname); err == nil { + if qhost != hostname && qhost != "" { + question["unicode_name"] = qhost + } + } else { + log.Debug("translating %q into unicode: %s", hostname, err) + } + jsonEntry = jobject{ "reason": entry.Result.Reason.String(), "elapsedMs": strconv.FormatFloat(entry.Elapsed.Seconds()*1000, 'f', -1, 64), @@ -74,11 +89,7 @@ func (l *queryLog) logEntryToJSONEntry(entry *logEntry) (jsonEntry jobject) { "client_info": entry.client, "client_proto": entry.ClientProto, "upstream": entry.Upstream, - "question": jobject{ - "host": entry.QHost, - "type": entry.QType, - "class": entry.QClass, - }, + "question": question, } if entry.ClientID != "" { diff --git a/internal/querylog/qlog_test.go b/internal/querylog/qlog_test.go index 709f4585..9ee31aa9 100644 --- a/internal/querylog/qlog_test.go +++ b/internal/querylog/qlog_test.go @@ -67,7 +67,7 @@ func TestQueryLog(t *testing.T) { }, { name: "by_domain_strict", sCr: []searchCriterion{{ - criterionType: ctDomainOrClient, + criterionType: ctTerm, strict: true, value: "TEST.example.org", }}, @@ -77,7 +77,7 @@ func TestQueryLog(t *testing.T) { }, { name: "by_domain_non-strict", sCr: []searchCriterion{{ - criterionType: ctDomainOrClient, + criterionType: ctTerm, strict: false, value: "example.ORG", }}, @@ -89,7 +89,7 @@ func TestQueryLog(t *testing.T) { }, { name: "by_client_ip_strict", sCr: []searchCriterion{{ - criterionType: ctDomainOrClient, + criterionType: ctTerm, strict: true, value: "2.2.2.2", }}, @@ -99,7 +99,7 @@ func TestQueryLog(t *testing.T) { }, { name: "by_client_ip_non-strict", sCr: []searchCriterion{{ - criterionType: ctDomainOrClient, + criterionType: ctTerm, strict: false, value: "2.2.2", }}, diff --git a/internal/querylog/searchcriterion.go b/internal/querylog/searchcriterion.go index d5f4d4f0..d726b5bc 100644 --- a/internal/querylog/searchcriterion.go +++ b/internal/querylog/searchcriterion.go @@ -11,9 +11,11 @@ import ( type criterionType int const ( - // ctDomainOrClient is for searching by the domain name, the client's IP - // address, or the clinet's ID. - ctDomainOrClient criterionType = iota + // ctTerm is for searching by the domain name, the client's IP + // address, the client's ID or the client's name. + // + // TODO(e.burkov): Make it support IDNA while #3012. + ctTerm criterionType = iota // ctFilteringStatus is for searching by the filtering status. // // See (*searchCriterion).ctFilteringStatusCase for details. @@ -114,7 +116,7 @@ func (c *searchCriterion) ctDomainOrClientCaseNonStrict( // optimisation purposes. func (c *searchCriterion) quickMatch(line string, findClient quickMatchClientFunc) (ok bool) { switch c.criterionType { - case ctDomainOrClient: + case ctTerm: host := readJSONValue(line, `"QH":"`) ip := readJSONValue(line, `"IP":"`) clientID := readJSONValue(line, `"CID":"`) @@ -141,7 +143,7 @@ func (c *searchCriterion) quickMatch(line string, findClient quickMatchClientFun // match checks if the log entry matches this search criterion. func (c *searchCriterion) match(entry *logEntry) bool { switch c.criterionType { - case ctDomainOrClient: + case ctTerm: return c.ctDomainOrClientCase(entry) case ctFilteringStatus: return c.ctFilteringStatusCase(entry.Result) diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index 7660cc27..4744f720 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -4,6 +4,17 @@ ## v0.107: API changes +### The new field `"unicode_name"` in `DNSQuestion` + +* The new optional field `"unicode_name"` is the Unicode representation of + question's domain name. It is only presented if the original question's + domain name is an IDN. + +### Documentation fix of `DNSQuestion` + +* Previously incorrectly named field `"host"` in `DNSQuestion` is now named + `"name"`. + ### Disabling Statistics * The API `POST /control/stats_config` HTTP API allows disabling statistics by diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index dbe6c2c0..5410427b 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -1823,9 +1823,12 @@ 'class': 'type': 'string' 'example': 'IN' - 'host': + 'name': 'type': 'string' - 'example': 'example.org' + 'example': 'xn--d1abbgf6aiiy.xn--p1ai' + 'unicode_name': + 'type': 'string' + 'example': 'президент.рф' 'type': 'type': 'string' 'example': 'A' diff --git a/scripts/hooks/pre-commit b/scripts/hooks/pre-commit index e38e9aa5..6c2e5d34 100755 --- a/scripts/hooks/pre-commit +++ b/scripts/hooks/pre-commit @@ -4,7 +4,7 @@ set -e -f -u # Show all temporary todos to the programmer but don't fail the commit # if there are any, because the commit could be in a temporary branch. -git grep -e 'TODO.*!!' -- ':!HACKING.md' ':!scripts/hooks/pre-commit' | more || : +git grep -e 'TODO.*!!' -- ':!HACKING.md' ':!scripts/hooks/pre-commit' | cat || : if [ "$( git diff --cached --name-only -- 'client/*.js' )" ] then