From ec6b1f7c42c8d2fc413d29fba55430b89fcbce2d Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Fri, 25 Jan 2019 16:01:27 +0300 Subject: [PATCH] Added golangci-lint configuration and prepared for the integrattion --- .golangci.yml | 55 +++++++++++++++++++++++++++++++++++++ config.go | 2 +- control.go | 43 ++++++++++++++++------------- dhcp.go | 8 +++--- dhcpd/check_other_dhcp.go | 2 +- dhcpd/dhcpd.go | 13 +++++---- dhcpd/standalone/main.go | 6 ++-- dns.go | 2 +- dnsfilter/dnsfilter_test.go | 4 +-- dnsforward/querylog.go | 7 ++++- dnsforward/stats.go | 4 +-- helpers.go | 2 +- upgrade.go | 2 +- 13 files changed, 108 insertions(+), 42 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..3cdceded --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,55 @@ +# options for analysis running +run: + # default concurrency is a available CPU number + concurrency: 4 + + # timeout for analysis, e.g. 30s, 5m, default is 1m + deadline: 2m + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + skip-files: + - ".*generated.*" + - dnsfilter/rule_to_regexp.go + + +# all available settings of specific linters +linters-settings: + errcheck: + # [deprecated] comma-separated list of pairs of the form pkg:regex + # the regex is used to ignore names within pkg. (default "fmt:.*"). + # see https://github.com/kisielk/errcheck#the-deprecated-method for details + ignore: fmt:.*,net:SetReadDeadline,net/http:^Write + gocyclo: + min-complexity: 20 + lll: + line-length: 200 + +linters: + enable-all: true + disable: + - interfacer + - gocritic + - scopelint + - gochecknoglobals + - gochecknoinits + - prealloc + - maligned + - goconst # disabled until it's possible to configure + fast: true + +issues: + # List of regexps of issue texts to exclude, empty list by default. + # But independently from this option we use default exclude patterns, + # it can be disabled by `exclude-use-default: false`. To list all + # excluded by default patterns execute `golangci-lint run --help` + exclude: + # structcheck cannot detect usages while they're there + - .parentalServer. is unused + - .safeBrowsingServer. is unused + # errcheck + - Error return value of .s.closeConn. is not checked + # goconst + - string .forcesafesearch.google.com. has 3 occurrences \ No newline at end of file diff --git a/config.go b/config.go index 040acf6e..ed96e874 100644 --- a/config.go +++ b/config.go @@ -10,7 +10,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/dnsfilter" "github.com/AdguardTeam/AdGuardHome/dnsforward" "github.com/hmage/golibs/log" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" ) const ( diff --git a/control.go b/control.go index a60421e8..4600c184 100644 --- a/control.go +++ b/control.go @@ -15,7 +15,7 @@ import ( "github.com/AdguardTeam/dnsproxy/upstream" "github.com/hmage/golibs/log" "github.com/miekg/dns" - "gopkg.in/asaskevich/govalidator.v4" + govalidator "gopkg.in/asaskevich/govalidator.v4" ) const updatePeriod = time.Minute * 30 @@ -321,27 +321,27 @@ func handleFilteringStatus(w http.ResponseWriter, r *http.Request) { } func handleFilteringAddURL(w http.ResponseWriter, r *http.Request) { - filter := filter{} - err := json.NewDecoder(r.Body).Decode(&filter) + f := filter{} + err := json.NewDecoder(r.Body).Decode(&f) if err != nil { httpError(w, http.StatusBadRequest, "Failed to parse request body json: %s", err) return } - if len(filter.URL) == 0 { + if len(f.URL) == 0 { http.Error(w, "URL parameter was not specified", 400) return } - if valid := govalidator.IsRequestURL(filter.URL); !valid { + if valid := govalidator.IsRequestURL(f.URL); !valid { http.Error(w, "URL parameter is not valid request URL", 400) return } // Check for duplicates for i := range config.Filters { - if config.Filters[i].URL == filter.URL { - errorText := fmt.Sprintf("Filter URL already added -- %s", filter.URL) + if config.Filters[i].URL == f.URL { + errorText := fmt.Sprintf("Filter URL already added -- %s", f.URL) log.Println(errorText) http.Error(w, errorText, http.StatusBadRequest) return @@ -349,34 +349,34 @@ func handleFilteringAddURL(w http.ResponseWriter, r *http.Request) { } // Set necessary properties - filter.ID = assignUniqueFilterID() - filter.Enabled = true + f.ID = assignUniqueFilterID() + f.Enabled = true // Download the filter contents - ok, err := filter.update(true) + ok, err := f.update(true) if err != nil { - errorText := fmt.Sprintf("Couldn't fetch filter from url %s: %s", filter.URL, err) + errorText := fmt.Sprintf("Couldn't fetch filter from url %s: %s", f.URL, err) log.Println(errorText) http.Error(w, errorText, http.StatusBadRequest) return } - if filter.RulesCount == 0 { - errorText := fmt.Sprintf("Filter at the url %s has no rules (maybe it points to blank page?)", filter.URL) + if f.RulesCount == 0 { + errorText := fmt.Sprintf("Filter at the url %s has no rules (maybe it points to blank page?)", f.URL) log.Println(errorText) http.Error(w, errorText, http.StatusBadRequest) return } if !ok { - errorText := fmt.Sprintf("Filter at the url %s is invalid (maybe it points to blank page?)", filter.URL) + errorText := fmt.Sprintf("Filter at the url %s is invalid (maybe it points to blank page?)", f.URL) log.Println(errorText) http.Error(w, errorText, http.StatusBadRequest) return } // Save the filter contents - err = filter.save() + err = f.save() if err != nil { - errorText := fmt.Sprintf("Failed to save filter %d due to %s", filter.ID, err) + errorText := fmt.Sprintf("Failed to save filter %d due to %s", f.ID, err) log.Println(errorText) http.Error(w, errorText, http.StatusBadRequest) return @@ -384,7 +384,7 @@ func handleFilteringAddURL(w http.ResponseWriter, r *http.Request) { // URL is deemed valid, append it to filters, update config, write new filter file and tell dns to reload it // TODO: since we directly feed filters in-memory, revisit if writing configs is always necessary - config.Filters = append(config.Filters, filter) + config.Filters = append(config.Filters, f) err = writeAllConfigs() if err != nil { errorText := fmt.Sprintf("Couldn't write config file: %s", err) @@ -393,9 +393,14 @@ func handleFilteringAddURL(w http.ResponseWriter, r *http.Request) { return } - reconfigureDNSServer() + err = reconfigureDNSServer() + if err != nil { + errorText := fmt.Sprintf("Couldn't reconfigure the DNS server: %s", err) + log.Println(errorText) + http.Error(w, errorText, http.StatusInternalServerError) + } - _, err = fmt.Fprintf(w, "OK %d rules\n", filter.RulesCount) + _, err = fmt.Fprintf(w, "OK %d rules\n", f.RulesCount) if err != nil { errorText := fmt.Sprintf("Couldn't write body: %s", err) log.Println(errorText) diff --git a/dhcp.go b/dhcp.go index 0e8503be..90d6a88a 100644 --- a/dhcp.go +++ b/dhcp.go @@ -101,9 +101,9 @@ func handleDHCPInterfaces(w http.ResponseWriter, r *http.Request) { MTU: ifaces[i].MTU, HardwareAddr: ifaces[i].HardwareAddr.String(), } - addrs, err := ifaces[i].Addrs() - if err != nil { - httpError(w, http.StatusInternalServerError, "Failed to get addresses for interface %v: %s", ifaces[i].Name, err) + addrs, errAddrs := ifaces[i].Addrs() + if errAddrs != nil { + httpError(w, http.StatusInternalServerError, "Failed to get addresses for interface %v: %s", ifaces[i].Name, errAddrs) return } for _, addr := range addrs { @@ -155,7 +155,7 @@ func handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Request) { } func startDHCPServer() error { - if config.DHCP.Enabled == false { + if !config.DHCP.Enabled { // not enabled, don't do anything return nil } diff --git a/dhcpd/check_other_dhcp.go b/dhcpd/check_other_dhcp.go index 76be82f9..b2d64b9f 100644 --- a/dhcpd/check_other_dhcp.go +++ b/dhcpd/check_other_dhcp.go @@ -59,7 +59,7 @@ func CheckIfOtherDHCPServersPresent(ifaceName string) (bool, error) { maxUDPsizeRaw := make([]byte, 2) binary.BigEndian.PutUint16(maxUDPsizeRaw, 1500) leaseTimeRaw := make([]byte, 4) - leaseTime := uint32(math.RoundToEven(time.Duration(time.Hour * 24 * 90).Seconds())) + leaseTime := uint32(math.RoundToEven((time.Hour * 24 * 90).Seconds())) binary.BigEndian.PutUint32(leaseTimeRaw, leaseTime) options := []dhcp4.Option{ {Code: dhcp4.OptionParameterRequestList, Value: requestList}, diff --git a/dhcpd/dhcpd.go b/dhcpd/dhcpd.go index df80fac3..0fcb8168 100644 --- a/dhcpd/dhcpd.go +++ b/dhcpd/dhcpd.go @@ -83,6 +83,7 @@ func (s *Server) Start(config *ServerConfig) error { s.leaseStart, err = parseIPv4(s.RangeStart) if err != nil { + s.closeConn() // in case it was already started return wrapErrPrint(err, "Failed to parse range start address %s", s.RangeStart) } @@ -178,7 +179,7 @@ func (s *Server) reserveLease(p dhcp4.Packet) (*Lease, error) { } // not assigned a lease, create new one, find IP from LRU log.Tracef("Lease not found for %s: creating new one", hwaddr) - ip, err := s.findFreeIP(p, hwaddr) + ip, err := s.findFreeIP(hwaddr) if err != nil { return nil, wrapErrPrint(err, "Couldn't find free IP for the lease %s", hwaddr.String()) } @@ -202,7 +203,7 @@ func (s *Server) locateLease(p dhcp4.Packet) *Lease { return nil } -func (s *Server) findFreeIP(p dhcp4.Packet, hwaddr net.HardwareAddr) (net.IP, error) { +func (s *Server) findFreeIP(hwaddr net.HardwareAddr) (net.IP, error) { // if IP pool is nil, lazy initialize it if s.IPpool == nil { s.IPpool = make(map[[4]byte]net.HardwareAddr) @@ -227,7 +228,7 @@ func (s *Server) findFreeIP(p dhcp4.Packet, hwaddr net.HardwareAddr) (net.IP, er if foundIP == nil { // TODO: LRU - return nil, fmt.Errorf("Couldn't find free entry in IP pool") + return nil, fmt.Errorf("couldn't find free entry in IP pool") } s.reserveIP(foundIP, hwaddr) @@ -281,7 +282,7 @@ func (s *Server) ServeDHCP(p dhcp4.Packet, msgType dhcp4.MessageType, options dh case dhcp4.Request: // Broadcast From Client - I'll take that IP (Also start for renewals) // start/renew a lease -- update lease time // some clients (OSX) just go right ahead and do Request first from previously known IP, if they get NAK, they restart full cycle with Discover then Request - return s.handleDHCP4Request(p, msgType, options) + return s.handleDHCP4Request(p, options) case dhcp4.Decline: // Broadcast From Client - Sorry I can't use that IP log.Tracef("Got from client: Decline") @@ -306,7 +307,7 @@ func (s *Server) ServeDHCP(p dhcp4.Packet, msgType dhcp4.MessageType, options dh return nil } -func (s *Server) handleDHCP4Request(p dhcp4.Packet, msgType dhcp4.MessageType, options dhcp4.Options) dhcp4.Packet { +func (s *Server) handleDHCP4Request(p dhcp4.Packet, options dhcp4.Options) dhcp4.Packet { log.Tracef("Got from client: Request") if server, ok := options[dhcp4.OptionServerIdentifier]; ok && !net.IP(server).Equal(s.ipnet.IP) { log.Tracef("Request message not for this DHCP server (%v vs %v)", server, s.ipnet.IP) @@ -315,7 +316,7 @@ func (s *Server) handleDHCP4Request(p dhcp4.Packet, msgType dhcp4.MessageType, o reqIP := net.IP(options[dhcp4.OptionRequestedIPAddress]) if reqIP == nil { - reqIP = net.IP(p.CIAddr()) + reqIP = p.CIAddr() } if reqIP.To4() == nil { diff --git a/dhcpd/standalone/main.go b/dhcpd/standalone/main.go index fff02664..9154e063 100644 --- a/dhcpd/standalone/main.go +++ b/dhcpd/standalone/main.go @@ -76,9 +76,9 @@ func main() { panic(err) } log.Printf("Now serving DHCP") - signal_channel := make(chan os.Signal) - signal.Notify(signal_channel, syscall.SIGINT, syscall.SIGTERM) - <-signal_channel + signalChannel := make(chan os.Signal) + signal.Notify(signalChannel, syscall.SIGINT, syscall.SIGTERM) + <-signalChannel } diff --git a/dns.go b/dns.go index e4325e9c..12c71def 100644 --- a/dns.go +++ b/dns.go @@ -52,7 +52,7 @@ func generateServerConfig() dnsforward.ServerConfig { func startDNSServer() error { if isRunning() { - return fmt.Errorf("Unable to start forwarding DNS server: Already running") + return fmt.Errorf("unable to start forwarding DNS server: Already running") } newconfig := generateServerConfig() diff --git a/dnsfilter/dnsfilter_test.go b/dnsfilter/dnsfilter_test.go index 857cdf08..9692a60d 100644 --- a/dnsfilter/dnsfilter_test.go +++ b/dnsfilter/dnsfilter_test.go @@ -869,7 +869,7 @@ func BenchmarkLotsOfRulesLotsOfHosts(b *testing.B) { for n := 0; n < b.N; n++ { havedata := scanner.Scan() if !havedata { - hostnames.Seek(0, 0) + _, _ = hostnames.Seek(0, 0) scanner = bufio.NewScanner(hostnames) havedata = scanner.Scan() } @@ -906,7 +906,7 @@ func BenchmarkLotsOfRulesLotsOfHostsParallel(b *testing.B) { for pb.Next() { havedata := scanner.Scan() if !havedata { - hostnames.Seek(0, 0) + _, _ = hostnames.Seek(0, 0) scanner = bufio.NewScanner(hostnames) havedata = scanner.Scan() } diff --git a/dnsforward/querylog.go b/dnsforward/querylog.go index 903edeb6..fc51d165 100644 --- a/dnsforward/querylog.go +++ b/dnsforward/querylog.go @@ -108,7 +108,12 @@ func logRequest(question *dns.Msg, answer *dns.Msg, result *dnsfilter.Result, el if len(flushBuffer) > 0 { // write to file // do it in separate goroutine -- we are stalling DNS response this whole time - go flushToFile(flushBuffer) + go func() { + err := flushToFile(flushBuffer) + if err != nil { + log.Printf("Failed to flush the query log: %s", err) + } + }() } } diff --git a/dnsforward/stats.go b/dnsforward/stats.go index 76db1d99..cbc25af7 100644 --- a/dnsforward/stats.go +++ b/dnsforward/stats.go @@ -340,11 +340,11 @@ func HandleStatsHistory(w http.ResponseWriter, r *http.Request) { // check if start and time times are within supported time range timeRange := timeUnit * statsHistoryElements if startTime.Add(timeRange).Before(now) { - http.Error(w, "start_time parameter is outside of supported range", 501) + http.Error(w, "start_time parameter is outside of supported range", http.StatusBadRequest) return } if endTime.Add(timeRange).Before(now) { - http.Error(w, "end_time parameter is outside of supported range", 501) + http.Error(w, "end_time parameter is outside of supported range", http.StatusBadRequest) return } diff --git a/helpers.go b/helpers.go index 3ba42544..28412a58 100644 --- a/helpers.go +++ b/helpers.go @@ -39,7 +39,7 @@ func safeWriteFile(path string, data []byte) error { func ensure(method string, handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { if r.Method != method { - http.Error(w, "This request must be "+method, 405) + http.Error(w, "This request must be "+method, http.StatusMethodNotAllowed) return } handler(w, r) diff --git a/upgrade.go b/upgrade.go index 452df4d0..4b142de6 100644 --- a/upgrade.go +++ b/upgrade.go @@ -7,7 +7,7 @@ import ( "path/filepath" "github.com/hmage/golibs/log" - "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v2" ) const currentSchemaVersion = 2 // used for upgrading from old configs to new config