all: imp code, docs

client/src/__locales/en.json

@@ -222,7 +222,7 @@
     "updated_upstream_dns_toast": "Upstream servers successfully saved",
     "dns_test_ok_toast": "Specified DNS servers are working correctly",
     "dns_test_not_ok_toast": "Server \"{{key}}\": could not be used, please check that you've written it correctly",
-    "dns_test_warning_toast": "Server \"{{key}}\": doesn't respond for the test request and thus may not work properly",
+    "dns_test_warning_toast": "Server \"{{key}}\" does not respond to test requests and may not work properly",
     "unblock": "Unblock",
     "block": "Block",
     "disallow_this_client": "Disallow this client",

internal/dnsforward/http.go

@@ -374,7 +374,7 @@ func newUpstreamConfig(upstreams []string) (conf *proxy.UpstreamConfig, err erro
 
 		_, err = validateUpstream(ups, domains)
 		if err != nil {
-			return nil, fmt.Errorf("validating upstream %s: %w", u, err)
+			return nil, fmt.Errorf("validating upstream %q: %w", u, err)
 		}
 	}
 
@@ -457,6 +457,11 @@ func ValidateUpstreamsPrivate(upstreams []string, privateNets netutil.SubnetSet)
 
 var protocols = []string{"udp://", "tcp://", "tls://", "https://", "sdns://", "quic://"}
 
+// validateUpstream returns an error if u alongside with domains is not a valid
+// upstream configuration.  useDefault is true if the upstream is
+// domain-specific and is configured to point at the default upstream server
+// which is validated separately.  The upstream is considered domain-specific
+// only if domains is nil.
 func validateUpstream(u string, domains []string) (useDefault bool, err error) {
 	// The special server address '#' means that default server must be used.
 	if useDefault = u == "#" && domains != nil; useDefault {
@@ -489,10 +494,11 @@ func validateUpstream(u string, domains []string) (useDefault bool, err error) {
 // separateUpstream returns the upstream and the specified domains.  domains is
 // nil when the upstream is not domains-specific.  Otherwise it may also be
 // empty.
-func separateUpstream(upstreamStr string) (upstream string, domains []string, err error) {
+func separateUpstream(upstreamStr string) (ups string, domains []string, err error) {
 	if !strings.HasPrefix(upstreamStr, "[/") {
 		return upstreamStr, nil, nil
 	}
+
 	defer func() { err = errors.Annotate(err, "bad upstream for domain %q: %w", upstreamStr) }()
 
 	parts := strings.Split(upstreamStr[2:], "/]")
@@ -521,8 +527,9 @@ func separateUpstream(upstreamStr string) (upstream string, domains []string, er
 	return parts[1], domains, nil
 }
 
-// excFunc is a signature of function to check if upstream exchanges correctly.
-type excFunc func(u upstream.Upstream) (err error)
+// healthCheckFunc is a signature of function to check if upstream exchanges
+// properly.
+type healthCheckFunc func(u upstream.Upstream) (err error)
 
 // checkDNSUpstreamExc checks if the DNS upstream exchanges correctly.
 func checkDNSUpstreamExc(u upstream.Upstream) (err error) {
@@ -558,7 +565,7 @@ func checkDNSUpstreamExc(u upstream.Upstream) (err error) {
 // checkPrivateUpstreamExc checks if the upstream for resolving private
 // addresses exchanges correctly.
 //
-// TODO(e.burkov):  Think on testing the ip6.arpa. as well.
+// TODO(e.burkov):  Think about testing the ip6.arpa. as well.
 func checkPrivateUpstreamExc(u upstream.Upstream) (err error) {
 	// inAddrArpaTLD is the special-use fully-qualified domain name for PTR IP
 	// address resolution.
@@ -588,22 +595,33 @@ func checkPrivateUpstreamExc(u upstream.Upstream) (err error) {
 // domainSpecificTestError is a wrapper for errors returned by checkDNS to mark
 // the tested upstream domain-specific and therefore consider its errors
 // non-critical.
+//
+// TODO(a.garipov):  Some common mechanism of distinguishing between errors and
+// warnings (non-critical errors) is desired.
 type domainSpecificTestError struct {
 	error
 }
 
-func checkDNS(input string, bootstrap []string, timeout time.Duration, ef excFunc) (err error) {
-	if IsCommentOrEmpty(input) {
+// checkDNS checks the upstream server defined by upstreamConfigStr using
+// healthCheck for actually exchange messages.  It uses bootstrap to resolve the
+// upstream's address.
+func checkDNS(
+	upstreamConfigStr string,
+	bootstrap []string,
+	timeout time.Duration,
+	healthCheck healthCheckFunc,
+) (err error) {
+	if IsCommentOrEmpty(upstreamConfigStr) {
 		return nil
 	}
 
-	// // Separate upstream from domains list.
-	input, domains, err := separateUpstream(input)
+	// Separate upstream from domains list.
+	upstreamAddr, domains, err := separateUpstream(upstreamConfigStr)
 	if err != nil {
 		return fmt.Errorf("wrong upstream format: %w", err)
 	}
 
-	useDefault, err := validateUpstream(input, domains)
+	useDefault, err := validateUpstream(upstreamAddr, domains)
 	if err != nil {
 		return fmt.Errorf("wrong upstream format: %w", err)
 	} else if useDefault {
@@ -614,27 +632,27 @@ func checkDNS(input string, bootstrap []string, timeout time.Duration, ef excFun
 		bootstrap = defaultBootstrap
 	}
 
-	log.Debug("checking if upstream %s works", input)
+	log.Debug("checking if upstream %s works", upstreamAddr)
 
 	var u upstream.Upstream
-	u, err = upstream.AddressToUpstream(input, &upstream.Options{
+	u, err = upstream.AddressToUpstream(upstreamAddr, &upstream.Options{
 		Bootstrap: bootstrap,
 		Timeout:   timeout,
 	})
 	if err != nil {
-		return fmt.Errorf("failed to choose upstream for %q: %w", input, err)
+		return fmt.Errorf("failed to choose upstream for %q: %w", upstreamAddr, err)
 	}
 
-	if err = ef(u); err != nil {
-		err = fmt.Errorf("upstream %q fails to exchange: %w", input, err)
+	if err = healthCheck(u); err != nil {
+		err = fmt.Errorf("upstream %q fails to exchange: %w", upstreamAddr, err)
 		if domains != nil {
 			return domainSpecificTestError{error: err}
 		}
 
 		return err
 	}
 
-	log.Debug("upstream %s is ok", input)
+	log.Debug("upstream %s is ok", upstreamAddr)
 
 	return nil
 }

internal/dnsforward/http_test.go

@@ -186,7 +186,7 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
 	}, {
 		name: "upstream_dns_bad",
 		wantSet: `validating upstream servers: ` +
-			`validating upstream !!!: bad ipport address "!!!": ` +
+			`validating upstream "!!!": bad ipport address "!!!": ` +
 			`address !!!: missing port in address`,
 	}, {
 		name: "bootstraps_bad",
@@ -293,23 +293,23 @@ func TestValidateUpstreams(t *testing.T) {
 		},
 	}, {
 		name:    "invalid",
-		wantErr: `validating upstream dhcp://fake.dns: wrong protocol`,
+		wantErr: `validating upstream "dhcp://fake.dns": wrong protocol`,
 		set:     []string{"dhcp://fake.dns"},
 	}, {
 		name:    "invalid",
-		wantErr: `validating upstream 1.2.3.4.5: bad ipport address "1.2.3.4.5": address 1.2.3.4.5: missing port in address`,
+		wantErr: `validating upstream "1.2.3.4.5": bad ipport address "1.2.3.4.5": address 1.2.3.4.5: missing port in address`,
 		set:     []string{"1.2.3.4.5"},
 	}, {
 		name:    "invalid",
-		wantErr: `validating upstream 123.3.7m: bad ipport address "123.3.7m": address 123.3.7m: missing port in address`,
+		wantErr: `validating upstream "123.3.7m": bad ipport address "123.3.7m": address 123.3.7m: missing port in address`,
 		set:     []string{"123.3.7m"},
 	}, {
 		name:    "invalid",
 		wantErr: `bad upstream for domain "[/host.com]tls://dns.adguard.com": missing separator`,
 		set:     []string{"[/host.com]tls://dns.adguard.com"},
 	}, {
 		name:    "invalid",
-		wantErr: `validating upstream [host.ru]#: bad ipport address "[host.ru]#": address [host.ru]#: missing port in address`,
+		wantErr: `validating upstream "[host.ru]#": bad ipport address "[host.ru]#": address [host.ru]#: missing port in address`,
 		set:     []string{"[host.ru]#"},
 	}, {
 		name:    "valid_default",