Merge branch 'master' into 3020-client-sources

.github/stale.yml

@@ -8,6 +8,7 @@
 - 'documentation'
 - 'enhancement'
 - 'feature request'
+- 'help wanted'
 - 'localization'
 - 'needs investigation'
 - 'recurrent'

CHANGELOG.md

@@ -122,6 +122,7 @@ In this release, the schema version has changed from 12 to 14.
 
 ### Fixed
 
+- Slow version update queries making other HTTP APIs unresponsible ([#4499]).
 - ARP tables refreshing process causing excessive PTR requests ([#3157]).
 
 [#1730]: https://github.com/AdguardTeam/AdGuardHome/issues/1730
@@ -140,6 +141,7 @@ In this release, the schema version has changed from 12 to 14.
 [#4221]: https://github.com/AdguardTeam/AdGuardHome/issues/4221
 [#4238]: https://github.com/AdguardTeam/AdGuardHome/issues/4238
 [#4276]: https://github.com/AdguardTeam/AdGuardHome/issues/4276
+[#4499]: https://github.com/AdguardTeam/AdGuardHome/issues/4499
 
 [repr]:         https://reproducible-builds.org/docs/source-date-epoch/
 [doq-draft-10]: https://datatracker.ietf.org/doc/html/draft-ietf-dprive-dnsoquic-10#section-10.2

client/src/__locales/en.json

@@ -9,7 +9,7 @@
     "bootstrap_dns": "Bootstrap DNS servers",
     "bootstrap_dns_desc": "Bootstrap DNS servers are used to resolve IP addresses of the DoH/DoT resolvers you specify as upstreams.",
     "local_ptr_title": "Private reverse DNS servers",
-    "local_ptr_desc": "The DNS servers that AdGuard Home uses for local PTR queries. These servers are used to resolve the hostnames of clients with private IP addresses, for example \"192.168.12.34\", using reverse DNS. If not set, AdGuard Home uses the addresses of the default DNS resolvers of your OS except for the addresses of AdGuard Home itself.",
+    "local_ptr_desc": "The DNS servers that AdGuard Home uses for local PTR queries. These servers are used to resolve PTR requests for addresses in private IP ranges, for example \"192.168.12.34\", using reverse DNS. If not set, AdGuard Home uses the addresses of the default DNS resolvers of your OS except for the addresses of AdGuard Home itself.",
     "local_ptr_default_resolver": "By default, AdGuard Home uses the following reverse DNS resolvers: {{ip}}.",
     "local_ptr_no_default_resolver": "AdGuard Home could not determine suitable private reverse DNS resolvers for this system.",
     "local_ptr_placeholder": "Enter one server address per line",

client/src/components/Logs/InfiniteTable.js

@@ -43,7 +43,7 @@ const InfiniteTable = ({
 
     useEffect(() => {
         listener();
-    }, [items.length < QUERY_LOGS_PAGE_LIMIT]);
+    }, [items.length < QUERY_LOGS_PAGE_LIMIT, isEntireLog]);
 
     useEffect(() => {
         const THROTTLE_TIME = 100;
@@ -66,15 +66,24 @@ const InfiniteTable = ({
 
     const isNothingFound = items.length === 0 && !processingGetLogs;
 
-    return <div className='logs__table' role='grid'>
-        {loading && <Loading />}
-        <Header />
-        {isNothingFound
-            ? <label className="logs__no-data">{t('nothing_found')}</label>
-            : <>{items.map(renderRow)}
-                    {!isEntireLog && <div ref={loader} className="logs__loading text-center">{t('loading_table_status')}</div>}
-            </>}
-    </div>;
+    return (
+        <div className="logs__table" role="grid">
+            {loading && <Loading />}
+            <Header />
+            {isNothingFound ? (
+                <label className="logs__no-data">{t('nothing_found')}</label>
+            ) : (
+                <>
+                    {items.map(renderRow)}
+                    {!isEntireLog && (
+                        <div ref={loader} className="logs__loading text-center">
+                            {t('loading_table_status')}
+                        </div>
+                    )}
+                </>
+            )}
+        </div>
+    );
 };
 
 InfiniteTable.propTypes = {

client/src/helpers/helpers.js

@@ -693,8 +693,8 @@ export const replaceZeroWithEmptyString = (value) => (parseInt(value, 10) === 0
  * @returns {string}
  */
 export const getLogsUrlParams = (search, response_status) => `?${queryString.stringify({
-    search,
-    response_status,
+    search: search || undefined,
+    response_status: response_status || undefined,
 })}`;
 
 export const processContent = (

internal/aghnet/net_nolinux.go → internal/aghnet/net_bsd.go

@@ -1,5 +1,5 @@
-//go:build !linux
-// +build !linux
+//go:build darwin || freebsd || openbsd
+// +build darwin freebsd openbsd
 
 package aghnet
 

internal/aghnet/net_windows.go

@@ -1,5 +1,5 @@
-//go:build !(linux || darwin || freebsd || openbsd)
-// +build !linux,!darwin,!freebsd,!openbsd
+//go:build windows
+// +build windows
 
 package aghnet
 
@@ -13,6 +13,10 @@ import (
 	"golang.org/x/sys/windows"
 )
 
+func canBindPrivilegedPorts() (can bool, err error) {
+	return true, nil
+}
+
 func ifaceHasStaticIP(string) (ok bool, err error) {
 	return false, aghos.Unsupported("checking static ip")
 }

internal/home/controlupdate.go

@@ -3,6 +3,7 @@ package home
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"os"
 	"os/exec"
@@ -27,12 +28,16 @@ type temporaryError interface {
 
 // Get the latest available version from the Internet
 func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("Content-Type", "application/json")
+
 	resp := &versionResponse{}
 	if Context.disableUpdate {
-		// w.Header().Set("Content-Type", "application/json")
 		resp.Disabled = true
-		_ = json.NewEncoder(w).Encode(resp)
-		// TODO(e.burkov): Add error handling and deal with headers.
+		err := json.NewEncoder(w).Encode(resp)
+		if err != nil {
+			aghhttp.Error(r, w, http.StatusInternalServerError, "writing body: %s", err)
+		}
+
 		return
 	}
 
@@ -44,30 +49,48 @@ func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) {
 	if r.ContentLength != 0 {
 		err = json.NewDecoder(r.Body).Decode(req)
 		if err != nil {
-			aghhttp.Error(r, w, http.StatusBadRequest, "JSON parse: %s", err)
+			aghhttp.Error(r, w, http.StatusBadRequest, "parsing request: %s", err)
 
 			return
 		}
 	}
 
-	for i := 0; i != 3; i++ {
-		func() {
-			Context.controlLock.Lock()
-			defer Context.controlLock.Unlock()
+	err = requestVersionInfo(resp, req.Recheck)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		aghhttp.Error(r, w, http.StatusBadGateway, "%s", err)
+
+		return
+	}
 
-			resp.VersionInfo, err = Context.updater.VersionInfo(req.Recheck)
-		}()
+	err = resp.setAllowedToAutoUpdate()
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		aghhttp.Error(r, w, http.StatusInternalServerError, "%s", err)
+
+		return
+	}
+
+	err = json.NewEncoder(w).Encode(resp)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusInternalServerError, "writing body: %s", err)
+	}
+}
 
+// requestVersionInfo sets the VersionInfo field of resp if it can reach the
+// update server.
+func requestVersionInfo(resp *versionResponse, recheck bool) (err error) {
+	for i := 0; i != 3; i++ {
+		resp.VersionInfo, err = Context.updater.VersionInfo(recheck)
 		if err != nil {
 			var terr temporaryError
 			if errors.As(err, &terr) && terr.Temporary() {
-				// Temporary network error.  This case may happen while
-				// we're restarting our DNS server.  Log and sleep for
-				// some time.
+				// Temporary network error.  This case may happen while we're
+				// restarting our DNS server.  Log and sleep for some time.
 				//
 				// See https://github.com/AdguardTeam/AdGuardHome/issues/934.
 				d := time.Duration(i) * time.Second
-				log.Info("temp net error: %q; sleeping for %s and retrying", err, d)
+				log.Info("update: temp net error: %q; sleeping for %s and retrying", err, d)
 				time.Sleep(d)
 
 				continue
@@ -76,29 +99,14 @@ func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) {
 
 		break
 	}
+
 	if err != nil {
 		vcu := Context.updater.VersionCheckURL()
-		// TODO(a.garipov): Figure out the purpose of %T verb.
-		aghhttp.Error(
-			r,
-			w,
-			http.StatusBadGateway,
-			"Couldn't get version check json from %s: %T %s\n",
-			vcu,
-			err,
-			err,
-		)
 
-		return
+		return fmt.Errorf("getting version info from %s: %s", vcu, err)
 	}
 
-	resp.confirmAutoUpdate()
-
-	w.Header().Set("Content-Type", "application/json")
-	err = json.NewEncoder(w).Encode(resp)
-	if err != nil {
-		aghhttp.Error(r, w, http.StatusInternalServerError, "Couldn't write body: %s", err)
-	}
+	return nil
 }
 
 // handleUpdate performs an update to the latest available version procedure.
@@ -132,31 +140,37 @@ func handleUpdate(w http.ResponseWriter, r *http.Request) {
 
 // versionResponse is the response for /control/version.json endpoint.
 type versionResponse struct {
-	Disabled bool `json:"disabled"`
 	updater.VersionInfo
+	Disabled bool `json:"disabled"`
 }
 
-// confirmAutoUpdate checks the real possibility of auto update.
-func (vr *versionResponse) confirmAutoUpdate() {
-	if vr.CanAutoUpdate != nil && *vr.CanAutoUpdate {
-		canUpdate := true
+// setAllowedToAutoUpdate sets CanAutoUpdate to true if AdGuard Home is actually
+// allowed to perform an automatic update by the OS.
+func (vr *versionResponse) setAllowedToAutoUpdate() (err error) {
+	if vr.CanAutoUpdate == nil || !*vr.CanAutoUpdate {
+		return
+	}
 
-		var tlsConf *tlsConfigSettings
-		if runtime.GOOS != "windows" {
-			tlsConf = &tlsConfigSettings{}
-			Context.tls.WriteDiskConfig(tlsConf)
-		}
+	tlsConf := &tlsConfigSettings{}
+	Context.tls.WriteDiskConfig(tlsConf)
 
-		if tlsConf != nil &&
-			((tlsConf.Enabled && (tlsConf.PortHTTPS < 1024 ||
-				tlsConf.PortDNSOverTLS < 1024 ||
-				tlsConf.PortDNSOverQUIC < 1024)) ||
-				config.BindPort < 1024 ||
-				config.DNS.Port < 1024) {
-			canUpdate, _ = aghnet.CanBindPrivilegedPorts()
+	canUpdate := true
+	if tlsConfUsesPrivilegedPorts(tlsConf) || config.BindPort < 1024 || config.DNS.Port < 1024 {
+		canUpdate, err = aghnet.CanBindPrivilegedPorts()
+		if err != nil {
+			return fmt.Errorf("checking ability to bind privileged ports: %w", err)
 		}
-		vr.CanAutoUpdate = &canUpdate
 	}
+
+	vr.CanAutoUpdate = &canUpdate
+
+	return nil
+}
+
+// tlsConfUsesPrivilegedPorts returns true if the provided TLS configuration
+// indicates that privileged ports are used.
+func tlsConfUsesPrivilegedPorts(c *tlsConfigSettings) (ok bool) {
+	return c.Enabled && (c.PortHTTPS < 1024 || c.PortDNSOverTLS < 1024 || c.PortDNSOverQUIC < 1024)
 }
 
 // finishUpdate completes an update procedure.

internal/updater/check.go

@@ -17,11 +17,11 @@ const versionCheckPeriod = 8 * time.Hour
 
 // VersionInfo contains information about a new version.
 type VersionInfo struct {
+	CanAutoUpdate        *bool  `json:"can_autoupdate,omitempty"`
 	NewVersion           string `json:"new_version,omitempty"`
 	Announcement         string `json:"announcement,omitempty"`
 	AnnouncementURL      string `json:"announcement_url,omitempty"`
 	SelfUpdateMinVersion string `json:"-"`
-	CanAutoUpdate        *bool  `json:"can_autoupdate,omitempty"`
 }
 
 // MaxResponseSize is responses on server's requests maximum length in bytes.

scripts/make/go-lint.sh

@@ -136,7 +136,6 @@ underscores() {
 			-e '_freebsd.go'\
 			-e '_linux.go'\
 			-e '_little.go'\
-			-e '_nolinux.go'\
 			-e '_openbsd.go'\
 			-e '_others.go'\
 			-e '_test.go'\