-
-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log blocking rule in resolver #1489
base: main
Are you sure you want to change the base?
Changes from all commits
5dcb29c
ad89678
638528b
12e8877
a0833fc
71822d2
abde77d
9e59fab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -11,7 +11,7 @@ import ( | |||||
|
||||||
type stringCache interface { | ||||||
elementCount() int | ||||||
contains(searchString string) bool | ||||||
contains(searchString string) (bool, string) | ||||||
} | ||||||
|
||||||
type cacheFactory interface { | ||||||
|
@@ -36,12 +36,12 @@ func (cache stringMap) elementCount() int { | |||||
return count | ||||||
} | ||||||
|
||||||
func (cache stringMap) contains(searchString string) bool { | ||||||
func (cache stringMap) contains(searchString string) (bool, string) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
normalized := normalizeEntry(searchString) | ||||||
searchLen := len(normalized) | ||||||
|
||||||
if searchLen == 0 { | ||||||
return false | ||||||
return false, "" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
searchBucketLen := len(cache[searchLen]) / searchLen | ||||||
|
@@ -54,11 +54,11 @@ func (cache stringMap) contains(searchString string) bool { | |||||
if blockRule == normalized { | ||||||
log.PrefixedLog("string_map").Debugf("block rule '%s' matched with '%s'", blockRule, searchString) | ||||||
|
||||||
return true | ||||||
return true, blockRule | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} | ||||||
|
||||||
return false | ||||||
return false, "" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
type stringCacheFactory struct { | ||||||
|
@@ -134,16 +134,16 @@ func (cache regexCache) elementCount() int { | |||||
return len(cache) | ||||||
} | ||||||
|
||||||
func (cache regexCache) contains(searchString string) bool { | ||||||
func (cache regexCache) contains(searchString string) (bool, string) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
for _, regex := range cache { | ||||||
if regex.MatchString(searchString) { | ||||||
log.PrefixedLog("regex_cache").Debugf("regex '%s' matched with '%s'", regex, searchString) | ||||||
|
||||||
return true | ||||||
return true, regex.String() | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} | ||||||
|
||||||
return false | ||||||
return false, "" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
|
||||||
type regexCacheFactory struct { | ||||||
|
@@ -197,8 +197,12 @@ func (cache wildcardCache) elementCount() int { | |||||
return cache.cnt | ||||||
} | ||||||
|
||||||
func (cache wildcardCache) contains(domain string) bool { | ||||||
return cache.trie.HasParentOf(domain) | ||||||
func (cache wildcardCache) contains(domain string) (bool, string) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
contains, path := cache.trie.HasParentOf(domain) | ||||||
rule := strings.Join(path, ".") | ||||||
rule = strings.Trim(rule, ".") | ||||||
|
||||||
return contains, rule | ||||||
} | ||||||
|
||||||
type wildcardCacheFactory struct { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -180,7 +180,7 @@ func benchmarkCache(b *testing.B, data []string, newFactory func() cacheFactory) | |||||
// - wildcards and regexes need a plain string query | ||||||
// - all benchmarks will do the same number of queries | ||||||
for _, s := range stringTestData { | ||||||
if !cache.contains(s) { | ||||||
if contains, _ := cache.contains(s); !contains { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
b.Fatalf("cache is missing value from stringTestData: %s", s) | ||||||
} | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be nice to swap the return values so it matches
map
:I'll put suggestions for the other uses, hopefully I don't miss anything!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not do this with
contains
method. The name implies that the main return type should be boolean. Consider error return value:err
should be at the end as it is not the main response from method.Could we rename
contains
tomatch
and get rid of boolean?