all: imp code, docs

internal/aghnet/subnetdetector.go

@@ -6,12 +6,12 @@ import (
 
 // SubnetDetector describes IP address properties.
 type SubnetDetector struct {
-	// spNets stores the collection of special-purpose address registries as
-	// defined by RFC 6890.
+	// spNets is the collection of special-purpose address registries as defined
+	// by RFC 6890.
 	spNets []*net.IPNet
 
-	// locServedNets stores the collection of locally-served networks as defined
-	// by RFC 6303.
+	// locServedNets is the collection of locally-served networks as defined by
+	// RFC 6303.
 	locServedNets []*net.IPNet
 }
 

internal/dnsforward/http.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"sort"
 	"strings"
 	"time"
 
@@ -170,14 +171,14 @@ func (req *dnsConfig) validate(snd *aghnet.SubnetDetector) (err error) {
 	if req.Upstreams != nil {
 		err = ValidateUpstreams(*req.Upstreams)
 		if err != nil {
-			return err
+			return fmt.Errorf("validating upstream servers: %w", err)
 		}
 	}
 
 	if req.LocalPTRUpstreams != nil {
 		err = ValidateUpstreamsPrivate(*req.LocalPTRUpstreams, snd)
 		if err != nil {
-			return err
+			return fmt.Errorf("validating private upstream servers: %w", err)
 		}
 	}
 
@@ -360,12 +361,12 @@ type LocalNetChecker interface {
 // type check
 var _ LocalNetChecker = (*aghnet.SubnetDetector)(nil)
 
-// assembleConfig validates upstreams and returns an appropriate upstream
+// newUpstreamConfig validates upstreams and returns an appropriate upstream
 // configuration or nil if it can't be built.
 //
 // TODO(e.burkov):  Perhaps proxy.ParseUpstreamsConfig should validate upstreams
 // slice already so that this function may be considered useless.
-func assembleConfig(upstreams []string) (conf *proxy.UpstreamConfig, err error) {
+func newUpstreamConfig(upstreams []string) (conf *proxy.UpstreamConfig, err error) {
 	// No need to validate comments and empty lines.
 	upstreams = stringutil.FilterOut(upstreams, IsCommentOrEmpty)
 	if len(upstreams) == 0 {
@@ -399,17 +400,31 @@ func assembleConfig(upstreams []string) (conf *proxy.UpstreamConfig, err error)
 //
 // TODO(e.burkov):  Move into aghnet or even into dnsproxy.
 func ValidateUpstreams(upstreams []string) (err error) {
-	_, err = assembleConfig(upstreams)
+	_, err = newUpstreamConfig(upstreams)
 
 	return err
 }
 
+// stringKeysSorted returns the sorted slice of string keys of m.
+//
+// TODO(e.burkov):  Use generics in Go 1.18.  Move into golibs.
+func stringKeysSorted(m map[string][]upstream.Upstream) (sorted []string) {
+	sorted = make([]string, 0, len(m))
+	for s := range m {
+		sorted = append(sorted, s)
+	}
+
+	sort.Strings(sorted)
+
+	return sorted
+}
+
 // ValidateUpstreamsPrivate validates each upstream and returns an error if any
 // upstream is invalid or if there are no default upstreams specified.  It also
 // checks each domain of domain-specific upstreams for being ARPA pointing to
 // a locally-served network.  lnc must not be nil.
 func ValidateUpstreamsPrivate(upstreams []string, lnc LocalNetChecker) (err error) {
-	conf, err := assembleConfig(upstreams)
+	conf, err := newUpstreamConfig(upstreams)
 	if err != nil {
 		return err
 	}
@@ -420,7 +435,7 @@ func ValidateUpstreamsPrivate(upstreams []string, lnc LocalNetChecker) (err erro
 
 	var errs []error
 
-	for domain := range conf.DomainReservedUpstreams {
+	for _, domain := range stringKeysSorted(conf.DomainReservedUpstreams) {
 		var subnet *net.IPNet
 		subnet, err = netutil.SubnetFromReversedAddr(domain)
 		if err != nil {

internal/dnsforward/http_test.go

@@ -183,8 +183,9 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
 		name:    "upstream_mode_fastest_addr",
 		wantSet: "",
 	}, {
-		name:    "upstream_dns_bad",
-		wantSet: `bad ipport address "!!!": address !!!: missing port in address`,
+		name: "upstream_dns_bad",
+		wantSet: `validating upstream servers: bad ipport address "!!!": ` +
+			`address !!!: missing port in address`,
 	}, {
 		name: "bootstraps_bad",
 		wantSet: `checking bootstrap a: invalid address: ` +
@@ -198,6 +199,10 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
 	}, {
 		name:    "local_ptr_upstreams_good",
 		wantSet: "",
+	}, {
+		name: "local_ptr_upstreams_bad",
+		wantSet: `validating private upstream servers: checking domain-specific upstreams: ` +
+			`bad arpa domain name "non.arpa": not a reversed ip network`,
 	}, {
 		name:    "local_ptr_upstreams_null",
 		wantSet: "",
@@ -430,8 +435,8 @@ func TestValidateUpstreamsPrivate(t *testing.T) {
 	}, {
 		name: "several_bad",
 		wantErr: `checking domain-specific upstreams: 2 errors: ` +
-			`"bad arpa domain name \"non.arpa\": not a reversed ip network", ` +
-			`"arpa domain \"1.2.3.4.in-addr.arpa.\" should point to a locally-served network"`,
+			`"arpa domain \"1.2.3.4.in-addr.arpa.\" should point to a locally-served network", ` +
+			`"bad arpa domain name \"non.arpa\": not a reversed ip network"`,
 		u: "[/non.arpa/1.2.3.4.in-addr.arpa/127.in-addr.arpa/]#",
 	}}
 

internal/dnsforward/testdata/TestDNSForwardHTTP_handleSetConfig.json

@@ -520,6 +520,43 @@
       ]
     }
   },
+  "local_ptr_upstreams_bad": {
+    "req": {
+      "local_ptr_upstreams": [
+        "123.123.123.123",
+        "[/non.arpa/]#"
+      ]
+    },
+    "want": {
+      "upstream_dns": [
+        "8.8.8.8:53",
+        "8.8.4.4:53"
+      ],
+      "upstream_dns_file": "",
+      "bootstrap_dns": [
+        "9.9.9.10",
+        "149.112.112.10",
+        "2620:fe::10",
+        "2620:fe::fe:10"
+      ],
+      "protection_enabled": true,
+      "ratelimit": 0,
+      "blocking_mode": "",
+      "blocking_ipv4": "",
+      "blocking_ipv6": "",
+      "edns_cs_enabled": false,
+      "dnssec_enabled": false,
+      "disable_ipv6": false,
+      "upstream_mode": "",
+      "cache_size": 0,
+      "cache_ttl_min": 0,
+      "cache_ttl_max": 0,
+      "cache_optimistic": false,
+      "resolve_clients": false,
+      "use_private_ptr_resolvers": false,
+      "local_ptr_upstreams": []
+    }
+  },
   "local_ptr_upstreams_null": {
     "req": {
       "local_ptr_upstreams": null