-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
chore: adds memoize implementation for regexes and ahocorasick #836
Changes from 2 commits
18a78cb
23382ac
4dae331
be2fc24
b8df0c2
73be18c
d43a91f
62043be
3ed990b
8d52562
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 |
---|---|---|
|
@@ -15,6 +15,7 @@ import ( | |
"github.com/corazawaf/coraza/v3/experimental/plugins/macro" | ||
"github.com/corazawaf/coraza/v3/experimental/plugins/plugintypes" | ||
"github.com/corazawaf/coraza/v3/internal/corazarules" | ||
"github.com/corazawaf/coraza/v3/internal/memoize" | ||
"github.com/corazawaf/coraza/v3/types" | ||
"github.com/corazawaf/coraza/v3/types/variables" | ||
) | ||
|
@@ -456,7 +457,12 @@ func (r *Rule) AddVariable(v variables.RuleVariable, key string, iscount bool) e | |
var re *regexp.Regexp | ||
if len(key) > 2 && key[0] == '/' && key[len(key)-1] == '/' { | ||
key = key[1 : len(key)-1] | ||
re = regexp.MustCompile(key) | ||
|
||
if vare, err := memoize.Do(key, func() (interface{}, error) { return regexp.Compile(key) }); err != nil { | ||
panic(err) | ||
} else { | ||
re = vare.(*regexp.Regexp) | ||
} | ||
} | ||
|
||
if multiphaseEvaluation { | ||
|
@@ -521,7 +527,11 @@ func (r *Rule) AddVariableNegation(v variables.RuleVariable, key string) error { | |
var re *regexp.Regexp | ||
if len(key) > 2 && key[0] == '/' && key[len(key)-1] == '/' { | ||
key = key[1 : len(key)-1] | ||
re = regexp.MustCompile(key) | ||
if vare, err := memoize.Do(key, func() (interface{}, error) { return regexp.Compile(key) }); err != nil { | ||
panic(err) | ||
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. We should never panic, you can return error here 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. Agree, however this should be fixed in |
||
} else { | ||
re = vare.(*regexp.Regexp) | ||
} | ||
} | ||
// Prevent sigsev | ||
if r == nil { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !tinygo | ||
|
||
// Highly inspired in https://github.com/patrickmn/go-cache/blob/master/cache.go | ||
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. This is ok but this file looks like an obvious mutex guarded map, not sure it's worth referencing anything. Anyways it looks like a |
||
|
||
package memoize | ||
|
||
import ( | ||
"sync" | ||
) | ||
|
||
// Cache is a simple in-memory key-value store. | ||
type cache struct { | ||
mu sync.RWMutex | ||
isClosed bool | ||
entries map[string]interface{} | ||
} | ||
|
||
// newCache returns a new cache. | ||
func newCache() *cache { | ||
return &cache{ | ||
entries: make(map[string]interface{}), | ||
} | ||
} | ||
|
||
// set the value for the given key. | ||
func (c *cache) set(key string, value interface{}) { | ||
c.mu.Lock() | ||
defer c.mu.Unlock() | ||
|
||
if c.isClosed { | ||
return | ||
} | ||
c.entries[key] = value | ||
} | ||
|
||
// get the value for the given key. | ||
func (c *cache) get(key string) (interface{}, bool) { | ||
c.mu.RLock() | ||
item, found := c.entries[key] | ||
if !found { | ||
c.mu.RUnlock() | ||
return nil, false | ||
} | ||
c.mu.RUnlock() | ||
return item, true | ||
} | ||
|
||
// Close the cache. | ||
func (c *cache) Close() { | ||
c.mu.Lock() | ||
c.isClosed = true | ||
c.entries = nil | ||
c.mu.Unlock() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !tinygo | ||
|
||
package memoize | ||
|
||
import "testing" | ||
|
||
func TestCache(t *testing.T) { | ||
tc := newCache() | ||
|
||
_, found := tc.get("key1") | ||
if want, have := false, found; want != have { | ||
t.Fatalf("unexpected value, want %t, have %t", want, have) | ||
} | ||
|
||
tc.set("key1", 1) | ||
|
||
item, found := tc.get("key1") | ||
if want, have := true, found; want != have { | ||
t.Fatalf("unexpected value, want %t, have %t", want, have) | ||
} | ||
|
||
if want, have := 1, item.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
tc.Close() | ||
|
||
tc.set("key1", 1) | ||
|
||
_, found = tc.get("key1") | ||
if want, have := false, found; want != have { | ||
t.Fatalf("unexpected value, want %t, have %t", want, have) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !tinygo | ||
|
||
// https://github.com/kofalt/go-memoize/blob/master/memoize.go | ||
|
||
package memoize | ||
|
||
import ( | ||
"golang.org/x/sync/singleflight" | ||
) | ||
|
||
var doer = makeDoer(newCache(), &singleflight.Group{}) | ||
|
||
// Do executes and returns the results of the given function, unless there was a cached | ||
// value of the same key. Only one execution is in-flight for a given key at a time. | ||
// The boolean return value indicates whether v was previously stored. | ||
func Do(key string, fn func() (interface{}, error)) (interface{}, error) { | ||
value, err, _ := doer(key, fn) | ||
return value, err | ||
} | ||
|
||
// makeDoer returns a function that executes and returns the results of the given function | ||
func makeDoer(cache *cache, group *singleflight.Group) func(string, func() (interface{}, error)) (interface{}, error, bool) { | ||
return func(key string, fn func() (interface{}, error)) (interface{}, error, bool) { | ||
// Check cache | ||
value, found := cache.get(key) | ||
if found { | ||
return value, nil, true | ||
} | ||
|
||
// Combine memoized function with a cache store | ||
value, err, _ := group.Do(key, func() (interface{}, error) { | ||
data, innerErr := fn() | ||
if innerErr == nil { | ||
cache.set(key, data) | ||
} | ||
|
||
return data, innerErr | ||
}) | ||
|
||
return value, err, false | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build !tinygo | ||
|
||
// https://github.com/kofalt/go-memoize/blob/master/memoize.go | ||
|
||
package memoize | ||
|
||
import ( | ||
"errors" | ||
"testing" | ||
|
||
"golang.org/x/sync/singleflight" | ||
) | ||
|
||
func TestSuccessCall(t *testing.T) { | ||
do := makeDoer(newCache(), &singleflight.Group{}) | ||
|
||
expensiveCalls := 0 | ||
|
||
// Function tracks how many times its been called | ||
expensive := func() (interface{}, error) { | ||
expensiveCalls++ | ||
return expensiveCalls, nil | ||
} | ||
|
||
// First call SHOULD NOT be cached | ||
result, err, cached := do("key1", expensive) | ||
if err != nil { | ||
t.Fatalf("unexpected error: %s", err.Error()) | ||
} | ||
|
||
if want, have := 1, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := false, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
|
||
// Second call on same key SHOULD be cached | ||
result, err, cached = do("key1", expensive) | ||
if err != nil { | ||
t.Fatalf("unexpected error: %s", err.Error()) | ||
} | ||
|
||
if want, have := 1, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := true, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
|
||
// First call on a new key SHOULD NOT be cached | ||
result, err, cached = do("key2", expensive) | ||
if err != nil { | ||
t.Fatalf("unexpected error: %s", err.Error()) | ||
} | ||
|
||
if want, have := 2, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := false, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
} | ||
|
||
func TestFailedCall(t *testing.T) { | ||
do := makeDoer(newCache(), &singleflight.Group{}) | ||
|
||
calls := 0 | ||
|
||
// This function will fail IFF it has not been called before. | ||
twoForTheMoney := func() (interface{}, error) { | ||
calls++ | ||
|
||
if calls == 1 { | ||
return calls, errors.New("Try again") | ||
} else { | ||
return calls, nil | ||
} | ||
} | ||
|
||
// First call should fail, and not be cached | ||
result, err, cached := do("key1", twoForTheMoney) | ||
if err == nil { | ||
t.Fatalf("expected error") | ||
} | ||
|
||
if want, have := 1, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := false, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
|
||
// Second call should succeed, and not be cached | ||
result, err, cached = do("key1", twoForTheMoney) | ||
if err != nil { | ||
t.Fatalf("unexpected error: %s", err.Error()) | ||
} | ||
|
||
if want, have := 2, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := false, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
|
||
// Third call should succeed, and be cached | ||
result, err, cached = do("key1", twoForTheMoney) | ||
if err != nil { | ||
t.Fatalf("unexpected error: %s", err.Error()) | ||
} | ||
|
||
if want, have := 2, result.(int); want != have { | ||
t.Fatalf("unexpected value, want %d, have %d", want, have) | ||
} | ||
|
||
if want, have := true, cached; want != have { | ||
t.Fatalf("unexpected caching, want %t, have %t", want, have) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// Copyright 2023 Juan Pablo Tosso and the OWASP Coraza contributors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
//go:build tinygo | ||
|
||
package memoize | ||
|
||
func Do(_ string, fn func() (interface{}, error)) (interface{}, error) { | ||
return fn() | ||
} |
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.
It should be worth extracting a function for the two usages
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.
You mean for the regex and binaryregex?