From dab4b372318612b543c60c437c660e027c888784 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Wed, 15 May 2024 17:31:54 -0500 Subject: [PATCH 01/11] regex path var check --- rules/lock/lock.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rules/lock/lock.go b/rules/lock/lock.go index c6b4ed4..43361f1 100644 --- a/rules/lock/lock.go +++ b/rules/lock/lock.go @@ -2,6 +2,8 @@ package lock import ( "errors" + "fmt" + "regexp" "time" "golang.org/x/net/context" @@ -54,6 +56,10 @@ type v3Locker struct { } func (v3l *v3Locker) Lock(key string, options ...Option) (RuleLock, error) { + validPath := regexp.MustCompile(`^[[:alnum:] \"\'\_\.\,\*\=\-]+$`) + if !validPath.MatchString(key) { + return nil, fmt.Errorf("Path variable contains an invalid character") + } return v3l.lockWithTimeout(key, v3l.lockTimeout) } func (v3l *v3Locker) lockWithTimeout(key string, timeout int) (RuleLock, error) { From baa135edd7e10a19a37ff569ef8ffe0257315dc1 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Thu, 16 May 2024 15:29:26 -0500 Subject: [PATCH 02/11] add testing --- rules/lock/lock.go | 2 +- rules/lock/lock_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/rules/lock/lock.go b/rules/lock/lock.go index 43361f1..36bb547 100644 --- a/rules/lock/lock.go +++ b/rules/lock/lock.go @@ -56,7 +56,7 @@ type v3Locker struct { } func (v3l *v3Locker) Lock(key string, options ...Option) (RuleLock, error) { - validPath := regexp.MustCompile(`^[[:alnum:] \"\'\_\.\,\*\=\-]+$`) + validPath := regexp.MustCompile(`^[[:alnum:] \/\"\'\_\.\,\*\=\-]+$`) if !validPath.MatchString(key) { return nil, fmt.Errorf("Path variable contains an invalid character") } diff --git a/rules/lock/lock_test.go b/rules/lock/lock_test.go index 1e5962e..3a41922 100644 --- a/rules/lock/lock_test.go +++ b/rules/lock/lock_test.go @@ -1,6 +1,7 @@ package lock import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -58,3 +59,39 @@ func Test_V3Locker(t *testing.T) { }) } } + +func Test_V3LockerRegex(t *testing.T) { + cfg, cl := teststore.InitV3Etcd(t) + _, err := v3.New(cfg) + require.NoError(t, err) + newSession := func(_ context.Context) (*v3c.Session, error) { + return v3c.NewSession(cl, v3c.WithTTL(30)) + } + + testcases := []struct { + name string + lockKey string + err error + }{ + { + name: "bad regex", + lockKey: "/test?/", + err: fmt.Errorf("Path variable contains an invalid character"), + }, + { + name: "good regex", + lockKey: "/test/", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + rlckr := v3Locker{ + newSession: newSession, + lockTimeout: 5, + } + _, err := rlckr.Lock(tc.lockKey) + assert.Equal(t, err, tc.err) + }) + } +} From 8337f7a58b3860a410380eee620ea0078578b87c Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Thu, 16 May 2024 16:57:52 -0500 Subject: [PATCH 03/11] refactor --- rules/engine.go | 10 ++++++++-- rules/engine_test.go | 13 +++++++++---- rules/lock/lock.go | 6 ------ rules/lock/lock_test.go | 37 ------------------------------------- 4 files changed, 17 insertions(+), 49 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index 2f0e2f4..3f3dd69 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -3,6 +3,7 @@ package rules import ( "fmt" "os" + "regexp" "strings" "time" @@ -69,7 +70,7 @@ type V3Engine interface { AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, - options ...RuleOption) + options ...RuleOption) error AddPolling(namespacePattern string, preconditions DynamicRule, ttl int, @@ -169,8 +170,13 @@ func (e *v3Engine) SetWatcherWrapper(watcherWrapper WrapWatcher) { func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, - options ...RuleOption) { + options ...RuleOption) error { + validPath := regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) + if !validPath.MatchString(lockPattern) { + return fmt.Errorf("Path contains an invalid character") + } e.addRuleWithIface(rule, lockPattern, callback, options...) + return nil } func (e *baseEngine) Stop() { diff --git a/rules/engine_test.go b/rules/engine_test.go index 8335bc2..8863856 100644 --- a/rules/engine_test.go +++ b/rules/engine_test.go @@ -1,6 +1,7 @@ package rules import ( + "fmt" "testing" "time" @@ -30,14 +31,18 @@ func TestV3EngineConstructor(t *testing.T) { eng := NewV3Engine(cfg, getTestLogger()) value := "val" rule, _ := NewEqualsLiteralRule("/key", &value) - eng.AddRule(rule, "/lock", v3DummyCallback, RuleID("test")) - assert.PanicsWithValue(t, "Rule ID option missing", func() { eng.AddRule(rule, "/lock", v3DummyCallback) }) - err := eng.AddPolling("/polling", rule, 30, v3DummyCallback) + err := eng.AddRule(rule, "/lock?@", v3DummyCallback, RuleID("test")) + assert.Equal(t, err, fmt.Errorf("Path contains an invalid character")) + err = eng.AddRule(rule, "/lock", v3DummyCallback, RuleID("test")) + assert.NoError(t, err) + assert.PanicsWithValue(t, "Rule ID option missing", func() { assert.NoError(t, eng.AddRule(rule, "/lock", v3DummyCallback)) }) + err = eng.AddPolling("/polling", rule, 30, v3DummyCallback) assert.NoError(t, err) assertEngineRunStop(t, eng) eng = NewV3Engine(cfg, getTestLogger(), KeyExpansion(map[string][]string{"a:": {"b"}})) - eng.AddRule(rule, "/lock", v3DummyCallback, RuleLockTimeout(30), RuleID("test")) + err = eng.AddRule(rule, "/lock", v3DummyCallback, RuleLockTimeout(30), RuleID("test")) + assert.NoError(t, err) err = eng.AddPolling("/polling", rule, 30, v3DummyCallback) assert.NoError(t, err) err = eng.AddPolling("/polling[", rule, 30, v3DummyCallback) diff --git a/rules/lock/lock.go b/rules/lock/lock.go index 36bb547..c6b4ed4 100644 --- a/rules/lock/lock.go +++ b/rules/lock/lock.go @@ -2,8 +2,6 @@ package lock import ( "errors" - "fmt" - "regexp" "time" "golang.org/x/net/context" @@ -56,10 +54,6 @@ type v3Locker struct { } func (v3l *v3Locker) Lock(key string, options ...Option) (RuleLock, error) { - validPath := regexp.MustCompile(`^[[:alnum:] \/\"\'\_\.\,\*\=\-]+$`) - if !validPath.MatchString(key) { - return nil, fmt.Errorf("Path variable contains an invalid character") - } return v3l.lockWithTimeout(key, v3l.lockTimeout) } func (v3l *v3Locker) lockWithTimeout(key string, timeout int) (RuleLock, error) { diff --git a/rules/lock/lock_test.go b/rules/lock/lock_test.go index 3a41922..1e5962e 100644 --- a/rules/lock/lock_test.go +++ b/rules/lock/lock_test.go @@ -1,7 +1,6 @@ package lock import ( - "fmt" "testing" "github.com/stretchr/testify/assert" @@ -59,39 +58,3 @@ func Test_V3Locker(t *testing.T) { }) } } - -func Test_V3LockerRegex(t *testing.T) { - cfg, cl := teststore.InitV3Etcd(t) - _, err := v3.New(cfg) - require.NoError(t, err) - newSession := func(_ context.Context) (*v3c.Session, error) { - return v3c.NewSession(cl, v3c.WithTTL(30)) - } - - testcases := []struct { - name string - lockKey string - err error - }{ - { - name: "bad regex", - lockKey: "/test?/", - err: fmt.Errorf("Path variable contains an invalid character"), - }, - { - name: "good regex", - lockKey: "/test/", - }, - } - - for _, tc := range testcases { - t.Run(tc.name, func(t *testing.T) { - rlckr := v3Locker{ - newSession: newSession, - lockTimeout: 5, - } - _, err := rlckr.Lock(tc.lockKey) - assert.Equal(t, err, tc.err) - }) - } -} From d6228713dbb3d1d58fe76372ab80ba76cc7a4a58 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Thu, 16 May 2024 17:08:52 -0500 Subject: [PATCH 04/11] err check --- rules/engine.go | 5 ++++- v3enginetest/main.go | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index 3f3dd69..f5c2bbe 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -268,7 +268,10 @@ func (e *v3Engine) AddPolling(namespacePattern string, preconditions DynamicRule lease: e.cl, engine: e, } - e.AddRule(rule, "/rule_locks"+namespacePattern+"lock", cbw.doRule, RuleID(namespacePattern)) + err = e.AddRule(rule, "/rule_locks"+namespacePattern+"lock", cbw.doRule, RuleID(namespacePattern)) + if err != nil { + return err + } return nil } diff --git a/v3enginetest/main.go b/v3enginetest/main.go index 2781800..285f66e 100644 --- a/v3enginetest/main.go +++ b/v3enginetest/main.go @@ -170,12 +170,13 @@ func main() { doneFalse := "false" doneRule, err := rules.NewEqualsLiteralRule(donePath, &doneFalse) check(err) - engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { + err = engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { path := task.Attr.Format(donePath) doneTrue := "true" _, err := kv.Put(task.Context, path, doneTrue) check(err) }, rules.RuleID(doneRuleID)) + check(err) engine.Run() time.Sleep(time.Second) From 49d49b59284b6e354907f75fe93b74df2e07d7a0 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Fri, 17 May 2024 13:35:17 -0500 Subject: [PATCH 05/11] refactor --- rules/engine.go | 19 +++++++++---------- rules/engine_test.go | 13 ++++--------- v3enginetest/main.go | 3 +-- 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index f5c2bbe..27488f7 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -70,7 +70,7 @@ type V3Engine interface { AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, - options ...RuleOption) error + options ...RuleOption) AddPolling(namespacePattern string, preconditions DynamicRule, ttl int, @@ -167,16 +167,18 @@ func (e *v3Engine) SetWatcherWrapper(watcherWrapper WrapWatcher) { e.watcherWrapper = watcherWrapper } +// valid paths patterns must be alphanumeric and may only contain a few special characters (:/"'_.,*=-) +var validPath = regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) + func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, - options ...RuleOption) error { - validPath := regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) + options ...RuleOption) { if !validPath.MatchString(lockPattern) { - return fmt.Errorf("Path contains an invalid character") + e.logger.Fatal("Path contains an invalid character") + } else { + e.addRuleWithIface(rule, lockPattern, callback, options...) } - e.addRuleWithIface(rule, lockPattern, callback, options...) - return nil } func (e *baseEngine) Stop() { @@ -268,10 +270,7 @@ func (e *v3Engine) AddPolling(namespacePattern string, preconditions DynamicRule lease: e.cl, engine: e, } - err = e.AddRule(rule, "/rule_locks"+namespacePattern+"lock", cbw.doRule, RuleID(namespacePattern)) - if err != nil { - return err - } + e.AddRule(rule, "/rule_locks"+namespacePattern+"lock", cbw.doRule, RuleID(namespacePattern)) return nil } diff --git a/rules/engine_test.go b/rules/engine_test.go index 8863856..8335bc2 100644 --- a/rules/engine_test.go +++ b/rules/engine_test.go @@ -1,7 +1,6 @@ package rules import ( - "fmt" "testing" "time" @@ -31,18 +30,14 @@ func TestV3EngineConstructor(t *testing.T) { eng := NewV3Engine(cfg, getTestLogger()) value := "val" rule, _ := NewEqualsLiteralRule("/key", &value) - err := eng.AddRule(rule, "/lock?@", v3DummyCallback, RuleID("test")) - assert.Equal(t, err, fmt.Errorf("Path contains an invalid character")) - err = eng.AddRule(rule, "/lock", v3DummyCallback, RuleID("test")) - assert.NoError(t, err) - assert.PanicsWithValue(t, "Rule ID option missing", func() { assert.NoError(t, eng.AddRule(rule, "/lock", v3DummyCallback)) }) - err = eng.AddPolling("/polling", rule, 30, v3DummyCallback) + eng.AddRule(rule, "/lock", v3DummyCallback, RuleID("test")) + assert.PanicsWithValue(t, "Rule ID option missing", func() { eng.AddRule(rule, "/lock", v3DummyCallback) }) + err := eng.AddPolling("/polling", rule, 30, v3DummyCallback) assert.NoError(t, err) assertEngineRunStop(t, eng) eng = NewV3Engine(cfg, getTestLogger(), KeyExpansion(map[string][]string{"a:": {"b"}})) - err = eng.AddRule(rule, "/lock", v3DummyCallback, RuleLockTimeout(30), RuleID("test")) - assert.NoError(t, err) + eng.AddRule(rule, "/lock", v3DummyCallback, RuleLockTimeout(30), RuleID("test")) err = eng.AddPolling("/polling", rule, 30, v3DummyCallback) assert.NoError(t, err) err = eng.AddPolling("/polling[", rule, 30, v3DummyCallback) diff --git a/v3enginetest/main.go b/v3enginetest/main.go index 285f66e..2781800 100644 --- a/v3enginetest/main.go +++ b/v3enginetest/main.go @@ -170,13 +170,12 @@ func main() { doneFalse := "false" doneRule, err := rules.NewEqualsLiteralRule(donePath, &doneFalse) check(err) - err = engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { + engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { path := task.Attr.Format(donePath) doneTrue := "true" _, err := kv.Put(task.Context, path, doneTrue) check(err) }, rules.RuleID(doneRuleID)) - check(err) engine.Run() time.Sleep(time.Second) From 32ede26971ca2d0cea902177633fc1a649517e89 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Fri, 17 May 2024 13:50:12 -0500 Subject: [PATCH 06/11] wording --- rules/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/engine.go b/rules/engine.go index 27488f7..c364065 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -167,7 +167,7 @@ func (e *v3Engine) SetWatcherWrapper(watcherWrapper WrapWatcher) { e.watcherWrapper = watcherWrapper } -// valid paths patterns must be alphanumeric and may only contain a few special characters (:/"'_.,*=-) +// valid paths patterns must be alphanumeric and may only contain select special characters (:/"'_.,*=-) var validPath = regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) func (e *v3Engine) AddRule(rule DynamicRule, From 5b115f61d8183a7be8282e439ec65387ffcb4ea6 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Fri, 17 May 2024 14:02:19 -0500 Subject: [PATCH 07/11] wording --- rules/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/engine.go b/rules/engine.go index c364065..c3eec2b 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -167,7 +167,7 @@ func (e *v3Engine) SetWatcherWrapper(watcherWrapper WrapWatcher) { e.watcherWrapper = watcherWrapper } -// valid paths patterns must be alphanumeric and may only contain select special characters (:/"'_.,*=-) +// valid path patterns must be alphanumeric and may only contain select special characters (:/"'_.,*=-) var validPath = regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) func (e *v3Engine) AddRule(rule DynamicRule, From 7e04d040217d4fdd7b5806360ad613796035adc3 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Fri, 17 May 2024 14:20:22 -0500 Subject: [PATCH 08/11] lock check --- rules/engine.go | 4 ++-- v3enginetest/main.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index c3eec2b..7829ad9 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -174,8 +174,8 @@ func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, options ...RuleOption) { - if !validPath.MatchString(lockPattern) { - e.logger.Fatal("Path contains an invalid character") + if !validPath.MatchString(lockPattern) || !strings.Contains(lockPattern, "lock") { + e.logger.Fatal("Path contains an invalid character or does not contain \"lock\"") } else { e.addRuleWithIface(rule, lockPattern, callback, options...) } diff --git a/v3enginetest/main.go b/v3enginetest/main.go index 2781800..b76da65 100644 --- a/v3enginetest/main.go +++ b/v3enginetest/main.go @@ -170,7 +170,7 @@ func main() { doneFalse := "false" doneRule, err := rules.NewEqualsLiteralRule(donePath, &doneFalse) check(err) - engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { + engine.AddRule(doneRule, "/rulesEngineDone/:id/lock", func(task *rules.V3RuleTask) { path := task.Attr.Format(donePath) doneTrue := "true" _, err := kv.Put(task.Context, path, doneTrue) From 10d99096834eb0a7f098a2459c35cbd78d5dfb00 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Mon, 20 May 2024 09:29:43 -0500 Subject: [PATCH 09/11] rm lock --- rules/engine.go | 4 ++-- v3enginetest/main.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index 7829ad9..c3eec2b 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -174,8 +174,8 @@ func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, options ...RuleOption) { - if !validPath.MatchString(lockPattern) || !strings.Contains(lockPattern, "lock") { - e.logger.Fatal("Path contains an invalid character or does not contain \"lock\"") + if !validPath.MatchString(lockPattern) { + e.logger.Fatal("Path contains an invalid character") } else { e.addRuleWithIface(rule, lockPattern, callback, options...) } diff --git a/v3enginetest/main.go b/v3enginetest/main.go index b76da65..2781800 100644 --- a/v3enginetest/main.go +++ b/v3enginetest/main.go @@ -170,7 +170,7 @@ func main() { doneFalse := "false" doneRule, err := rules.NewEqualsLiteralRule(donePath, &doneFalse) check(err) - engine.AddRule(doneRule, "/rulesEngineDone/:id/lock", func(task *rules.V3RuleTask) { + engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { path := task.Attr.Format(donePath) doneTrue := "true" _, err := kv.Put(task.Context, path, doneTrue) From c5c4fc2abc599f8549f7a604e87f6d54ffdb86b8 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Mon, 20 May 2024 09:55:40 -0500 Subject: [PATCH 10/11] add lock --- rules/engine.go | 7 +++---- v3enginetest/main.go | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/rules/engine.go b/rules/engine.go index c3eec2b..2bbed0f 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -174,11 +174,10 @@ func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string, callback V3RuleTaskCallback, options ...RuleOption) { - if !validPath.MatchString(lockPattern) { - e.logger.Fatal("Path contains an invalid character") - } else { - e.addRuleWithIface(rule, lockPattern, callback, options...) + if !validPath.MatchString(lockPattern) || !strings.Contains(lockPattern, "lock") { + e.logger.Fatal("Path contains an invalid character or does not contain \"lock\"") } + e.addRuleWithIface(rule, lockPattern, callback, options...) } func (e *baseEngine) Stop() { diff --git a/v3enginetest/main.go b/v3enginetest/main.go index 2781800..b76da65 100644 --- a/v3enginetest/main.go +++ b/v3enginetest/main.go @@ -170,7 +170,7 @@ func main() { doneFalse := "false" doneRule, err := rules.NewEqualsLiteralRule(donePath, &doneFalse) check(err) - engine.AddRule(doneRule, "/rulesEngineDone/:id", func(task *rules.V3RuleTask) { + engine.AddRule(doneRule, "/rulesEngineDone/:id/lock", func(task *rules.V3RuleTask) { path := task.Attr.Format(donePath) doneTrue := "true" _, err := kv.Put(task.Context, path, doneTrue) From 12725af831dc5a28161cf79d7f2bb6ba8e2bb515 Mon Sep 17 00:00:00 2001 From: Olivia Golden Date: Tue, 21 May 2024 13:45:22 -0500 Subject: [PATCH 11/11] # --- rules/engine.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/engine.go b/rules/engine.go index 2bbed0f..a0e6c25 100644 --- a/rules/engine.go +++ b/rules/engine.go @@ -168,7 +168,7 @@ func (e *v3Engine) SetWatcherWrapper(watcherWrapper WrapWatcher) { } // valid path patterns must be alphanumeric and may only contain select special characters (:/"'_.,*=-) -var validPath = regexp.MustCompile(`^[[:alnum:] \:\/\"\'\_\.\,\*\=\-]*$`) +var validPath = regexp.MustCompile(`^[[:alnum:] \#\:\/\"\'\_\.\,\*\=\-]*$`) func (e *v3Engine) AddRule(rule DynamicRule, lockPattern string,