From 54bdacdde282a4b9b7dde55df7ed4aeda3df375d Mon Sep 17 00:00:00 2001 From: Andrey Meshkov Date: Tue, 30 Oct 2018 17:16:20 +0300 Subject: [PATCH] Fix review comments: NextFilterId collisions --- app.go | 6 ++++-- config.go | 16 ++++++++++------ control.go | 16 ++++++++-------- coredns_plugin/coredns_plugin.go | 6 +++--- dnsfilter/dnsfilter.go | 8 ++++---- 5 files changed, 29 insertions(+), 23 deletions(-) diff --git a/app.go b/app.go index 6d24bb9b..647ffa63 100644 --- a/app.go +++ b/app.go @@ -303,8 +303,10 @@ func upgradeConfigSchema(oldVersion int, newVersion int) error { filter := &config.Filters[i] // otherwise we will be operating on a copy - log.Printf("Seting ID=%d for filter %s", i, filter.URL) - filter.ID = i + 1 // start with ID=1 + // Set the filter ID + log.Printf("Seting ID=%d for filter %s", NextFilterId, filter.URL) + filter.ID = NextFilterId + NextFilterId++ // Forcibly update the filter _, err := filter.update(true) diff --git a/config.go b/config.go index e0d1d39f..bf63c0ee 100644 --- a/config.go +++ b/config.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "gopkg.in/yaml.v2" "io/ioutil" "log" "os" @@ -11,6 +10,8 @@ import ( "sync" "text/template" "time" + + "gopkg.in/yaml.v2" ) // Current schema version. We compare it with the value from @@ -20,8 +21,11 @@ const SchemaVersion = 1 // Directory where we'll store all downloaded filters contents const FiltersDir = "filters" +// User filter ID is always 0 +const UserFilterId = 0 + // Just a counter that we use for incrementing the filter ID -var NextFilterId int +var NextFilterId = time.Now().Unix() // configuration is loaded from YAML type configuration struct { @@ -46,7 +50,7 @@ type configuration struct { } type coreDnsFilter struct { - ID int `yaml:"-"` + ID int64 `yaml:"-"` Path string `yaml:"-"` } @@ -70,7 +74,7 @@ type coreDNSConfig struct { } type filter struct { - ID int `json:"id" yaml:"id"` // auto-assigned when filter is added (see NextFilterId) + ID int64 `json:"id" yaml:"id"` // auto-assigned when filter is added (see NextFilterId) URL string `json:"url"` Name string `json:"name" yaml:"name"` Enabled bool `json:"enabled"` @@ -119,8 +123,8 @@ func getUserFilter() filter { } userFilter := filter{ - // User filter always has ID=0 - ID: 0, + // User filter always has constant ID=0 + ID: UserFilterId, contents: contents, Enabled: true, } diff --git a/control.go b/control.go index 79bc811b..1e1084e8 100644 --- a/control.go +++ b/control.go @@ -15,7 +15,7 @@ import ( "strings" "time" - coreDnsPlugin "github.com/AdguardTeam/AdGuardHome/coredns_plugin" + corednsplugin "github.com/AdguardTeam/AdGuardHome/coredns_plugin" "github.com/miekg/dns" "gopkg.in/asaskevich/govalidator.v4" ) @@ -39,7 +39,7 @@ var client = &http.Client{ // coredns run control // ------------------- func tellCoreDNSToReload() { - coreDnsPlugin.Reload <- true + corednsplugin.Reload <- true } func writeAllConfigsAndReloadCoreDNS() error { @@ -790,7 +790,7 @@ func (filter *filter) load() error { // Path to the filter contents func (filter *filter) getFilterFilePath() string { - return filepath.Join(config.ourBinaryDir, config.ourDataDir, FiltersDir, strconv.Itoa(filter.ID)+".txt") + return filepath.Join(config.ourBinaryDir, config.ourDataDir, FiltersDir, strconv.FormatInt(filter.ID, 10)+".txt") } // ------------ @@ -950,15 +950,15 @@ func registerControlHandlers() { http.HandleFunc("/control/status", optionalAuth(ensureGET(handleStatus))) http.HandleFunc("/control/enable_protection", optionalAuth(ensurePOST(handleProtectionEnable))) http.HandleFunc("/control/disable_protection", optionalAuth(ensurePOST(handleProtectionDisable))) - http.HandleFunc("/control/querylog", optionalAuth(ensureGET(coreDnsPlugin.HandleQueryLog))) + http.HandleFunc("/control/querylog", optionalAuth(ensureGET(corednsplugin.HandleQueryLog))) http.HandleFunc("/control/querylog_enable", optionalAuth(ensurePOST(handleQueryLogEnable))) http.HandleFunc("/control/querylog_disable", optionalAuth(ensurePOST(handleQueryLogDisable))) http.HandleFunc("/control/set_upstream_dns", optionalAuth(ensurePOST(handleSetUpstreamDNS))) http.HandleFunc("/control/test_upstream_dns", optionalAuth(ensurePOST(handleTestUpstreamDNS))) - http.HandleFunc("/control/stats_top", optionalAuth(ensureGET(coreDnsPlugin.HandleStatsTop))) - http.HandleFunc("/control/stats", optionalAuth(ensureGET(coreDnsPlugin.HandleStats))) - http.HandleFunc("/control/stats_history", optionalAuth(ensureGET(coreDnsPlugin.HandleStatsHistory))) - http.HandleFunc("/control/stats_reset", optionalAuth(ensurePOST(coreDnsPlugin.HandleStatsReset))) + http.HandleFunc("/control/stats_top", optionalAuth(ensureGET(corednsplugin.HandleStatsTop))) + http.HandleFunc("/control/stats", optionalAuth(ensureGET(corednsplugin.HandleStats))) + http.HandleFunc("/control/stats_history", optionalAuth(ensureGET(corednsplugin.HandleStatsHistory))) + http.HandleFunc("/control/stats_reset", optionalAuth(ensurePOST(corednsplugin.HandleStatsReset))) http.HandleFunc("/control/version.json", optionalAuth(handleGetVersionJSON)) http.HandleFunc("/control/filtering/enable", optionalAuth(ensurePOST(handleFilteringEnable))) http.HandleFunc("/control/filtering/disable", optionalAuth(ensurePOST(handleFilteringDisable))) diff --git a/coredns_plugin/coredns_plugin.go b/coredns_plugin/coredns_plugin.go index ff04908d..5209e37b 100644 --- a/coredns_plugin/coredns_plugin.go +++ b/coredns_plugin/coredns_plugin.go @@ -52,7 +52,7 @@ var ( ) type plugFilter struct { - ID uint32 + ID int64 Path string } @@ -146,7 +146,7 @@ func setupPlugin(c *caddy.Controller) (*plug, error) { return nil, c.ArgErr() } - filterId, err := strconv.Atoi(c.Val()) + filterId, err := strconv.ParseInt(c.Val(), 10, 64) if err != nil { return nil, c.ArgErr() } @@ -157,7 +157,7 @@ func setupPlugin(c *caddy.Controller) (*plug, error) { // Initialize filter and add it to the list p.settings.Filters = append(p.settings.Filters, plugFilter{ - ID: uint32(filterId), + ID: filterId, Path: filterPath, }) } diff --git a/dnsfilter/dnsfilter.go b/dnsfilter/dnsfilter.go index 8b1c4e05..224bcf8b 100644 --- a/dnsfilter/dnsfilter.go +++ b/dnsfilter/dnsfilter.go @@ -71,7 +71,7 @@ type rule struct { isImportant bool // user-supplied data - listID uint32 + listID int64 // suffix matching isSuffix bool @@ -146,7 +146,7 @@ type Result struct { Reason Reason `json:",omitempty"` // Reason for blocking / unblocking Rule string `json:",omitempty"` // Original rule text Ip net.IP `json:",omitempty"` // Not nil only in the case of a hosts file syntax - FilterID uint32 `json:",omitempty"` // Filter ID the rule belongs to + FilterID int64 `json:",omitempty"` // Filter ID the rule belongs to } // Matched can be used to see if any match at all was found, no matter filtered or not @@ -734,7 +734,7 @@ func (d *Dnsfilter) lookupCommon(host string, lookupstats *LookupStats, cache gc // // AddRule adds a rule, checking if it is a valid rule first and if it wasn't added already -func (d *Dnsfilter) AddRule(input string, filterListID uint32) error { +func (d *Dnsfilter) AddRule(input string, filterListID int64) error { input = strings.TrimSpace(input) d.storageMutex.RLock() _, exists := d.storage[input] @@ -797,7 +797,7 @@ func (d *Dnsfilter) AddRule(input string, filterListID uint32) error { } // Parses the hosts-syntax rules. Returns false if the input string is not of hosts-syntax. -func (d *Dnsfilter) parseEtcHosts(input string, filterListID uint32) bool { +func (d *Dnsfilter) parseEtcHosts(input string, filterListID int64) bool { // Strip the trailing comment ruleText := input if pos := strings.IndexByte(ruleText, '#'); pos != -1 {