From 7f29d4e5463c40fb682c968152f5e9c33c71c3cc Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 7 Dec 2020 15:38:05 +0300 Subject: [PATCH] Pull request: all: fix lint and naming issues vol. 2 Merge in DNS/adguard-home from 2276-fix-lint-2 to master Updates #2276. Squashed commit of the following: commit 24760b9586bb31be134ef9518dbece485560b1a0 Author: Ainar Garipov Date: Mon Dec 7 14:39:50 2020 +0300 all: fix lint and naming issues vol. 2 --- .../{check_other_dhcp.go => checkother.go} | 0 ..._dhcp_windows.go => checkother_windows.go} | 0 internal/dhcpd/dhcpd.go | 2 + internal/dhcpd/nclient4/client.go | 22 --- .../dhcpd/{network_utils.go => netutil.go} | 0 ...{network_utils_test.go => netutil_test.go} | 0 .../dhcpd/{router_adv.go => routeradv.go} | 15 +- .../{router_adv_test.go => routeradv_test.go} | 0 internal/dhcpd/v46_windows.go | 5 +- internal/dhcpd/v6.go | 8 +- .../{blocked_services.go => blocked.go} | 0 internal/dnsfilter/dnsfilter.go | 25 ++- internal/dnsfilter/safe_search.go | 148 ------------------ internal/dnsfilter/safesearch.go | 147 +++++++++++++++++ internal/dnsfilter/{sb_pc.go => sbpc.go} | 0 .../dnsfilter/{sb_pc_test.go => sbpc_test.go} | 0 internal/dnsforward/access_test.go | 3 +- internal/dnsforward/{handle_dns.go => dns.go} | 0 .../{dnsforward_http.go => http.go} | 0 .../{dnsforward_http_test.go => http_test.go} | 0 internal/dnsforward/msg.go | 2 +- 21 files changed, 188 insertions(+), 189 deletions(-) rename internal/dhcpd/{check_other_dhcp.go => checkother.go} (100%) rename internal/dhcpd/{check_other_dhcp_windows.go => checkother_windows.go} (100%) rename internal/dhcpd/{network_utils.go => netutil.go} (100%) rename internal/dhcpd/{network_utils_test.go => netutil_test.go} (100%) rename internal/dhcpd/{router_adv.go => routeradv.go} (96%) rename internal/dhcpd/{router_adv_test.go => routeradv_test.go} (100%) rename internal/dnsfilter/{blocked_services.go => blocked.go} (100%) delete mode 100644 internal/dnsfilter/safe_search.go rename internal/dnsfilter/{sb_pc.go => sbpc.go} (100%) rename internal/dnsfilter/{sb_pc_test.go => sbpc_test.go} (100%) rename internal/dnsforward/{handle_dns.go => dns.go} (100%) rename internal/dnsforward/{dnsforward_http.go => http.go} (100%) rename internal/dnsforward/{dnsforward_http_test.go => http_test.go} (100%) diff --git a/internal/dhcpd/check_other_dhcp.go b/internal/dhcpd/checkother.go similarity index 100% rename from internal/dhcpd/check_other_dhcp.go rename to internal/dhcpd/checkother.go diff --git a/internal/dhcpd/check_other_dhcp_windows.go b/internal/dhcpd/checkother_windows.go similarity index 100% rename from internal/dhcpd/check_other_dhcp_windows.go rename to internal/dhcpd/checkother_windows.go diff --git a/internal/dhcpd/dhcpd.go b/internal/dhcpd/dhcpd.go index 999e9c7a..9371a29a 100644 --- a/internal/dhcpd/dhcpd.go +++ b/internal/dhcpd/dhcpd.go @@ -52,6 +52,7 @@ type ServerConfig struct { HTTPRegister func(string, string, func(http.ResponseWriter, *http.Request)) `yaml:"-"` } +// OnLeaseChangedT is a callback for lease changes. type OnLeaseChangedT func(flags int) // flags for onLeaseChanged() @@ -74,6 +75,7 @@ type Server struct { onLeaseChanged []OnLeaseChangedT } +// ServerInterface is an interface for servers. type ServerInterface interface { Leases(flags int) []Lease SetOnLeaseChanged(onLeaseChanged OnLeaseChangedT) diff --git a/internal/dhcpd/nclient4/client.go b/internal/dhcpd/nclient4/client.go index bb0473f1..b7f073c2 100644 --- a/internal/dhcpd/nclient4/client.go +++ b/internal/dhcpd/nclient4/client.go @@ -19,9 +19,7 @@ import ( "context" "errors" "fmt" - "log" "net" - "os" "sync" "sync/atomic" "time" @@ -317,26 +315,6 @@ func WithTimeout(d time.Duration) ClientOpt { } } -// WithSummaryLogger logs one-line DHCPv4 message summaries when sent & received. -func WithSummaryLogger() ClientOpt { - return func(c *Client) (err error) { - c.logger = ShortSummaryLogger{ - Printfer: log.New(os.Stderr, "[dhcpv4] ", log.LstdFlags), - } - return - } -} - -// WithDebugLogger logs multi-line full DHCPv4 messages when sent & received. -func WithDebugLogger() ClientOpt { - return func(c *Client) (err error) { - c.logger = DebugLogger{ - Printfer: log.New(os.Stderr, "[dhcpv4] ", log.LstdFlags), - } - return - } -} - // WithLogger set the logger (see interface Logger). func WithLogger(newLogger Logger) ClientOpt { return func(c *Client) (err error) { diff --git a/internal/dhcpd/network_utils.go b/internal/dhcpd/netutil.go similarity index 100% rename from internal/dhcpd/network_utils.go rename to internal/dhcpd/netutil.go diff --git a/internal/dhcpd/network_utils_test.go b/internal/dhcpd/netutil_test.go similarity index 100% rename from internal/dhcpd/network_utils_test.go rename to internal/dhcpd/netutil_test.go diff --git a/internal/dhcpd/router_adv.go b/internal/dhcpd/routeradv.go similarity index 96% rename from internal/dhcpd/router_adv.go rename to internal/dhcpd/routeradv.go index e3d386f0..f1d63c7d 100644 --- a/internal/dhcpd/router_adv.go +++ b/internal/dhcpd/routeradv.go @@ -180,15 +180,18 @@ func (ra *raCtx) Init() error { data := createICMPv6RAPacket(params) var err error + success := false ipAndScope := ra.ipAddr.String() + "%" + ra.ifaceName ra.conn, err = icmp.ListenPacket("ip6:ipv6-icmp", ipAndScope) if err != nil { return fmt.Errorf("dhcpv6 ra: icmp.ListenPacket: %w", err) } - success := false defer func() { if !success { - ra.Close() + cerr := ra.Close() + if cerr != nil { + log.Error("closing context: %s", cerr) + } } }() @@ -227,13 +230,15 @@ func (ra *raCtx) Init() error { return nil } -// Close - close module -func (ra *raCtx) Close() { +// Close closes the module. +func (ra *raCtx) Close() (err error) { log.Debug("dhcpv6 ra: closing") ra.stop.Store(1) if ra.conn != nil { - ra.conn.Close() + return ra.conn.Close() } + + return nil } diff --git a/internal/dhcpd/router_adv_test.go b/internal/dhcpd/routeradv_test.go similarity index 100% rename from internal/dhcpd/router_adv_test.go rename to internal/dhcpd/routeradv_test.go diff --git a/internal/dhcpd/v46_windows.go b/internal/dhcpd/v46_windows.go index ebae25af..a02d9101 100644 --- a/internal/dhcpd/v46_windows.go +++ b/internal/dhcpd/v46_windows.go @@ -1,11 +1,12 @@ +// +build windows + package dhcpd // 'u-root/u-root' package, a dependency of 'insomniacslk/dhcp' package, doesn't build on Windows import "net" -type winServer struct { -} +type winServer struct{} func (s *winServer) ResetLeases(leases []*Lease) {} func (s *winServer) GetLeases(flags int) []Lease { return nil } diff --git a/internal/dhcpd/v6.go b/internal/dhcpd/v6.go index 7b13dcfd..2dd41b5c 100644 --- a/internal/dhcpd/v6.go +++ b/internal/dhcpd/v6.go @@ -628,7 +628,10 @@ func (s *v6Server) Start() error { // Stop - stop server func (s *v6Server) Stop() { - s.ra.Close() + err := s.ra.Close() + if err != nil { + log.Error("dhcpv6: s.ra.Close: %s", err) + } // DHCPv6 server may not be initialized if ra_slaac_only=true if s.srv == nil { @@ -636,10 +639,11 @@ func (s *v6Server) Stop() { } log.Debug("DHCPv6: stopping") - err := s.srv.Close() + err = s.srv.Close() if err != nil { log.Error("DHCPv6: srv.Close: %s", err) } + // now server.Serve() will return s.srv = nil } diff --git a/internal/dnsfilter/blocked_services.go b/internal/dnsfilter/blocked.go similarity index 100% rename from internal/dnsfilter/blocked_services.go rename to internal/dnsfilter/blocked.go diff --git a/internal/dnsfilter/dnsfilter.go b/internal/dnsfilter/dnsfilter.go index 2b58b02e..855bbf60 100644 --- a/internal/dnsfilter/dnsfilter.go +++ b/internal/dnsfilter/dnsfilter.go @@ -263,11 +263,20 @@ func (d *Dnsfilter) Close() { } func (d *Dnsfilter) reset() { + var err error + if d.rulesStorage != nil { - _ = d.rulesStorage.Close() + err = d.rulesStorage.Close() + if err != nil { + log.Error("dnsfilter: rulesStorage.Close: %s", err) + } } + if d.rulesStorageWhite != nil { - d.rulesStorageWhite.Close() + err = d.rulesStorageWhite.Close() + if err != nil { + log.Error("dnsfilter: rulesStorageWhite.Close: %s", err) + } } } @@ -336,9 +345,9 @@ func (d *Dnsfilter) CheckHost(host string, qtype uint16, setts *RequestFiltering // Now check the hosts file -- do we have any rules for it? // just like DNS rewrites, it has higher priority than filtering rules. if d.Config.AutoHosts != nil { - matched, err := d.checkAutoHosts(host, qtype, &result) + matched := d.checkAutoHosts(host, qtype, &result) if matched { - return result, err + return result, nil } } @@ -403,13 +412,13 @@ func (d *Dnsfilter) CheckHost(host string, qtype uint16, setts *RequestFiltering return Result{}, nil } -func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (matched bool, err error) { +func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (matched bool) { ips := d.Config.AutoHosts.Process(host, qtype) if ips != nil { result.Reason = RewriteEtcHosts result.IPList = ips - return true, nil + return true } revHosts := d.Config.AutoHosts.ProcessReverse(host, qtype) @@ -422,10 +431,10 @@ func (d *Dnsfilter) checkAutoHosts(host string, qtype uint16, result *Result) (m result.ReverseHosts[i] = revHosts[i] + "." } - return true, nil + return true } - return false, nil + return false } // Process rewrites table diff --git a/internal/dnsfilter/safe_search.go b/internal/dnsfilter/safe_search.go deleted file mode 100644 index 6bd6bc94..00000000 --- a/internal/dnsfilter/safe_search.go +++ /dev/null @@ -1,148 +0,0 @@ -package dnsfilter - -import ( - "bytes" - "encoding/binary" - "encoding/gob" - "encoding/json" - "fmt" - "net" - "net/http" - "time" - - "github.com/AdguardTeam/golibs/cache" - "github.com/AdguardTeam/golibs/log" -) - -/* -expire byte[4] -res Result -*/ -func (d *Dnsfilter) setCacheResult(cache cache.Cache, host string, res Result) int { - var buf bytes.Buffer - - expire := uint(time.Now().Unix()) + d.Config.CacheTime*60 - exp := make([]byte, 4) - binary.BigEndian.PutUint32(exp, uint32(expire)) - _, _ = buf.Write(exp) - - enc := gob.NewEncoder(&buf) - err := enc.Encode(res) - if err != nil { - log.Error("gob.Encode(): %s", err) - return 0 - } - val := buf.Bytes() - _ = cache.Set([]byte(host), val) - return len(val) -} - -func getCachedResult(cache cache.Cache, host string) (Result, bool) { - data := cache.Get([]byte(host)) - if data == nil { - return Result{}, false - } - - exp := int(binary.BigEndian.Uint32(data[:4])) - if exp <= int(time.Now().Unix()) { - cache.Del([]byte(host)) - return Result{}, false - } - - var buf bytes.Buffer - buf.Write(data[4:]) - dec := gob.NewDecoder(&buf) - r := Result{} - err := dec.Decode(&r) - if err != nil { - log.Debug("gob.Decode(): %s", err) - return Result{}, false - } - - return r, true -} - -// SafeSearchDomain returns replacement address for search engine -func (d *Dnsfilter) SafeSearchDomain(host string) (string, bool) { - val, ok := safeSearchDomains[host] - return val, ok -} - -func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) { - if log.GetLevel() >= log.DEBUG { - timer := log.StartTimer() - defer timer.LogElapsed("SafeSearch: lookup for %s", host) - } - - // Check cache. Return cached result if it was found - cachedValue, isFound := getCachedResult(gctx.safeSearchCache, host) - if isFound { - // atomic.AddUint64(&gctx.stats.Safesearch.CacheHits, 1) - log.Tracef("SafeSearch: found in cache: %s", host) - return cachedValue, nil - } - - safeHost, ok := d.SafeSearchDomain(host) - if !ok { - return Result{}, nil - } - - res := Result{IsFiltered: true, Reason: FilteredSafeSearch} - if ip := net.ParseIP(safeHost); ip != nil { - res.IP = ip - valLen := d.setCacheResult(gctx.safeSearchCache, host, res) - log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen) - return res, nil - } - - // TODO this address should be resolved with upstream that was configured in dnsforward - addrs, err := net.LookupIP(safeHost) - if err != nil { - log.Tracef("SafeSearchDomain for %s was found but failed to lookup for %s cause %s", host, safeHost, err) - return Result{}, err - } - - for _, i := range addrs { - if ipv4 := i.To4(); ipv4 != nil { - res.IP = ipv4 - break - } - } - - if len(res.IP) == 0 { - return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost) - } - - // Cache result - valLen := d.setCacheResult(gctx.safeSearchCache, host, res) - log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen) - return res, nil -} - -func (d *Dnsfilter) handleSafeSearchEnable(w http.ResponseWriter, r *http.Request) { - d.Config.SafeSearchEnabled = true - d.Config.ConfigModified() -} - -func (d *Dnsfilter) handleSafeSearchDisable(w http.ResponseWriter, r *http.Request) { - d.Config.SafeSearchEnabled = false - d.Config.ConfigModified() -} - -func (d *Dnsfilter) handleSafeSearchStatus(w http.ResponseWriter, r *http.Request) { - data := map[string]interface{}{ - "enabled": d.Config.SafeSearchEnabled, - } - jsonVal, err := json.Marshal(data) - if err != nil { - httpError(r, w, http.StatusInternalServerError, "Unable to marshal status json: %s", err) - return - } - - w.Header().Set("Content-Type", "application/json") - _, err = w.Write(jsonVal) - if err != nil { - httpError(r, w, http.StatusInternalServerError, "Unable to write response json: %s", err) - return - } -} diff --git a/internal/dnsfilter/safesearch.go b/internal/dnsfilter/safesearch.go index db6554d0..9485260b 100644 --- a/internal/dnsfilter/safesearch.go +++ b/internal/dnsfilter/safesearch.go @@ -1,5 +1,152 @@ package dnsfilter +import ( + "bytes" + "encoding/binary" + "encoding/gob" + "encoding/json" + "fmt" + "net" + "net/http" + "time" + + "github.com/AdguardTeam/golibs/cache" + "github.com/AdguardTeam/golibs/log" +) + +/* +expire byte[4] +res Result +*/ +func (d *Dnsfilter) setCacheResult(cache cache.Cache, host string, res Result) int { + var buf bytes.Buffer + + expire := uint(time.Now().Unix()) + d.Config.CacheTime*60 + exp := make([]byte, 4) + binary.BigEndian.PutUint32(exp, uint32(expire)) + _, _ = buf.Write(exp) + + enc := gob.NewEncoder(&buf) + err := enc.Encode(res) + if err != nil { + log.Error("gob.Encode(): %s", err) + return 0 + } + val := buf.Bytes() + _ = cache.Set([]byte(host), val) + return len(val) +} + +func getCachedResult(cache cache.Cache, host string) (Result, bool) { + data := cache.Get([]byte(host)) + if data == nil { + return Result{}, false + } + + exp := int(binary.BigEndian.Uint32(data[:4])) + if exp <= int(time.Now().Unix()) { + cache.Del([]byte(host)) + return Result{}, false + } + + var buf bytes.Buffer + buf.Write(data[4:]) + dec := gob.NewDecoder(&buf) + r := Result{} + err := dec.Decode(&r) + if err != nil { + log.Debug("gob.Decode(): %s", err) + return Result{}, false + } + + return r, true +} + +// SafeSearchDomain returns replacement address for search engine +func (d *Dnsfilter) SafeSearchDomain(host string) (string, bool) { + val, ok := safeSearchDomains[host] + return val, ok +} + +func (d *Dnsfilter) checkSafeSearch(host string) (Result, error) { + if log.GetLevel() >= log.DEBUG { + timer := log.StartTimer() + defer timer.LogElapsed("SafeSearch: lookup for %s", host) + } + + // Check cache. Return cached result if it was found + cachedValue, isFound := getCachedResult(gctx.safeSearchCache, host) + if isFound { + // atomic.AddUint64(&gctx.stats.Safesearch.CacheHits, 1) + log.Tracef("SafeSearch: found in cache: %s", host) + return cachedValue, nil + } + + safeHost, ok := d.SafeSearchDomain(host) + if !ok { + return Result{}, nil + } + + res := Result{IsFiltered: true, Reason: FilteredSafeSearch} + if ip := net.ParseIP(safeHost); ip != nil { + res.IP = ip + valLen := d.setCacheResult(gctx.safeSearchCache, host, res) + log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen) + return res, nil + } + + // TODO this address should be resolved with upstream that was configured in dnsforward + addrs, err := net.LookupIP(safeHost) + if err != nil { + log.Tracef("SafeSearchDomain for %s was found but failed to lookup for %s cause %s", host, safeHost, err) + return Result{}, err + } + + for _, i := range addrs { + if ipv4 := i.To4(); ipv4 != nil { + res.IP = ipv4 + break + } + } + + if len(res.IP) == 0 { + return Result{}, fmt.Errorf("no ipv4 addresses in safe search response for %s", safeHost) + } + + // Cache result + valLen := d.setCacheResult(gctx.safeSearchCache, host, res) + log.Debug("SafeSearch: stored in cache: %s (%d bytes)", host, valLen) + return res, nil +} + +func (d *Dnsfilter) handleSafeSearchEnable(w http.ResponseWriter, r *http.Request) { + d.Config.SafeSearchEnabled = true + d.Config.ConfigModified() +} + +func (d *Dnsfilter) handleSafeSearchDisable(w http.ResponseWriter, r *http.Request) { + d.Config.SafeSearchEnabled = false + d.Config.ConfigModified() +} + +func (d *Dnsfilter) handleSafeSearchStatus(w http.ResponseWriter, r *http.Request) { + data := map[string]interface{}{ + "enabled": d.Config.SafeSearchEnabled, + } + jsonVal, err := json.Marshal(data) + if err != nil { + httpError(r, w, http.StatusInternalServerError, "Unable to marshal status json: %s", err) + return + } + + w.Header().Set("Content-Type", "application/json") + _, err = w.Write(jsonVal) + if err != nil { + httpError(r, w, http.StatusInternalServerError, "Unable to write response json: %s", err) + return + } +} + var safeSearchDomains = map[string]string{ "yandex.com": "213.180.193.56", "yandex.ru": "213.180.193.56", diff --git a/internal/dnsfilter/sb_pc.go b/internal/dnsfilter/sbpc.go similarity index 100% rename from internal/dnsfilter/sb_pc.go rename to internal/dnsfilter/sbpc.go diff --git a/internal/dnsfilter/sb_pc_test.go b/internal/dnsfilter/sbpc_test.go similarity index 100% rename from internal/dnsfilter/sb_pc_test.go rename to internal/dnsfilter/sbpc_test.go diff --git a/internal/dnsforward/access_test.go b/internal/dnsforward/access_test.go index b760d554..250b4931 100644 --- a/internal/dnsforward/access_test.go +++ b/internal/dnsforward/access_test.go @@ -50,7 +50,8 @@ func TestIsBlockedIPDisallowed(t *testing.T) { func TestIsBlockedIPBlockedDomain(t *testing.T) { a := &accessCtx{} - assert.True(t, a.Init(nil, nil, []string{"host1", + assert.True(t, a.Init(nil, nil, []string{ + "host1", "host2", "*.host.com", "||host3.com^", diff --git a/internal/dnsforward/handle_dns.go b/internal/dnsforward/dns.go similarity index 100% rename from internal/dnsforward/handle_dns.go rename to internal/dnsforward/dns.go diff --git a/internal/dnsforward/dnsforward_http.go b/internal/dnsforward/http.go similarity index 100% rename from internal/dnsforward/dnsforward_http.go rename to internal/dnsforward/http.go diff --git a/internal/dnsforward/dnsforward_http_test.go b/internal/dnsforward/http_test.go similarity index 100% rename from internal/dnsforward/dnsforward_http_test.go rename to internal/dnsforward/http_test.go diff --git a/internal/dnsforward/msg.go b/internal/dnsforward/msg.go index d0811df4..d4fd4937 100644 --- a/internal/dnsforward/msg.go +++ b/internal/dnsforward/msg.go @@ -1,12 +1,12 @@ package dnsforward import ( - "log" "net" "time" "github.com/AdguardTeam/AdGuardHome/internal/dnsfilter" "github.com/AdguardTeam/dnsproxy/proxy" + "github.com/AdguardTeam/golibs/log" "github.com/miekg/dns" )