all: imp docs, code

AdguardTeam · May 26, 2021 · 771b7a3 · 771b7a3
1 parent be96089
commit 771b7a3
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,9 +15,8 @@ and this project adheres to
 
 ### Added
 
-- The ability to control using the resolvers for addresses from locally-served
-  networks ([#3184]).
-
+- The ability to completely disable reverse DNS resolving of IPs from
+  locally-served networks ([#3184]).
 - New flag `--local-frontend` to serve dinamically changeable frontend files
   from disk as opposed to the ones that were compiled into the binary.
 

diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go
@@ -428,6 +428,7 @@ func (s *Server) processLocalPTR(ctx *dnsContext) (rc resultCode) {
 			return resultCodeError
 		}
 	}
+
 	if d.Res == nil {
 		d.Res = s.genNXDomain(d.Req)
 

diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go
@@ -168,7 +168,12 @@ func NewCustomServer(internalProxy *proxy.Proxy) *Server {
 	return s
 }
 
-// Close - close object
+// Close gracefully closes the server.  It is safe for concurrent use.
+//
+// TODO(e.burkov): A better approach would be making Stop method waiting for all
+// its workers finished.  But it would require the upstream.Upstream to have the
+// Close method to prevent from hanging while waiting for unresponsive server to
+// respond.
 func (s *Server) Close() {
 	s.serverLock.Lock()
 	defer s.serverLock.Unlock()
@@ -549,16 +554,16 @@ func (s *Server) Reconfigure(config *ServerConfig) error {
 func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	var p *proxy.Proxy
 
-	defer func() {
-		if p != nil {
-			p.ServeHTTP(w, r)
-		}
-	}()
+	func() {
+		s.serverLock.RLock()
+		defer s.serverLock.RUnlock()
 
-	s.serverLock.RLock()
-	defer s.serverLock.RUnlock()
+		p = s.dnsProxy
+	}()
 
-	p = s.dnsProxy
+	if p != nil {
+		p.ServeHTTP(w, r)
+	}
 }
 
 // IsBlockedIP - return TRUE if this client should be blocked

diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go
@@ -117,16 +117,9 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*filtering.Result, error) {
 	return &res, err
 }
 
-// checkHostRulesSync checks host if needed performing an appropriate locks.  It
-// also synchronizes access to s.dnsFilter so it won't be suddenly uninitialized
-// while in use.  This could happen after proxy server has been stopped, but its
-// workers aren't exited yet.
-//
-// TODO(e.burkov): A better approach would be making Stop method waiting for all
-// its workers finished.  But it would require the upstream.Upstream to have the
-// Close method to prevent from hanging while waiting for unresponsive server to
-// respond.
-func (s *Server) checkHostRulesSync(host string, qtype uint16, setts *filtering.Settings) (
+// checkHostRules checks the host against filters.  It is safe for concurrent
+// use.
+func (s *Server) checkHostRules(host string, qtype uint16, setts *filtering.Settings) (
 	r *filtering.Result,
 	err error,
 ) {
@@ -138,8 +131,6 @@ func (s *Server) checkHostRulesSync(host string, qtype uint16, setts *filtering.
 	}
 
 	var res filtering.Result
-	// Checking dnsFilter for nil is already done before calling the
-	// filterDNSResponse method.
 	res, err = s.dnsFilter.CheckHostRules(host, qtype, setts)
 	if err != nil {
 		return nil, err
@@ -173,7 +164,7 @@ func (s *Server) filterDNSResponse(ctx *dnsContext) (*filtering.Result, error) {
 			continue
 		}
 
-		res, err := s.checkHostRulesSync(host, d.Req.Question[0].Qtype, ctx.setts)
+		res, err := s.checkHostRules(host, d.Req.Question[0].Qtype, ctx.setts)
 		if err != nil {
 			return nil, err
 		} else if res == nil {

diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go
@@ -41,7 +41,7 @@ type dnsConfig struct {
 	CacheMinTTL         *uint32   `json:"cache_ttl_min"`
 	CacheMaxTTL         *uint32   `json:"cache_ttl_max"`
 	ResolveClients      *bool     `json:"resolve_clients"`
-	UsePrivateResolvers *bool     `json:"use_private_resolvers"`
+	UsePrivateResolvers *bool     `json:"local_ips_rdns"`
 	LocalPTRUpstreams   *[]string `json:"local_ptr_upstreams"`
 }
 

diff --git a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json
@@ -24,7 +24,7 @@
     "cache_ttl_min": 0,
     "cache_ttl_max": 0,
     "resolve_clients": false,
-    "use_private_resolvers": false,
+    "local_ips_rdns": false,
     "local_ptr_upstreams": []
   },
   "fastest_addr": {
@@ -52,7 +52,7 @@
     "cache_ttl_min": 0,
     "cache_ttl_max": 0,
     "resolve_clients": false,
-    "use_private_resolvers": false,
+    "local_ips_rdns": false,
     "local_ptr_upstreams": []
   },
   "parallel": {
@@ -80,7 +80,7 @@
     "cache_ttl_min": 0,
     "cache_ttl_max": 0,
     "resolve_clients": false,
-    "use_private_resolvers": false,
+    "local_ips_rdns": false,
     "local_ptr_upstreams": []
   }
 }
diff --git a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json
@@ -31,7 +31,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -63,7 +63,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -96,7 +96,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -129,7 +129,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -162,7 +162,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -195,7 +195,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -228,7 +228,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -261,7 +261,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -294,7 +294,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -327,7 +327,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -362,7 +362,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -397,7 +397,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -431,7 +431,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -464,7 +464,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   },
@@ -499,7 +499,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": [
         "123.123.123.123"
       ]
@@ -534,7 +534,7 @@
       "cache_ttl_min": 0,
       "cache_ttl_max": 0,
       "resolve_clients": false,
-      "use_private_resolvers": false,
+      "local_ips_rdns": false,
       "local_ptr_upstreams": []
     }
   }

diff --git a/internal/home/config.go b/internal/home/config.go
@@ -108,9 +108,10 @@ type dnsConfig struct {
 	// ResolveClients enables and disables resolving clients with RDNS.
 	ResolveClients bool `yaml:"resolve_clients"`
 
-	// UsePrivateResolvers defines if the PTR requests for addresses from
-	// locally-served networks should be handled.
-	UsePrivateResolvers bool `yaml:"use_private_resolvers"`
+	// UsePrivateResolvers defines if the PTR requests for unknown addresses
+	// from locally-served networks should be resolved via private PTR
+	// resolvers.
+	UsePrivateResolvers bool `yaml:"use_private_ptr_resolvers"`
 
 	// LocalPTRResolvers is the slice of addresses to be used as upstreams
 	// for PTR queries for locally-served networks.

diff --git a/internal/home/rdns.go b/internal/home/rdns.go
@@ -16,7 +16,8 @@ type RDNS struct {
 	exchanger dnsforward.RDNSExchanger
 	clients   *clientsContainer
 
-	//
+	// usePrivate is used to store the state of current private RDNS
+	// resolving settings and to react to it's changes.
 	usePrivate uint32
 
 	// ipCh used to pass client's IP to rDNS workerLoop.
@@ -61,8 +62,8 @@ func NewRDNS(
 	return rDNS
 }
 
-// ensurePrivateCache checks if the state of private resolvers using is changed
-// and clears the ipCache if so.
+// ensurePrivateCache ensures that the state of the RDNS cache is consistent
+// with the current private client RDNS resolving settings.
 //
 // TODO(e.burkov): Clearing cache each time this value changed is not a perfect
 // approach since only unresolved locally-served addresses should be removed.

diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md
@@ -4,10 +4,10 @@
 
 ## v0.107: API changes
 
-### The field `"use_private_resolvers"` in DNS configuration
+### The field `"use_private_ptr_resolvers"` in DNS configuration
 
-* The new optional field  `"use_private_resolvers"` of `"DNSConfig"` specifies
-  if the DNS server should use `"local_ptr_upstreams"` at all.
+* The new optional field  `"use_private_ptr_resolvers"` of `"DNSConfig"`
+  specifies if the DNS server should use `"local_ptr_upstreams"` at all.
 
 ## v0.106: API changes
 

diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml
@@ -1300,7 +1300,7 @@
           - ''
           - 'parallel'
           - 'fastest_addr'
-        'use_private_resolvers':
+        'local_ips_rdns':
           'type': 'boolean'
         'resolve_clients':
           'type': 'boolean'