From 6c18b71010cf28e1dfe147fd6400058f6e9224be Mon Sep 17 00:00:00 2001
From: Andrey Meshkov <am@adguard.com>
Date: Wed, 5 Feb 2020 14:30:43 +0300
Subject: [PATCH] *(dnsforward): fix safe search returning nxdomain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

✅ Closes: https://github.com/AdguardTeam/AdGuardHome/issues/1387
---
 dnsfilter/dnsfilter.go   |  2 +-
 dnsfilter/security.go    | 16 ++++++++--------
 dnsforward/dnsforward.go | 16 ++++++++++++++++
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/dnsfilter/dnsfilter.go b/dnsfilter/dnsfilter.go
index 5ae9d523..ad73ec96 100644
--- a/dnsfilter/dnsfilter.go
+++ b/dnsfilter/dnsfilter.go
@@ -225,7 +225,7 @@ func (d *Dnsfilter) filtersInitializer() {
 // Close - close the object
 func (d *Dnsfilter) Close() {
 	if d.rulesStorage != nil {
-		d.rulesStorage.Close()
+		_ = d.rulesStorage.Close()
 	}
 }
 
diff --git a/dnsfilter/security.go b/dnsfilter/security.go
index c0341c99..7f2b397a 100644
--- a/dnsfilter/security.go
+++ b/dnsfilter/security.go
@@ -127,8 +127,8 @@ func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) {
 	res := Result{IsFiltered: true, Reason: FilteredSafeSearch}
 	if ip := net.ParseIP(safeHost); ip != nil {
 		res.IP = ip
-		len := d.setCacheResult(gctx.safeSearchCache, host, res)
-		log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, len)
+		valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
+		log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
 		return res, nil
 	}
 
@@ -151,8 +151,8 @@ func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) {
 	}
 
 	// Cache result
-	len := d.setCacheResult(gctx.safeSearchCache, host, res)
-	log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, len)
+	valLen := d.setCacheResult(gctx.safeSearchCache, host, res)
+	log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen)
 	return res, nil
 }
 
@@ -243,8 +243,8 @@ func (d *Dnsfilter) checkSafeBrowsing(host string) (Result, error) {
 		result.Rule = "adguard-malware-shavar"
 	}
 
-	len := d.setCacheResult(gctx.safebrowsingCache, host, result)
-	log.Debug("SafeBrowsing: stored in cache: %s (%d bytes)", host, len)
+	valLen := d.setCacheResult(gctx.safebrowsingCache, host, result)
+	log.Debug("SafeBrowsing: stored in cache: %s (%d bytes)", host, valLen)
 	return result, nil
 }
 
@@ -283,8 +283,8 @@ func (d *Dnsfilter) checkParental(host string) (Result, error) {
 		result.Rule = "parental CATEGORY_BLACKLISTED"
 	}
 
-	len := d.setCacheResult(gctx.parentalCache, host, result)
-	log.Debug("Parental: stored in cache: %s (%d bytes)", host, len)
+	valLen := d.setCacheResult(gctx.parentalCache, host, result)
+	log.Debug("Parental: stored in cache: %s (%d bytes)", host, valLen)
 	return result, err
 }
 
diff --git a/dnsforward/dnsforward.go b/dnsforward/dnsforward.go
index 1afdc262..7c001e27 100644
--- a/dnsforward/dnsforward.go
+++ b/dnsforward/dnsforward.go
@@ -883,7 +883,16 @@ func (s *Server) genDNSFilterMessage(d *proxy.DNSContext, result *dnsfilter.Resu
 	case dnsfilter.FilteredParental:
 		return s.genBlockedHost(m, s.conf.ParentalBlockHost, d)
 	default:
+		// If the query was filtered by "Safe search", dnsfilter also must return
+		// the IP address that must be used in response.
+		// In this case regardless of the filtering method, we should return it
+		if result.Reason == dnsfilter.FilteredSafeSearch && result.IP != nil {
+			return s.genResponseWithIP(m, result.IP)
+		}
+
 		if s.conf.BlockingMode == "null_ip" {
+			// it means that we should return 0.0.0.0 or :: for any blocked request
+
 			switch m.Question[0].Qtype {
 			case dns.TypeA:
 				return s.genARecord(m, []byte{0, 0, 0, 0})
@@ -892,6 +901,8 @@ func (s *Server) genDNSFilterMessage(d *proxy.DNSContext, result *dnsfilter.Resu
 			}
 
 		} else if s.conf.BlockingMode == "custom_ip" {
+			// means that we should return custom IP for any blocked request
+
 			switch m.Question[0].Qtype {
 			case dns.TypeA:
 				return s.genARecord(m, s.conf.BlockingIPAddrv4)
@@ -900,9 +911,14 @@ func (s *Server) genDNSFilterMessage(d *proxy.DNSContext, result *dnsfilter.Resu
 			}
 
 		} else if s.conf.BlockingMode == "nxdomain" {
+			// means that we should return NXDOMAIN for any blocked request
+
 			return s.genNXDomain(m)
 		}
 
+		// Default blocking mode
+		// If there's an IP specified in the rule, return it
+		// If there is no IP, return NXDOMAIN
 		if result.IP != nil {
 			return s.genResponseWithIP(m, result.IP)
 		}