From 7b8588afa4409117f88eaeffd3e8ebfdc508e72f Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Sun, 5 Apr 2020 18:34:43 +0300 Subject: [PATCH] *: fix golangci-lint warnings --- .golangci.yml | 4 ++- dhcpd/dhcpd.go | 20 +++++------ dhcpd/network_utils.go | 16 ++++----- dhcpd/standalone/main.go | 2 +- dnsforward/dnsforward.go | 2 +- home/clients.go | 16 ++++----- home/control_update.go | 18 +++++----- home/dns.go | 2 +- home/filter.go | 6 ++-- home/tls.go | 4 +++ home/web.go | 76 ++++++++++++++++++++-------------------- util/network_utils.go | 2 +- 12 files changed, 86 insertions(+), 82 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fdeed11c..341a62d4 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -64,4 +64,6 @@ issues: - Error return value of .s.closeConn. is not checked - Error return value of ..*.Shutdown. # goconst - - string .forcesafesearch.google.com. has 3 occurrences \ No newline at end of file + - string .forcesafesearch.google.com. has 3 occurrences + # gosec: Subprocess launched with function call as argument or cmd arguments + - G204 \ No newline at end of file diff --git a/dhcpd/dhcpd.go b/dhcpd/dhcpd.go index b40a8ae8..2f1725b3 100644 --- a/dhcpd/dhcpd.go +++ b/dhcpd/dhcpd.go @@ -227,7 +227,7 @@ func (s *Server) Start() error { // TODO: don't close if interface and addresses are the same if s.conn != nil { - s.closeConn() + _ = s.closeConn() } iface, err := net.InterfaceByName(s.conf.InterfaceName) @@ -251,7 +251,7 @@ func (s *Server) Start() error { if err != nil && !s.stopping { log.Printf("dhcp4.Serve() returned with error: %s", err) } - c.Close() // in case Serve() exits for other reason than listening socket closure + _ = c.Close() // in case Serve() exits for other reason than listening socket closure s.running = false s.cond.Signal() }() @@ -609,10 +609,10 @@ func (s *Server) handleDecline(p dhcp4.Packet, options dhcp4.Options) dhcp4.Pack // AddStaticLease adds a static lease (thread-safe) func (s *Server) AddStaticLease(l Lease) error { if len(l.IP) != 4 { - return fmt.Errorf("Invalid IP") + return fmt.Errorf("invalid IP") } if len(l.HWAddr) != 6 { - return fmt.Errorf("Invalid MAC") + return fmt.Errorf("invalid MAC") } l.Expiry = time.Unix(leaseExpireStatic, 0) @@ -637,9 +637,9 @@ func (s *Server) AddStaticLease(l Lease) error { func (s *Server) rmDynamicLeaseWithIP(ip net.IP) error { var newLeases []*Lease for _, lease := range s.leases { - if bytes.Equal(lease.IP.To4(), ip) { + if net.IP.Equal(lease.IP.To4(), ip) { if lease.Expiry.Unix() == leaseExpireStatic { - return fmt.Errorf("Static lease with the same IP already exists") + return fmt.Errorf("static lease with the same IP already exists") } continue } @@ -654,7 +654,7 @@ func (s *Server) rmDynamicLeaseWithIP(ip net.IP) error { func (s *Server) rmLease(l Lease) error { var newLeases []*Lease for _, lease := range s.leases { - if bytes.Equal(lease.IP.To4(), l.IP) { + if net.IP.Equal(lease.IP.To4(), l.IP) { if !bytes.Equal(lease.HWAddr, l.HWAddr) || lease.Hostname != l.Hostname { return fmt.Errorf("Lease not found") @@ -671,17 +671,17 @@ func (s *Server) rmLease(l Lease) error { // RemoveStaticLease removes a static lease (thread-safe) func (s *Server) RemoveStaticLease(l Lease) error { if len(l.IP) != 4 { - return fmt.Errorf("Invalid IP") + return fmt.Errorf("invalid IP") } if len(l.HWAddr) != 6 { - return fmt.Errorf("Invalid MAC") + return fmt.Errorf("invalid MAC") } s.leasesLock.Lock() if s.findReservedHWaddr(l.IP) == nil { s.leasesLock.Unlock() - return fmt.Errorf("Lease not found") + return fmt.Errorf("lease not found") } err := s.rmLease(l) diff --git a/dhcpd/network_utils.go b/dhcpd/network_utils.go index 79c13285..c7a7fae8 100644 --- a/dhcpd/network_utils.go +++ b/dhcpd/network_utils.go @@ -33,7 +33,7 @@ func HasStaticIP(ifaceName string) (bool, error) { return hasStaticIPDarwin(ifaceName) } - return false, fmt.Errorf("Cannot check if IP is static: not supported on %s", runtime.GOOS) + return false, fmt.Errorf("cannot check if IP is static: not supported on %s", runtime.GOOS) } // Set a static IP for the specified network interface @@ -46,7 +46,7 @@ func SetStaticIP(ifaceName string) error { return setStaticIPDarwin(ifaceName) } - return fmt.Errorf("Cannot set static IP on %s", runtime.GOOS) + return fmt.Errorf("cannot set static IP on %s", runtime.GOOS) } // for dhcpcd.conf @@ -114,7 +114,7 @@ func getGatewayIP(ifaceName string) string { func setStaticIPDhcpdConf(ifaceName string) error { ip := util.GetSubnet(ifaceName) if len(ip) == 0 { - return errors.New("Can't get IP address") + return errors.New("can't get IP address") } ip4, _, err := net.ParseCIDR(ip) @@ -198,7 +198,7 @@ func setStaticIPDarwin(ifaceName string) error { return err } if code != 0 { - return fmt.Errorf("Failed to set DNS servers, code=%d", code) + return fmt.Errorf("failed to set DNS servers, code=%d", code) } // Actually configures hardware port to have static IP @@ -208,7 +208,7 @@ func setStaticIPDarwin(ifaceName string) error { return err } if code != 0 { - return fmt.Errorf("Failed to set DNS servers, code=%d", code) + return fmt.Errorf("failed to set DNS servers, code=%d", code) } return nil @@ -220,7 +220,7 @@ func getCurrentHardwarePortInfo(ifaceName string) (hardwarePortInfo, error) { m := getNetworkSetupHardwareReports() hardwarePort, ok := m[ifaceName] if !ok { - return hardwarePortInfo{}, fmt.Errorf("Could not find hardware port for %s", ifaceName) + return hardwarePortInfo{}, fmt.Errorf("could not find hardware port for %s", ifaceName) } return getHardwarePortInfo(hardwarePort) @@ -274,7 +274,7 @@ func getHardwarePortInfo(hardwarePort string) (hardwarePortInfo, error) { match := re.FindStringSubmatch(out) if len(match) == 0 { - return h, errors.New("Could not find hardware port info") + return h, errors.New("could not find hardware port info") } h.name = hardwarePort @@ -300,7 +300,7 @@ func getEtcResolvConfServers() ([]string, error) { matches := re.FindAllStringSubmatch(string(body), -1) if len(matches) == 0 { - return nil, errors.New("Found no DNS servers in /etc/resolv.conf") + return nil, errors.New("found no DNS servers in /etc/resolv.conf") } addrs := make([]string, 0) diff --git a/dhcpd/standalone/main.go b/dhcpd/standalone/main.go index d84e3339..a9aab9ee 100644 --- a/dhcpd/standalone/main.go +++ b/dhcpd/standalone/main.go @@ -80,7 +80,7 @@ func main() { panic(err) } log.Printf("Now serving DHCP") - signalChannel := make(chan os.Signal) + signalChannel := make(chan os.Signal, 1) signal.Notify(signalChannel, syscall.SIGINT, syscall.SIGTERM) <-signalChannel diff --git a/dnsforward/dnsforward.go b/dnsforward/dnsforward.go index 9c9121c5..de396e9f 100644 --- a/dnsforward/dnsforward.go +++ b/dnsforward/dnsforward.go @@ -407,7 +407,7 @@ func matchDNSName(dnsNames []string, sni string) bool { func (s *Server) onGetCertificate(ch *tls.ClientHelloInfo) (*tls.Certificate, error) { if s.conf.StrictSNICheck && !matchDNSName(s.conf.dnsNames, ch.ServerName) { log.Info("DNS: TLS: unknown SNI in Client Hello: %s", ch.ServerName) - return nil, fmt.Errorf("Invalid SNI") + return nil, fmt.Errorf("invalid SNI") } return &s.conf.cert, nil } diff --git a/home/clients.go b/home/clients.go index b943d25d..90750776 100644 --- a/home/clients.go +++ b/home/clients.go @@ -375,7 +375,7 @@ func (clients *clientsContainer) FindAutoClient(ip string) (ClientHost, bool) { // Check if Client object's fields are correct func (clients *clientsContainer) check(c *Client) error { if len(c.Name) == 0 { - return fmt.Errorf("Invalid Name") + return fmt.Errorf("invalid Name") } if len(c.IDs) == 0 { @@ -399,12 +399,12 @@ func (clients *clientsContainer) check(c *Client) error { continue } - return fmt.Errorf("Invalid ID: %s", id) + return fmt.Errorf("invalid ID: %s", id) } for _, t := range c.Tags { if !clients.tagKnown(t) { - return fmt.Errorf("Invalid tag: %s", t) + return fmt.Errorf("invalid tag: %s", t) } } sort.Strings(c.Tags) @@ -412,7 +412,7 @@ func (clients *clientsContainer) check(c *Client) error { if len(c.Upstreams) != 0 { err := dnsforward.ValidateUpstreams(c.Upstreams) if err != nil { - return fmt.Errorf("Invalid upstream servers: %s", err) + return fmt.Errorf("invalid upstream servers: %s", err) } } @@ -440,7 +440,7 @@ func (clients *clientsContainer) Add(c Client) (bool, error) { for _, id := range c.IDs { c2, ok := clients.idIndex[id] if ok { - return false, fmt.Errorf("Another client uses the same ID (%s): %s", id, c2.Name) + return false, fmt.Errorf("another client uses the same ID (%s): %s", id, c2.Name) } } @@ -501,14 +501,14 @@ func (clients *clientsContainer) Update(name string, c Client) error { old, ok := clients.list[name] if !ok { - return fmt.Errorf("Client not found") + return fmt.Errorf("client not found") } // check Name index if old.Name != c.Name { _, ok = clients.list[c.Name] if ok { - return fmt.Errorf("Client already exists") + return fmt.Errorf("client already exists") } } @@ -517,7 +517,7 @@ func (clients *clientsContainer) Update(name string, c Client) error { for _, id := range c.IDs { c2, ok := clients.idIndex[id] if ok && c2 != old { - return fmt.Errorf("Another client uses the same ID (%s): %s", id, c2.Name) + return fmt.Errorf("another client uses the same ID (%s): %s", id, c2.Name) } } diff --git a/home/control_update.go b/home/control_update.go index 0eaff619..b0080e44 100644 --- a/home/control_update.go +++ b/home/control_update.go @@ -112,6 +112,9 @@ func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) { for i := 0; i != 3; i++ { log.Tracef("Downloading data from %s", versionCheckURL) resp, err = Context.client.Get(versionCheckURL) + if resp != nil && resp.Body != nil { + defer resp.Body.Close() + } if err != nil && strings.HasSuffix(err.Error(), "i/o timeout") { // This case may happen while we're restarting DNS server // https://github.com/AdguardTeam/AdGuardHome/issues/934 @@ -123,9 +126,6 @@ func handleGetVersionJSON(w http.ResponseWriter, r *http.Request) { httpError(w, http.StatusBadGateway, "Couldn't get version check json from %s: %T %s\n", versionCheckURL, err, err) return } - if resp != nil && resp.Body != nil { - defer resp.Body.Close() - } // read the body entirely body, err := ioutil.ReadAll(resp.Body) @@ -187,11 +187,11 @@ func getUpdateInfo(jsonData []byte) (*updateInfo, error) { u.pkgURL = versionJSON[fmt.Sprintf("download_%s_%s", runtime.GOOS, runtime.GOARCH)].(string) u.newVer = versionJSON["version"].(string) if len(u.pkgURL) == 0 || len(u.newVer) == 0 { - return nil, fmt.Errorf("Invalid JSON") + return nil, fmt.Errorf("invalid JSON") } if u.newVer == versionString { - return nil, fmt.Errorf("No need to update") + return nil, fmt.Errorf("no need to update") } u.updateDir = filepath.Join(workDir, fmt.Sprintf("agh-update-%s", u.newVer)) @@ -199,7 +199,7 @@ func getUpdateInfo(jsonData []byte) (*updateInfo, error) { _, pkgFileName := filepath.Split(u.pkgURL) if len(pkgFileName) == 0 { - return nil, fmt.Errorf("Invalid JSON") + return nil, fmt.Errorf("invalid JSON") } u.pkgName = filepath.Join(u.updateDir, pkgFileName) @@ -215,7 +215,7 @@ func getUpdateInfo(jsonData []byte) (*updateInfo, error) { } u.curBinName = filepath.Join(workDir, binName) if !util.FileExists(u.curBinName) { - return nil, fmt.Errorf("Executable file %s doesn't exist", u.curBinName) + return nil, fmt.Errorf("executable file %s doesn't exist", u.curBinName) } u.bkpBinName = filepath.Join(u.backupDir, binName) u.newBinName = filepath.Join(u.updateDir, "AdGuardHome", binName) @@ -432,7 +432,7 @@ func doUpdate(u *updateInfo) error { return fmt.Errorf("targzFileUnpack() failed: %s", err) } } else { - return fmt.Errorf("Unknown package extension") + return fmt.Errorf("unknown package extension") } log.Tracef("Checking configuration") @@ -517,9 +517,7 @@ func finishUpdate(u *updateInfo) { log.Fatalf("exec.Command() failed: %s", err) } os.Exit(0) - } else { - log.Info("Restarting: %v", os.Args) err := syscall.Exec(u.curBinName, os.Args, os.Environ()) if err != nil { diff --git a/home/dns.go b/home/dns.go index f1a5a501..b03d0de3 100644 --- a/home/dns.go +++ b/home/dns.go @@ -37,7 +37,7 @@ func initDNSServer() error { } Context.stats, err = stats.New(statsConf) if err != nil { - return fmt.Errorf("Couldn't initialize statistics module") + return fmt.Errorf("couldn't initialize statistics module") } conf := querylog.Config{ Enabled: config.DNS.QueryLogEnabled, diff --git a/home/filter.go b/home/filter.go index af03e230..3bfc47c2 100644 --- a/home/filter.go +++ b/home/filter.go @@ -290,7 +290,7 @@ func (f *Filtering) periodicallyRefreshFilters() { func (f *Filtering) refreshFilters(flags int, important bool) (int, error) { set := atomic.CompareAndSwapUint32(&f.refreshStatus, 0, 1) if !important && !set { - return 0, fmt.Errorf("Filters update procedure is already running") + return 0, fmt.Errorf("filters update procedure is already running") } f.refreshLock.Lock() @@ -550,13 +550,13 @@ func (f *Filtering) updateIntl(filter *filter) (bool, error) { if firstChunkLen == len(firstChunk) || err == io.EOF { if !isPrintableText(firstChunk) { - return false, fmt.Errorf("Data contains non-printable characters") + return false, fmt.Errorf("data contains non-printable characters") } s := strings.ToLower(string(firstChunk)) if strings.Index(s, "= 0 || strings.Index(s, "= 0 { - return false, fmt.Errorf("Data is HTML, not plain text") + return false, fmt.Errorf("data is HTML, not plain text") } htmlTest = false diff --git a/home/tls.go b/home/tls.go index d5836a31..1e6267fe 100644 --- a/home/tls.go +++ b/home/tls.go @@ -306,6 +306,8 @@ func verifyCertChain(data *tlsConfigStatus, certChain string, serverName string) if decoded.Type == "CERTIFICATE" { certs = append(certs, decoded) } else { + // ignore "this result of append is never used" warning + // nolint skippedBytes = append(skippedBytes, decoded.Type) } } @@ -387,6 +389,8 @@ func validatePkey(data *tlsConfigStatus, pkey string) error { key = decoded break } else { + // ignore "this result of append is never used" + // nolint skippedBytes = append(skippedBytes, decoded.Type) } } diff --git a/home/web.go b/home/web.go index a6c515e0..04b2696e 100644 --- a/home/web.go +++ b/home/web.go @@ -81,11 +81,11 @@ func WebCheckPortAvailable(port int) bool { } // TLSConfigChanged - called when TLS configuration has changed -func (w *Web) TLSConfigChanged(tlsConf tlsConfigSettings) { +func (web *Web) TLSConfigChanged(tlsConf tlsConfigSettings) { log.Debug("Web: applying new TLS configuration") - w.conf.PortHTTPS = tlsConf.PortHTTPS - w.forceHTTPS = (tlsConf.ForceHTTPS && tlsConf.Enabled && tlsConf.PortHTTPS != 0) - w.portHTTPS = tlsConf.PortHTTPS + web.conf.PortHTTPS = tlsConf.PortHTTPS + web.forceHTTPS = (tlsConf.ForceHTTPS && tlsConf.Enabled && tlsConf.PortHTTPS != 0) + web.portHTTPS = tlsConf.PortHTTPS enabled := tlsConf.Enabled && tlsConf.PortHTTPS != 0 && @@ -100,31 +100,31 @@ func (w *Web) TLSConfigChanged(tlsConf tlsConfigSettings) { } } - w.httpsServer.cond.L.Lock() - if w.httpsServer.server != nil { - w.httpsServer.server.Shutdown(context.TODO()) + web.httpsServer.cond.L.Lock() + if web.httpsServer.server != nil { + _ = web.httpsServer.server.Shutdown(context.TODO()) } - w.httpsServer.enabled = enabled - w.httpsServer.cert = cert - w.httpsServer.cond.Broadcast() - w.httpsServer.cond.L.Unlock() + web.httpsServer.enabled = enabled + web.httpsServer.cert = cert + web.httpsServer.cond.Broadcast() + web.httpsServer.cond.L.Unlock() } // Start - start serving HTTP requests -func (w *Web) Start() { +func (web *Web) Start() { // for https, we have a separate goroutine loop - go w.httpServerLoop() + go web.httpServerLoop() // this loop is used as an ability to change listening host and/or port - for !w.httpsServer.shutdown { + for !web.httpsServer.shutdown { printHTTPAddresses("http") // we need to have new instance, because after Shutdown() the Server is not usable - address := net.JoinHostPort(w.conf.BindHost, strconv.Itoa(w.conf.BindPort)) - w.httpServer = &http.Server{ + address := net.JoinHostPort(web.conf.BindHost, strconv.Itoa(web.conf.BindPort)) + web.httpServer = &http.Server{ Addr: address, } - err := w.httpServer.ListenAndServe() + err := web.httpServer.ListenAndServe() if err != http.ErrServerClosed { cleanupAlways() log.Fatal(err) @@ -134,46 +134,46 @@ func (w *Web) Start() { } // Close - stop HTTP server, possibly waiting for all active connections to be closed -func (w *Web) Close() { +func (web *Web) Close() { log.Info("Stopping HTTP server...") - w.httpsServer.cond.L.Lock() - w.httpsServer.shutdown = true - w.httpsServer.cond.L.Unlock() - if w.httpsServer.server != nil { - _ = w.httpsServer.server.Shutdown(context.TODO()) + web.httpsServer.cond.L.Lock() + web.httpsServer.shutdown = true + web.httpsServer.cond.L.Unlock() + if web.httpsServer.server != nil { + _ = web.httpsServer.server.Shutdown(context.TODO()) } - if w.httpServer != nil { - _ = w.httpServer.Shutdown(context.TODO()) + if web.httpServer != nil { + _ = web.httpServer.Shutdown(context.TODO()) } log.Info("Stopped HTTP server") } -func (w *Web) httpServerLoop() { +func (web *Web) httpServerLoop() { for { - w.httpsServer.cond.L.Lock() - if w.httpsServer.shutdown { - w.httpsServer.cond.L.Unlock() + web.httpsServer.cond.L.Lock() + if web.httpsServer.shutdown { + web.httpsServer.cond.L.Unlock() break } // this mechanism doesn't let us through until all conditions are met - for !w.httpsServer.enabled { // sleep until necessary data is supplied - w.httpsServer.cond.Wait() - if w.httpsServer.shutdown { - w.httpsServer.cond.L.Unlock() + for !web.httpsServer.enabled { // sleep until necessary data is supplied + web.httpsServer.cond.Wait() + if web.httpsServer.shutdown { + web.httpsServer.cond.L.Unlock() return } } - w.httpsServer.cond.L.Unlock() + web.httpsServer.cond.L.Unlock() // prepare HTTPS server - address := net.JoinHostPort(w.conf.BindHost, strconv.Itoa(w.conf.PortHTTPS)) - w.httpsServer.server = &http.Server{ + address := net.JoinHostPort(web.conf.BindHost, strconv.Itoa(web.conf.PortHTTPS)) + web.httpsServer.server = &http.Server{ Addr: address, TLSConfig: &tls.Config{ - Certificates: []tls.Certificate{w.httpsServer.cert}, + Certificates: []tls.Certificate{web.httpsServer.cert}, MinVersion: tls.VersionTLS12, RootCAs: Context.tlsRoots, CipherSuites: Context.tlsCiphers, @@ -181,7 +181,7 @@ func (w *Web) httpServerLoop() { } printHTTPAddresses("https") - err := w.httpsServer.server.ListenAndServeTLS("", "") + err := web.httpsServer.server.ListenAndServeTLS("", "") if err != http.ErrServerClosed { cleanupAlways() log.Fatal(err) diff --git a/util/network_utils.go b/util/network_utils.go index 4683b39d..a0e482bc 100644 --- a/util/network_utils.go +++ b/util/network_utils.go @@ -30,7 +30,7 @@ type NetInterface struct { func GetValidNetInterfaces() ([]net.Interface, error) { ifaces, err := net.Interfaces() if err != nil { - return nil, fmt.Errorf("Couldn't get list of interfaces: %s", err) + return nil, fmt.Errorf("couldn't get list of interfaces: %s", err) } netIfaces := []net.Interface{}