Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Log blocking rule in resolver #1489

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5dcb29c
log blocking rule in resolver
zc-devs May 21, 2024
ad89678
linter
zc-devs May 21, 2024
638528b
pre-alloc
zc-devs May 23, 2024
12e8877
allocating
zc-devs May 23, 2024
a0833fc
nil
zc-devs May 23, 2024
71822d2
reworked trie
zc-devs May 25, 2024
abde77d
Merge branch 'main' into 1458-improve-logging-2
zc-devs May 25, 2024
9e59fab
linter
zc-devs May 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 5 additions & 15 deletions cache/stringcache/chained_grouped_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
package stringcache

import (
"sort"

"golang.org/x/exp/maps"
)

type ChainedGroupedCache struct {
caches []GroupedStringCache
}
Expand All @@ -25,20 +19,16 @@ func (c *ChainedGroupedCache) ElementCount(group string) int {
return sum
}

func (c *ChainedGroupedCache) Contains(searchString string, groups []string) []string {
groupMatchedMap := make(map[string]struct{}, len(groups))
func (c *ChainedGroupedCache) Contains(searchString string, groups []string) map[string]string {
result := make(map[string]string, len(groups))

for _, cache := range c.caches {
for _, group := range cache.Contains(searchString, groups) {
groupMatchedMap[group] = struct{}{}
for group, rule := range cache.Contains(searchString, groups) {
result[group] = rule
}
}

matchedGroups := maps.Keys(groupMatchedMap)

sort.Strings(matchedGroups)

return matchedGroups
return result
}

func (c *ChainedGroupedCache) Refresh(group string) GroupFactory {
Expand Down
19 changes: 10 additions & 9 deletions cache/stringcache/chained_grouped_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/0xERR0R/blocky/cache/stringcache"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"golang.org/x/exp/maps"
)

var _ = Describe("Chained grouped cache", func() {
Expand Down Expand Up @@ -58,8 +59,8 @@ var _ = Describe("Chained grouped cache", func() {

It("should find strings", func() {
factory.Finish()
Expect(cache.Contains("string1", []string{"group1"})).Should(ConsistOf("group1"))
Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string1", []string{"group1"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string2", []string{"group1", "someOtherGroup"}))).Should(ConsistOf("group1"))
})
})
})
Expand All @@ -86,9 +87,9 @@ var _ = Describe("Chained grouped cache", func() {
It("should contain 4 elements in 2 groups", func() {
Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2))
Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2))
Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(ConsistOf("group1"))
Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group1", "group2"))
Expect(maps.Keys(cache.Contains("g1", []string{"group1", "group2"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("g2", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("both", []string{"group1", "group2"}))).Should(ConsistOf("group1", "group2"))
})

It("should replace group content on refresh", func() {
Expand All @@ -98,10 +99,10 @@ var _ = Describe("Chained grouped cache", func() {

Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1))
Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2))
Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(BeEmpty())
Expect(cache.Contains("newString", []string{"group1", "group2"})).Should(ConsistOf("group1"))
Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("g1", []string{"group1", "group2"}))).Should(BeEmpty())
Expect(maps.Keys(cache.Contains("newString", []string{"group1", "group2"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("g2", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("both", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
})

It("should replace empty groups on refresh", func() {
Expand Down
4 changes: 2 additions & 2 deletions cache/stringcache/grouped_cache_interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package stringcache

type GroupedStringCache interface {
// Contains checks if one or more groups in the cache contains the search string.
// Returns group(s) containing the string or empty slice if string was not found
Contains(searchString string, groups []string) []string
// Returns group(s) containing the string as keys, and the rule that caused the block as value, empty map if string was not found
Contains(searchString string, groups []string) map[string]string

// Refresh creates new factory for the group to be refreshed.
// Calling Finish on the factory will perform the group refresh.
Expand Down
11 changes: 7 additions & 4 deletions cache/stringcache/in_memory_grouped_cache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,19 @@ func (c *InMemoryGroupedCache) ElementCount(group string) int {
return cache.elementCount()
}

func (c *InMemoryGroupedCache) Contains(searchString string, groups []string) []string {
var result []string
func (c *InMemoryGroupedCache) Contains(searchString string, groups []string) map[string]string {
result := make(map[string]string)
zc-devs marked this conversation as resolved.
Show resolved Hide resolved

for _, group := range groups {
c.lock.RLock()
cache, found := c.caches[group]
c.lock.RUnlock()

if found && cache.contains(searchString) {
result = append(result, group)
if found {
match, rule := cache.contains(searchString)
if match {
result[group] = rule
}
}
}

Expand Down
33 changes: 17 additions & 16 deletions cache/stringcache/in_memory_grouped_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/0xERR0R/blocky/cache/stringcache"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"golang.org/x/exp/maps"
)

var _ = Describe("In-Memory grouped cache", func() {
Expand Down Expand Up @@ -65,8 +66,8 @@ var _ = Describe("In-Memory grouped cache", func() {

It("should find strings", func() {
factory.Finish()
Expect(cache.Contains("string1", []string{"group1"})).Should(ConsistOf("group1"))
Expect(cache.Contains("string2", []string{"group1", "someOtherGroup"})).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string1", []string{"group1"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string2", []string{"group1", "someOtherGroup"}))).Should(ConsistOf("group1"))
})
})
When("Regex grouped cache is used", func() {
Expand All @@ -81,9 +82,9 @@ var _ = Describe("In-Memory grouped cache", func() {

It("should ignore non-regex", func() {
Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1))
Expect(cache.Contains("string1", []string{"group1"})).Should(BeEmpty())
Expect(cache.Contains("string2", []string{"group1"})).Should(ConsistOf("group1"))
Expect(cache.Contains("shouldalsomatchstring2", []string{"group1"})).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string1", []string{"group1"}))).Should(BeEmpty())
Expect(maps.Keys(cache.Contains("string2", []string{"group1"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("shouldalsomatchstring2", []string{"group1"}))).Should(ConsistOf("group1"))
})
})
When("Wildcard grouped cache is used", func() {
Expand All @@ -99,10 +100,10 @@ var _ = Describe("In-Memory grouped cache", func() {

It("should ignore non-wildcard", func() {
Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1))
Expect(cache.Contains("string1", []string{"group1"})).Should(BeEmpty())
Expect(cache.Contains("string2", []string{"group1"})).Should(BeEmpty())
Expect(cache.Contains("string3", []string{"group1"})).Should(ConsistOf("group1"))
Expect(cache.Contains("shouldalsomatch.string3", []string{"group1"})).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("string1", []string{"group1"}))).Should(BeEmpty())
Expect(maps.Keys(cache.Contains("string2", []string{"group1"}))).Should(BeEmpty())
Expect(maps.Keys(cache.Contains("string3", []string{"group1"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("shouldalsomatch.string3", []string{"group1"}))).Should(ConsistOf("group1"))
})
})
})
Expand All @@ -126,9 +127,9 @@ var _ = Describe("In-Memory grouped cache", func() {
It("should contain 4 elements in 2 groups", func() {
Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 2))
Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2))
Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(ConsistOf("group1"))
Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group1", "group2"))
Expect(maps.Keys(cache.Contains("g1", []string{"group1", "group2"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("g2", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("both", []string{"group1", "group2"}))).Should(ConsistOf("group1", "group2"))
})

It("Should replace group content on refresh", func() {
Expand All @@ -138,10 +139,10 @@ var _ = Describe("In-Memory grouped cache", func() {

Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1))
Expect(cache.ElementCount("group2")).Should(BeNumerically("==", 2))
Expect(cache.Contains("g1", []string{"group1", "group2"})).Should(BeEmpty())
Expect(cache.Contains("newString", []string{"group1", "group2"})).Should(ConsistOf("group1"))
Expect(cache.Contains("g2", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(cache.Contains("both", []string{"group1", "group2"})).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("g1", []string{"group1", "group2"}))).Should(BeEmpty())
Expect(maps.Keys(cache.Contains("newString", []string{"group1", "group2"}))).Should(ConsistOf("group1"))
Expect(maps.Keys(cache.Contains("g2", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
Expect(maps.Keys(cache.Contains("both", []string{"group1", "group2"}))).Should(ConsistOf("group2"))
})
})
})
Expand Down
18 changes: 9 additions & 9 deletions cache/stringcache/string_caches.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

type stringCache interface {
elementCount() int
contains(searchString string) bool
contains(searchString string) (bool, string)
Copy link
Collaborator

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:

Suggested change
contains(searchString string) (bool, string)
contains(searchString string) (string, bool)

I'll put suggestions for the other uses, hopefully I don't miss anything!

Copy link
Contributor Author

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:

contains(searchString string) (bool, err)

err should be at the end as it is not the main response from method.

Could we rename contains to match and get rid of boolean?

// returns rule if matched, otherwise empty string
func match(searchString string) (string)

}

type cacheFactory interface {
Expand All @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (cache stringMap) contains(searchString string) (bool, string) {
func (cache stringMap) contains(searchString string) (string, bool) {

normalized := normalizeEntry(searchString)
searchLen := len(normalized)

if searchLen == 0 {
return false
return false, ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return false, ""
return "", false

}

searchBucketLen := len(cache[searchLen]) / searchLen
Expand All @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return true, blockRule
return blockRule, true

}
}

return false
return false, ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return false, ""
return "", false

}

type stringCacheFactory struct {
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (cache regexCache) contains(searchString string) (bool, string) {
func (cache regexCache) contains(searchString string) (string, bool) {

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()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return true, regex.String()
return regex.String(), true

}
}

return false
return false, ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return false, ""
return "", false

}

type regexCacheFactory struct {
Expand Down Expand Up @@ -197,7 +197,7 @@ func (cache wildcardCache) elementCount() int {
return cache.cnt
}

func (cache wildcardCache) contains(domain string) bool {
func (cache wildcardCache) contains(domain string) (bool, string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (cache wildcardCache) contains(domain string) (bool, string) {
func (cache wildcardCache) contains(domain string) (string, bool) {

return cache.trie.HasParentOf(domain)
}

Expand Down
2 changes: 1 addition & 1 deletion cache/stringcache/string_caches_benchmark_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if contains, _ := cache.contains(s); !contains {
if _, contains := cache.contains(s); !contains {

b.Fatalf("cache is missing value from stringTestData: %s", s)
}
}
Expand Down
Loading
Loading