all: imp code

go.mod

@@ -3,7 +3,7 @@ module github.com/AdguardTeam/AdGuardHome
 go 1.20
 
 require (
-	github.com/AdguardTeam/dnsproxy v0.60.2-0.20231213112429-14cbbed218dc
+	github.com/AdguardTeam/dnsproxy v0.60.2-0.20231214160436-0f49b8b5cafc
 	github.com/AdguardTeam/golibs v0.18.0
 	github.com/AdguardTeam/urlfilter v0.17.3
 	github.com/NYTimes/gziphandler v1.1.1

go.sum

@@ -1,5 +1,5 @@
-github.com/AdguardTeam/dnsproxy v0.60.2-0.20231213112429-14cbbed218dc h1:D60Z2i4nR13VbWRXrFqO3ZuwA143xSKqNPh+1BEmn+4=
+github.com/AdguardTeam/dnsproxy v0.60.2-0.20231214160436-0f49b8b5cafc h1:cwVAnTs6By/NT6PGWt66G+pH0MDL9nfuwLz8VcwotlM=
-github.com/AdguardTeam/dnsproxy v0.60.2-0.20231213112429-14cbbed218dc/go.mod h1:B7FvvTFQZBfey1cJXQo732EyCLX6xj4JqrciCawATzg=
+github.com/AdguardTeam/dnsproxy v0.60.2-0.20231214160436-0f49b8b5cafc/go.mod h1:B7FvvTFQZBfey1cJXQo732EyCLX6xj4JqrciCawATzg=
 github.com/AdguardTeam/golibs v0.18.0 h1:ckS2YK7t2Ub6UkXl0fnreVaM15Zb07Hh1gmFqttjpWg=
 github.com/AdguardTeam/golibs v0.18.0/go.mod h1:DKhCIXHcUYtBhU8ibTLKh1paUL96n5zhQBlx763sj+U=
 github.com/AdguardTeam/urlfilter v0.17.3 h1:fg/ObbnO0Cv6aw0tW6N/ETDMhhNvmcUUOZ7HlmKC3rw=

internal/dnsforward/configvalidator.go

@@ -2,7 +2,6 @@ package dnsforward
 
 import (
 	"fmt"
-	"strings"
 	"sync"
 
 	"github.com/AdguardTeam/dnsproxy/proxy"
@@ -10,20 +9,19 @@ import (
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/miekg/dns"
-	"golang.org/x/exp/slices"
 )
 
 // upstreamConfigValidator parses the [*proxy.UpstreamConfig] and checks the
 // actual DNS availability of each upstream.
 type upstreamConfigValidator struct {
 	// general is the general upstream configuration.
-	general []*upstreamResult
+	general map[string]*upstreamResult
 
 	// fallback is the fallback upstream configuration.
-	fallback []*upstreamResult
+	fallback map[string]*upstreamResult
 
 	// private is the private upstream configuration.
-	private []*upstreamResult
+	private map[string]*upstreamResult
 }
 
 // upstreamResult is a result of validation of an [upstream.Upstream] within an
@@ -36,23 +34,10 @@ type upstreamResult struct {
 	// err is the error either from parsing or from checking the upstream.
 	err error
 
-	// original is the piece of configuration that have either been turned to an
-	// upstream or caused an error.
-	original string
-
 	// isSpecific is true if the upstream is domain-specific.
 	isSpecific bool
 }
 
-// compare compares two [upstreamResult]s.  It returns 0 if they are equal, -1
-// if ur should be sorted before other, and 1 otherwise.
-//
-// TODO(e.burkov):  Perhaps it makes sense to sort the results with errors near
-// the end.
-func (ur *upstreamResult) compare(other *upstreamResult) (res int) {
-	return strings.Compare(ur.original, other.original)
-}
-
 // newUpstreamConfigValidator parses the upstream configuration and returns a
 // validator for it.  cv already contains the parsed upstreams along with errors
 // related.
@@ -62,109 +47,99 @@ func newUpstreamConfigValidator(
 	private []string,
 	opts *upstream.Options,
 ) (cv *upstreamConfigValidator) {
-	cv = &upstreamConfigValidator{}
+	cv = &upstreamConfigValidator{
-
+		general:  map[string]*upstreamResult{},
-	cv.general = cv.insertLinesResults(cv.general, general, opts)
+		fallback: map[string]*upstreamResult{},
-	cv.fallback = cv.insertLinesResults(cv.fallback, fallback, opts)
+		private:  map[string]*upstreamResult{},
-	cv.private = cv.insertLinesResults(cv.private, private, opts)
-
-	return cv
-}
-
-// insertLinesResults parses lines and inserts the result into s.  It can insert
-// multiple results as well as none.
-func (cv *upstreamConfigValidator) insertLinesResults(
-	s []*upstreamResult,
-	lines []string,
-	opts *upstream.Options,
-) (result []*upstreamResult) {
-	conf, err := proxy.ParseUpstreamsConfig(lines, opts)
-	if err != nil {
-		s = cv.insertErrResults(s, err)
 	}
 
-	return cv.insertConfResults(s, conf)
+	conf, err := proxy.ParseUpstreamsConfig(general, opts)
+	if err != nil {
+		cv.insertErrResults(cv.general, "Upstream DNS Servers", err)
+	}
+	cv.insertConfResults(cv.general, conf)
+
+	conf, err = proxy.ParseUpstreamsConfig(fallback, opts)
+	if err != nil {
+		cv.insertErrResults(cv.fallback, "Fallback DNS Servers", err)
+	}
+	cv.insertConfResults(cv.fallback, conf)
+
+	conf, err = proxy.ParseUpstreamsConfig(private, opts)
+	if err != nil {
+		cv.insertErrResults(cv.private, "Private DNS Servers", err)
+	}
+	cv.insertConfResults(cv.private, conf)
+
+	return cv
 }
 
 // insertErrResults parses err and inserts the result into s.  It can insert
 // multiple results as well as none.
 func (cv *upstreamConfigValidator) insertErrResults(
-	s []*upstreamResult,
+	m map[string]*upstreamResult,
+	section string,
 	err error,
-) (result []*upstreamResult) {
+) {
-	wrapper, ok := err.(interface{ Unwrap() []error })
+	wrapper, ok := err.(errors.WrapperSlice)
 	if !ok {
-		return s
+		log.Debug("dnsforward: unwrapping: %s", err)
+
+		return
 	}
 
 	errs := wrapper.Unwrap()
 	for _, e := range errs {
 		var parseErr *proxy.ParseError
 		if !errors.As(e, &parseErr) {
+			log.Debug("dnsforward: inserting: %s", err)
+
 			continue
 		}
 
 		idx := parseErr.Idx
-		s = cv.insert(s, &upstreamResult{
-			err:      err,
-			original: fmt.Sprintf("Line: %d", idx+1),
-		})
-	}
 
-	return s
+		original := fmt.Sprintf("Line: %d %s", idx+1, section)
+		m[original] = &upstreamResult{err: errors.Unwrap(e)}
+	}
 }
 
 // insertConfResults parses conf and inserts the result into s.  It can insert
 // multiple results as well as none.
 func (cv *upstreamConfigValidator) insertConfResults(
-	s []*upstreamResult,
+	m map[string]*upstreamResult,
 	conf *proxy.UpstreamConfig,
-) (result []*upstreamResult) {
+) {
-	s = cv.insertListResults(s, conf.Upstreams, false)
+	cv.insertListResults(m, conf.Upstreams, false)
 
 	for _, ups := range conf.DomainReservedUpstreams {
-		s = cv.insertListResults(s, ups, true)
+		cv.insertListResults(m, ups, true)
 	}
 
 	for _, ups := range conf.SpecifiedDomainUpstreams {
-		s = cv.insertListResults(s, ups, true)
+		cv.insertListResults(m, ups, true)
 	}
-
-	return s
 }
 
 // insertListResults constructs upstream results from the upstream list and
 // inserts into s.  It can insert multiple results as well as none.
 func (cv *upstreamConfigValidator) insertListResults(
-	s []*upstreamResult,
+	m map[string]*upstreamResult,
 	ups []upstream.Upstream,
 	specific bool,
-) (result []*upstreamResult) {
+) {
 	for _, u := range ups {
-		s = cv.insert(s, &upstreamResult{
+		addr := u.Address()
+		_, ok := m[addr]
+		if ok {
+			continue
+		}
+
+		m[addr] = &upstreamResult{
 			server:     u,
-			original:   u.Address(),
 			isSpecific: specific,
-		})
+		}
 	}
-
-	return s
-}
-
-// insert inserts r into slice in a sorted order, except duplicates.  slice must
-// not be nil.
-func (cv *upstreamConfigValidator) insert(
-	s []*upstreamResult,
-	r *upstreamResult,
-) (result []*upstreamResult) {
-	i, has := slices.BinarySearchFunc(s, r, (*upstreamResult).compare)
-	if has {
-		log.Debug("dnsforward: duplicate configuration %q", r.original)
-
-		return s
-	}
-
-	return slices.Insert(s, i, r)
 }
 
 // check tries to exchange with each successfully parsed upstream and enriches
@@ -237,8 +212,10 @@ func (cv *upstreamConfigValidator) checkSrv(
 // close closes all the upstreams that were successfully parsed.  It enriches
 // the results with deferred closing errors.
 func (cv *upstreamConfigValidator) close() {
-	for _, slice := range [][]*upstreamResult{cv.general, cv.fallback, cv.private} {
+	all := []map[string]*upstreamResult{cv.general, cv.fallback, cv.private}
-		for _, r := range slice {
+
+	for _, m := range all {
+		for _, r := range m {
 			if r.server != nil {
 				r.err = errors.WithDeferred(r.err, r.server.Close())
 			}
@@ -253,14 +230,14 @@ func (cv *upstreamConfigValidator) close() {
 func (cv *upstreamConfigValidator) status() (results map[string]string) {
 	result := map[string]string{}
 
-	for _, res := range cv.general {
+	for original, res := range cv.general {
-		resultToStatus("general", res, result)
+		resultToStatus("general", original, res, result)
 	}
-	for _, res := range cv.fallback {
+	for original, res := range cv.fallback {
-		resultToStatus("fallback", res, result)
+		resultToStatus("fallback", original, res, result)
 	}
-	for _, res := range cv.private {
+	for original, res := range cv.private {
-		resultToStatus("private", res, result)
+		resultToStatus("private", original, res, result)
 	}
 
 	return result
@@ -273,24 +250,29 @@ func (cv *upstreamConfigValidator) status() (results map[string]string) {
 // TODO(e.burkov):  Currently, the HTTP handler expects that all the results are
 // put together in a single map, which may lead to collisions, see AG-27539.
 // Improve the results compilation.
-func resultToStatus(section string, res *upstreamResult, resMap map[string]string) {
+func resultToStatus(
+	section string,
+	original string,
+	res *upstreamResult,
+	resMap map[string]string,
+) {
 	val := "OK"
 	if res.err != nil {
 		val = res.err.Error()
 	}
 
-	prevVal := resMap[res.original]
+	prevVal := resMap[original]
 	switch prevVal {
 	case "":
-		resMap[res.original] = val
+		resMap[original] = val
 	case val:
-		log.Debug("dnsforward: duplicating %s config line %q", section, res.original)
+		log.Debug("dnsforward: duplicating %s config line %q", section, original)
 	default:
 		log.Debug(
 			"dnsforward: warning: %s config line %q (%v) had different result %v",
 			section,
 			val,
-			res.original,
+			original,
 			prevVal,
 		)
 	}

internal/dnsforward/upstreams_internal_test.go

@@ -119,7 +119,9 @@ func TestUpstreamConfigValidator(t *testing.T) {
 		fallback: []string{"[/example/" + goodUps},
 		private:  []string{"[/example//bad.123/]" + goodUps},
 		want: map[string]string{
-			"Line: 1": `index: 0: cannot prepare the upstream 0 "[/example/]/]` + goodUps + `": unsupported url scheme: `,
+			"Line: 1 Upstream DNS Servers": "cannot prepare the upstream: unsupported url scheme: ",
+			"Line: 1 Private DNS Servers":  `bad domain name "bad.123": bad top-level domain name label "123": all octets are numeric`,
+			"Line: 1 Fallback DNS Servers": "wrong upstream specification",
 		},
 	}, {
 		name: "bad_proto",
@@ -127,7 +129,7 @@ func TestUpstreamConfigValidator(t *testing.T) {
 			"bad://1.2.3.4",
 		},
 		want: map[string]string{
-			"Line: 1": `index: 0: cannot prepare the upstream 0 "bad://1.2.3.4": unsupported url scheme: bad`,
+			"Line: 1 Upstream DNS Servers": `cannot prepare the upstream: unsupported url scheme: bad`,
 		},
 	}}