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 <e.burkov@adguard.com> Date: Tue Jun 29 13:11:10 2021 +0300 client: mv punycode label commit 6585dcaece9f590d7f02afb5aa25953ab0c2555b Author: Ildar Kamalov <ik@adguard.com> Date: Tue Jun 29 12:32:40 2021 +0300 client: handle unicode name commit c0f61bfbb9bdf919be7b07c112c4b7a52f3ad6a1 Author: Eugene Burkov <e.burkov@adguard.com> Date: Mon Jun 28 20:00:57 2021 +0300 all: imp log of changes commit 41388abc8770ce164bcba327fcf0013133b5e6f7 Author: Eugene Burkov <e.burkov@adguard.com> Date: Mon Jun 28 19:52:23 2021 +0300 scripts: imp hooks commit 9c4ba933fbd9340e1de061d4f451218238650c0f Author: Eugene Burkov <e.burkov@adguard.com> Date: Mon Jun 28 19:47:27 2021 +0300 all: imp code, docs commit 61bd6d6f926480cb8c2f9bd3cd2b61e1532f71cf Author: Eugene Burkov <e.burkov@adguard.com> Date: Mon Jun 28 16:09:25 2021 +0300 querylog: add ascii hostname, convert to unicode
2021-06-29 13:36:52 +03:00 · 2021-06-29 13:36:52 +03:00 · 16e5e09c2e
parent 9d1656b5c1
commit 16e5e09c2e
11 changed files with 139 additions and 52 deletions
--- 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
--- 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",
--- 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
-                && <a href={sourceData.url} target="_blank" rel="noopener noreferrer"
-                      className="link--green">{sourceData.name}</a>,
+        source_label: sourceData && (
+            <a
+                href={sourceData.url}
+                target="_blank"
+                rel="noopener noreferrer"
+                className="link--green"
+            >
+                {sourceData.name}
+            </a>
+        ),
    };

    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 <div key={idx} className={className}>{preparedContent}</div>;
+
+        const className = classNames(
+            'text-truncate o-hidden',
+            { 'overflow-break': preparedContent?.length > 100 },
+        );
+
+        return (
+            <div key={idx} className={className}>
+                {preparedContent}
+            </div>
+        );
    };

    const getGrid = (contentObj, title, className) => [
-        <div key={title} className={classNames('pb-2 grid--title', className)}>{t(title)}</div>,
-        <div key={`${title}-1`}
-             className="grid grid--limited">{React.Children.map(Object.entries(contentObj), renderGrid)}</div>,
+        <div key={title} className={classNames('pb-2 grid--title', className)}>
+            {t(title)}
+        </div>,
+        <div key={`${title}-1`} className="grid grid--limited">
+            {React.Children.map(Object.entries(contentObj), renderGrid)}
+        </div>,
    ];

    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 <div className="d-flex o-hidden logs__cell logs__cell logs__cell--domain" role="gridcell">
-        {dnssec_enabled && <IconTooltip
-                className={lockIconClass}
-                tooltipClass='py-4 px-5 pb-45'
-                canShowTooltip={!!answer_dnssec}
-                xlinkHref='lock'
-                columnClass='w-100'
-                content='validated_with_dnssec'
-                placement='bottom'
-        />}
-        <IconTooltip className={privacyIconClass} tooltipClass='pt-4 pb-5 px-5 mw-75'
-                     xlinkHref='privacy' contentItemClass='key-colon' renderContent={renderContent}
-                     place='bottom' />
-        <div className={valueClass}>
-            <div className="text-truncate" title={domain}>{domain}</div>
-            {details && isDetailed
-            && <div className="detailed-info d-none d-sm-block text-truncate"
-                    title={details}>{details}</div>}
+    return (
+        <div
+            className="d-flex o-hidden logs__cell logs__cell logs__cell--domain"
+            role="gridcell"
+        >
+            {dnssec_enabled && (
+                <IconTooltip
+                    className={lockIconClass}
+                    tooltipClass="py-4 px-5 pb-45"
+                    canShowTooltip={!!answer_dnssec}
+                    xlinkHref="lock"
+                    columnClass="w-100"
+                    content="validated_with_dnssec"
+                    placement="bottom"
+                />
+            )}
+            <IconTooltip
+                className={privacyIconClass}
+                tooltipClass="pt-4 pb-5 px-5 mw-75"
+                xlinkHref="privacy"
+                contentItemClass="key-colon"
+                renderContent={renderContent}
+                place="bottom"
+            />
+            <div className={valueClass}>
+                {unicodeName ? (
+                    <div className="text-truncate" title={unicodeName}>
+                        {unicodeName}
+                    </div>
+                ) : (
+                    <div className="text-truncate" title={domain}>
+                        {domain}
+                    </div>
+                )}
+                {details && isDetailed && (
+                    <div
+                        className="detailed-info d-none d-sm-block text-truncate"
+                        title={details}
+                    >
+                        {details}
+                    </div>
+                )}
+            </div>
        </div>
-    </div>;
+    );
 };

 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,
--- 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,
--- 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,
 	}

--- 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 != "" {
--- 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",
 		}},
--- 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)
--- 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
--- 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'
--- 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