Pull request: 3013 querylog idna

Merge in DNS/adguard-home from 3013-idna to master Closes #3013. Squashed commit of the following: commit 567d4c3 Author: Eugene Burkov <[email protected]> Date: Tue Jun 29 13:11:10 2021 +0300 client: mv punycode label commit 6585dca Author: Ildar Kamalov <[email protected]> Date: Tue Jun 29 12:32:40 2021 +0300 client: handle unicode name commit c0f61bf Author: Eugene Burkov <[email protected]> Date: Mon Jun 28 20:00:57 2021 +0300 all: imp log of changes commit 41388ab Author: Eugene Burkov <[email protected]> Date: Mon Jun 28 19:52:23 2021 +0300 scripts: imp hooks commit 9c4ba93 Author: Eugene Burkov <[email protected]> Date: Mon Jun 28 19:47:27 2021 +0300 all: imp code, docs commit 61bd6d6 Author: Eugene Burkov <[email protected]> Date: Mon Jun 28 16:09:25 2021 +0300 querylog: add ascii hostname, convert to unicode
AdguardTeam · Jun 29, 2021 · 16e5e09 · 16e5e09
1 parent 9d1656b
commit 16e5e09
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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(', ');
-
-    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>}
+    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}>
+                {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,

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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