From a2d2420e9cd454e698691ec806fc89258818101b Mon Sep 17 00:00:00 2001 From: Eric Chlebek Date: Wed, 14 Oct 2020 10:22:13 -0700 Subject: [PATCH] Backport js vm fixes from 6.x to 5.21 (#4053) Signed-off-by: Eric Chlebek --- CHANGELOG.md | 1 + js/js.go | 51 +++++++++++++++++++++++++++++++++++++++++++------- js/vm_cache.go | 50 +++++++++++++++++++++++++++++-------------------- 3 files changed, 75 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e7e278cb6..7f8e6d659a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Versioning](http://semver.org/spec/v2.0.0.html). ### Fixed - Close the response body when done reading from it while downloading assets. +- Fixed a bug where event filter or asset filter execution could cause a crash. ## [5.21.2] - 2020-08-31 diff --git a/js/js.go b/js/js.go index c0e5df7846..477731ee34 100644 --- a/js/js.go +++ b/js/js.go @@ -47,14 +47,10 @@ func ParseExpressions(expressions []string) error { return nil } -func newOttoVM(assets JavascriptAssets) (*otto.Otto, error) { +func newOttoVM(assets JavascriptAssets, key string) (*otto.Otto, error) { ottoOnce.Do(func() { ottoCache = newVMCache() }) - key := "" - if assets != nil { - key = assets.Key() - } vm := ottoCache.Acquire(key) if vm != nil { return vm, nil @@ -72,6 +68,13 @@ func newOttoVM(assets JavascriptAssets) (*otto.Otto, error) { return ottoCache.Acquire(key), nil } +func releaseOttoVM(key string) { + ottoOnce.Do(func() { + panic("releaseOttoVM called before newOttoVM") + }) + ottoCache.Dispose(key) +} + func addAssets(vm *otto.Otto, assets JavascriptAssets) error { scripts, err := assets.Scripts() if err != nil { @@ -122,10 +125,15 @@ func addTimeFuncs(vm *otto.Otto) error { // If scripts is non-nil, then the scripts will be evaluated in the // expression's runtime context before the expression is evaluated. func Evaluate(expr string, parameters interface{}, assets JavascriptAssets) (bool, error) { - jsvm, err := newOttoVM(assets) + key := "" + if assets != nil { + key = assets.Key() + } + jsvm, err := newOttoVM(assets, key) if err != nil { return false, err } + defer releaseOttoVM(key) if params, ok := parameters.(map[string]interface{}); ok { for name, value := range params { if err := jsvm.Set(name, value); err != nil { @@ -140,6 +148,34 @@ func Evaluate(expr string, parameters interface{}, assets JavascriptAssets) (boo return value.ToBoolean() } +// EvalPredicateWithVM is like Evaluate, but allows passing a vm explicitly. +func EvalPredicateWithVM(vm *otto.Otto, parameters map[string]interface{}, expr string) (bool, error) { + for name, value := range parameters { + if err := vm.Set(name, value); err != nil { + return false, err + } + } + value, err := vm.Run(expr) + if err != nil { + return false, err + } + return value.ToBoolean() +} + +// WithOttoVM provides a context manager for working with cached VMs. +func WithOttoVM(assets JavascriptAssets, fn func(vm *otto.Otto) error) error { + key := "" + if assets != nil { + key = assets.Key() + } + jsvm, err := newOttoVM(assets, key) + if err != nil { + return err + } + defer releaseOttoVM(key) + return fn(jsvm) +} + // EntityFilterResult is returned by EvaluateEntityFilters type EntityFilterResult struct { Value bool @@ -159,10 +195,11 @@ type EntityFilterResult struct { // If the function cannot set up a javascript VM, or has issues setting vars, // then the function returns a nil slice and a non-nil error. func MatchEntities(expressions []string, entities []interface{}) ([]bool, error) { - jsvm, err := newOttoVM(nil) + jsvm, err := newOttoVM(nil, "") if err != nil { return nil, fmt.Errorf("error evaluating entity filters: %s", err) } + defer releaseOttoVM("") scripts := make([]*otto.Script, 0, len(expressions)) for _, expr := range expressions { script, err := jsvm.Compile("", expr) diff --git a/js/vm_cache.go b/js/vm_cache.go index 993c272e98..45c1beaaec 100644 --- a/js/vm_cache.go +++ b/js/vm_cache.go @@ -1,6 +1,7 @@ package js import ( + "fmt" "sync" time "github.com/echlebek/timeproxy" @@ -17,22 +18,21 @@ const ( ) // vmCache provides an internal mechanism for caching javascript contexts -// according to which assets are loaded into them. Javascrip contexts which +// according to which assets are loaded into them. Javascript contexts which // are not used for cacheMaxAge are disposed of. type vmCache struct { - vms map[string]*cacheValue + vms sync.Map done chan struct{} - sync.Mutex } type cacheValue struct { lastRead int64 + mu sync.Mutex vm *otto.Otto } func newVMCache() *vmCache { cache := &vmCache{ - vms: make(map[string]*cacheValue), done: make(chan struct{}), } go cache.reapLoop() @@ -58,34 +58,44 @@ func (c *vmCache) reapLoop() { } func (c *vmCache) reap() { - c.Lock() - for k, v := range c.vms { - valueTime := time.Unix(v.lastRead, 0) - if valueTime.Before(time.Now().Add(-cacheMaxAge)) { - delete(c.vms, k) + c.vms.Range(func(key, value interface{}) bool { + obj := value.(*cacheValue) + obj.mu.Lock() + defer obj.mu.Unlock() + valueTime := time.Unix(obj.lastRead, 0) + if time.Since(valueTime) > cacheMaxAge { + c.vms.Delete(key) } - } - defer c.Unlock() + return true + }) } // Acquire gets a VM from the cache. It is a copy of the cached value. +// The cache item is locked while in use. +// Users must call Dispose with the key after Acquire. func (c *vmCache) Acquire(key string) *otto.Otto { - c.Lock() - defer c.Unlock() - val, ok := c.vms[key] + val, ok := c.vms.Load(key) if !ok { return nil } - if val.vm == nil { - return nil + obj := val.(*cacheValue) + obj.mu.Lock() + obj.lastRead = time.Now().Unix() + return obj.vm.Copy() +} + +// Dispose releases the lock on the cache item. +func (c *vmCache) Dispose(key string) { + val, ok := c.vms.Load(key) + if !ok { + panic(fmt.Sprintf("dispose called on %q, but not found", key)) } - return val.vm.Copy() + obj := val.(*cacheValue) + obj.mu.Unlock() } // Init initializes the value in the cache. func (c *vmCache) Init(key string, vm *otto.Otto) { - c.Lock() - defer c.Unlock() val := &cacheValue{lastRead: time.Now().Unix(), vm: vm} - c.vms[key] = val + c.vms.Store(key, val) }