From 9e2da4fe7095490aeb89f6ac4f17f48349974a68 Mon Sep 17 00:00:00 2001 From: zjncs <18910855655@163.com> Date: Fri, 4 Sep 2026 11:03:11 +0800 Subject: [PATCH 1/3] fix: size cache key buffer per rule in cached enforcers --- enforcer_cached.go | 2 +- enforcer_cached_synced.go | 2 +- enforcer_cached_synced_test.go | 34 ++++++++++++++++++++++++++++++++++ enforcer_cached_test.go | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/enforcer_cached.go b/enforcer_cached.go index fc8883ba5..a9b4be404 100644 --- a/enforcer_cached.go +++ b/enforcer_cached.go @@ -114,8 +114,8 @@ func (e *CachedEnforcer) RemovePolicy(params ...interface{}) (bool, error) { func (e *CachedEnforcer) RemovePolicies(rules [][]string) (bool, error) { if len(rules) != 0 { if atomic.LoadInt32(&e.enableCache) != 0 { - irule := make([]interface{}, len(rules[0])) for _, rule := range rules { + irule := make([]interface{}, len(rule)) for i, param := range rule { irule[i] = param } diff --git a/enforcer_cached_synced.go b/enforcer_cached_synced.go index cc46afedd..3ae261349 100644 --- a/enforcer_cached_synced.go +++ b/enforcer_cached_synced.go @@ -167,8 +167,8 @@ func (e *SyncedCachedEnforcer) checkOneAndRemoveCache(params ...interface{}) (bo func (e *SyncedCachedEnforcer) checkManyAndRemoveCache(rules [][]string) (bool, error) { if len(rules) != 0 { if atomic.LoadInt32(&e.enableCache) != 0 { - irule := make([]interface{}, len(rules[0])) for _, rule := range rules { + irule := make([]interface{}, len(rule)) for i, param := range rule { irule[i] = param } diff --git a/enforcer_cached_synced_test.go b/enforcer_cached_synced_test.go index 1c7da267d..04ac1fdca 100644 --- a/enforcer_cached_synced_test.go +++ b/enforcer_cached_synced_test.go @@ -83,3 +83,37 @@ func TestSyncCache(t *testing.T) { testSyncEnforceCache(t, e, "alice", "data2", "read", true) testSyncEnforceCache(t, e, "alice", "data2", "write", true) } + +// TestSyncRemovePoliciesCacheRaggedRulesStaleEntry verifies that RemovePolicies +// invalidates the cached decision of every rule in the batch even when the +// rules have different lengths. The key buffer used to be sized from the first +// rule and reused, so a later shorter rule produced a key with a stale tail and +// the cached decision of a removed rule survived the removal. +func TestSyncRemovePoliciesCacheRaggedRulesStaleEntry(t *testing.T) { + e, _ := NewSyncedCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") + + _, _ = e.AddPolicies([][]string{{"bob", "data2", "write", "extra"}}) + testSyncEnforceCache(t, e, "alice", "data1", "read", true) + + _, _ = e.RemovePolicies([][]string{ + {"bob", "data2", "write", "extra"}, + {"alice", "data1", "read"}, + }) + + testSyncEnforceCache(t, e, "alice", "data1", "read", false) +} + +// TestSyncRemovePoliciesCacheRaggedRulesLongerRule verifies that a rule longer +// than the first rule of the batch does not panic. The key buffer used to be +// sized from the first rule and reused, so writing a longer rule panicked with +// index out of range. +func TestSyncRemovePoliciesCacheRaggedRulesLongerRule(t *testing.T) { + e, _ := NewSyncedCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") + + testSyncEnforceCache(t, e, "alice", "data1", "read", true) + + _, _ = e.RemovePolicies([][]string{ + {"alice", "data1", "read"}, + {"bob", "data2", "write", "extra"}, + }) +} diff --git a/enforcer_cached_test.go b/enforcer_cached_test.go index b404b6510..a11725dc8 100644 --- a/enforcer_cached_test.go +++ b/enforcer_cached_test.go @@ -74,3 +74,37 @@ func TestCache(t *testing.T) { testEnforceCache(t, e, "alice", "data2", "read", false) testEnforceCache(t, e, "alice", "data2", "write", false) } + +// TestRemovePoliciesCacheRaggedRulesStaleEntry verifies that RemovePolicies +// invalidates the cached decision of every rule in the batch even when the +// rules have different lengths. The key buffer used to be sized from the first +// rule and reused, so a later shorter rule produced a key with a stale tail and +// the cached decision of a removed rule survived the removal. +func TestRemovePoliciesCacheRaggedRulesStaleEntry(t *testing.T) { + e, _ := NewCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") + + _, _ = e.AddPolicies([][]string{{"bob", "data2", "write", "extra"}}) + testEnforceCache(t, e, "alice", "data1", "read", true) + + _, _ = e.RemovePolicies([][]string{ + {"bob", "data2", "write", "extra"}, + {"alice", "data1", "read"}, + }) + + testEnforceCache(t, e, "alice", "data1", "read", false) +} + +// TestRemovePoliciesCacheRaggedRulesLongerRule verifies that a rule longer +// than the first rule of the batch does not panic. The key buffer used to be +// sized from the first rule and reused, so writing a longer rule panicked with +// index out of range. +func TestRemovePoliciesCacheRaggedRulesLongerRule(t *testing.T) { + e, _ := NewCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") + + testEnforceCache(t, e, "alice", "data1", "read", true) + + _, _ = e.RemovePolicies([][]string{ + {"alice", "data1", "read"}, + {"bob", "data2", "write", "extra"}, + }) +} From 9a152294913512b8b40ce06ce87aa5ea9849e76f Mon Sep 17 00:00:00 2001 From: Yang Luo Date: Wed, 9 Sep 2026 22:00:09 +0800 Subject: [PATCH 2/3] Update enforcer_cached_synced_test.go --- enforcer_cached_synced_test.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/enforcer_cached_synced_test.go b/enforcer_cached_synced_test.go index 04ac1fdca..1c7da267d 100644 --- a/enforcer_cached_synced_test.go +++ b/enforcer_cached_synced_test.go @@ -83,37 +83,3 @@ func TestSyncCache(t *testing.T) { testSyncEnforceCache(t, e, "alice", "data2", "read", true) testSyncEnforceCache(t, e, "alice", "data2", "write", true) } - -// TestSyncRemovePoliciesCacheRaggedRulesStaleEntry verifies that RemovePolicies -// invalidates the cached decision of every rule in the batch even when the -// rules have different lengths. The key buffer used to be sized from the first -// rule and reused, so a later shorter rule produced a key with a stale tail and -// the cached decision of a removed rule survived the removal. -func TestSyncRemovePoliciesCacheRaggedRulesStaleEntry(t *testing.T) { - e, _ := NewSyncedCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") - - _, _ = e.AddPolicies([][]string{{"bob", "data2", "write", "extra"}}) - testSyncEnforceCache(t, e, "alice", "data1", "read", true) - - _, _ = e.RemovePolicies([][]string{ - {"bob", "data2", "write", "extra"}, - {"alice", "data1", "read"}, - }) - - testSyncEnforceCache(t, e, "alice", "data1", "read", false) -} - -// TestSyncRemovePoliciesCacheRaggedRulesLongerRule verifies that a rule longer -// than the first rule of the batch does not panic. The key buffer used to be -// sized from the first rule and reused, so writing a longer rule panicked with -// index out of range. -func TestSyncRemovePoliciesCacheRaggedRulesLongerRule(t *testing.T) { - e, _ := NewSyncedCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") - - testSyncEnforceCache(t, e, "alice", "data1", "read", true) - - _, _ = e.RemovePolicies([][]string{ - {"alice", "data1", "read"}, - {"bob", "data2", "write", "extra"}, - }) -} From 69b34290b614e3cf07d7c7a4b6830cb36abfe288 Mon Sep 17 00:00:00 2001 From: Yang Luo Date: Wed, 9 Sep 2026 22:00:38 +0800 Subject: [PATCH 3/3] Update enforcer_cached_test.go --- enforcer_cached_test.go | 34 ---------------------------------- 1 file changed, 34 deletions(-) diff --git a/enforcer_cached_test.go b/enforcer_cached_test.go index a11725dc8..b404b6510 100644 --- a/enforcer_cached_test.go +++ b/enforcer_cached_test.go @@ -74,37 +74,3 @@ func TestCache(t *testing.T) { testEnforceCache(t, e, "alice", "data2", "read", false) testEnforceCache(t, e, "alice", "data2", "write", false) } - -// TestRemovePoliciesCacheRaggedRulesStaleEntry verifies that RemovePolicies -// invalidates the cached decision of every rule in the batch even when the -// rules have different lengths. The key buffer used to be sized from the first -// rule and reused, so a later shorter rule produced a key with a stale tail and -// the cached decision of a removed rule survived the removal. -func TestRemovePoliciesCacheRaggedRulesStaleEntry(t *testing.T) { - e, _ := NewCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") - - _, _ = e.AddPolicies([][]string{{"bob", "data2", "write", "extra"}}) - testEnforceCache(t, e, "alice", "data1", "read", true) - - _, _ = e.RemovePolicies([][]string{ - {"bob", "data2", "write", "extra"}, - {"alice", "data1", "read"}, - }) - - testEnforceCache(t, e, "alice", "data1", "read", false) -} - -// TestRemovePoliciesCacheRaggedRulesLongerRule verifies that a rule longer -// than the first rule of the batch does not panic. The key buffer used to be -// sized from the first rule and reused, so writing a longer rule panicked with -// index out of range. -func TestRemovePoliciesCacheRaggedRulesLongerRule(t *testing.T) { - e, _ := NewCachedEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv") - - testEnforceCache(t, e, "alice", "data1", "read", true) - - _, _ = e.RemovePolicies([][]string{ - {"alice", "data1", "read"}, - {"bob", "data2", "write", "extra"}, - }) -}