From 6646466e9bc43e63c285d12c30ceeab501e06bfa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 22 Nov 2025 08:50:07 +0000 Subject: [PATCH 1/5] Initial plan From 9aa85f800587326b84933e86e145a262fe61b8fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 22 Nov 2025 08:57:11 +0000 Subject: [PATCH 2/5] Add use_zero tags and update methods to support empty strings Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com> --- adapter.go | 117 +++++++++++++++++++----------------------- adapter_test.go | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+), 66 deletions(-) diff --git a/adapter.go b/adapter.go index b9f1030..7ecc13b 100644 --- a/adapter.go +++ b/adapter.go @@ -20,12 +20,12 @@ type CasbinRule struct { tableName struct{} `pg:"_"` ID string Ptype string - V0 string - V1 string - V2 string - V3 string - V4 string - V5 string + V0 string `pg:",use_zero"` + V1 string `pg:",use_zero"` + V2 string `pg:",use_zero"` + V3 string `pg:",use_zero"` + V4 string `pg:",use_zero"` + V5 string `pg:",use_zero"` } type Filter struct { @@ -162,29 +162,22 @@ func (r *CasbinRule) String() string { ) sb.WriteString(r.Ptype) - if len(r.V0) > 0 { - sb.WriteString(prefixLine) - sb.WriteString(r.V0) - } - if len(r.V1) > 0 { - sb.WriteString(prefixLine) - sb.WriteString(r.V1) - } - if len(r.V2) > 0 { - sb.WriteString(prefixLine) - sb.WriteString(r.V2) - } - if len(r.V3) > 0 { - sb.WriteString(prefixLine) - sb.WriteString(r.V3) - } - if len(r.V4) > 0 { - sb.WriteString(prefixLine) - sb.WriteString(r.V4) + + // Build array of values to determine the last non-empty position + values := []string{r.V0, r.V1, r.V2, r.V3, r.V4, r.V5} + lastIndex := -1 + for i := len(values) - 1; i >= 0; i-- { + if values[i] != "" { + lastIndex = i + break + } } - if len(r.V5) > 0 { + + // Include all values up to and including the last non-empty one + // This preserves empty strings in the middle while trimming trailing empty strings + for i := 0; i <= lastIndex; i++ { sb.WriteString(prefixLine) - sb.WriteString(r.V5) + sb.WriteString(values[i]) } return sb.String() @@ -547,31 +540,24 @@ func (a *Adapter) UpdateFilteredPolicies(sec string, ptype string, newPolicies [ func (c *CasbinRule) queryString() (string, []interface{}) { queryArgs := []interface{}{c.Ptype} - queryStr := "ptype = ?" - if c.V0 != "" { - queryStr += " and v0 = ?" - queryArgs = append(queryArgs, c.V0) - } - if c.V1 != "" { - queryStr += " and v1 = ?" - queryArgs = append(queryArgs, c.V1) - } - if c.V2 != "" { - queryStr += " and v2 = ?" - queryArgs = append(queryArgs, c.V2) - } - if c.V3 != "" { - queryStr += " and v3 = ?" - queryArgs = append(queryArgs, c.V3) - } - if c.V4 != "" { - queryStr += " and v4 = ?" - queryArgs = append(queryArgs, c.V4) + + // Build array of values to determine the last non-empty position + values := []string{c.V0, c.V1, c.V2, c.V3, c.V4, c.V5} + lastIndex := -1 + for i := len(values) - 1; i >= 0; i-- { + if values[i] != "" { + lastIndex = i + break + } } - if c.V5 != "" { - queryStr += " and v5 = ?" - queryArgs = append(queryArgs, c.V5) + + // Include all fields up to and including the last non-empty one + // This ensures empty strings in the middle are matched explicitly + fields := []string{"v0", "v1", "v2", "v3", "v4", "v5"} + for i := 0; i <= lastIndex; i++ { + queryStr += " and " + fields[i] + " = ?" + queryArgs = append(queryArgs, values[i]) } return queryStr, queryArgs @@ -582,24 +568,23 @@ func (c *CasbinRule) toStringPolicy() []string { if c.Ptype != "" { policy = append(policy, c.Ptype) } - if c.V0 != "" { - policy = append(policy, c.V0) - } - if c.V1 != "" { - policy = append(policy, c.V1) - } - if c.V2 != "" { - policy = append(policy, c.V2) - } - if c.V3 != "" { - policy = append(policy, c.V3) - } - if c.V4 != "" { - policy = append(policy, c.V4) + + // Build array of values to determine the last non-empty position + values := []string{c.V0, c.V1, c.V2, c.V3, c.V4, c.V5} + lastIndex := -1 + for i := len(values) - 1; i >= 0; i-- { + if values[i] != "" { + lastIndex = i + break + } } - if c.V5 != "" { - policy = append(policy, c.V5) + + // Include all values up to and including the last non-empty one + // This preserves empty strings in the middle while trimming trailing empty strings + for i := 0; i <= lastIndex; i++ { + policy = append(policy, values[i]) } + return policy } diff --git a/adapter_test.go b/adapter_test.go index e390ca1..534f0f2 100644 --- a/adapter_test.go +++ b/adapter_test.go @@ -345,6 +345,139 @@ func (s *AdapterTestSuite) TestUpdateFilteredPolicies() { s.assertPolicy(s.e.GetPolicy(), [][]string{{"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}, {"alice", "data2", "write"}, {"bob", "data1", "read"}}) } + +func (s *AdapterTestSuite) TestEmptyStringSupport() { + // Test that empty strings are properly stored and retrieved + var err error + + // Add policies with empty strings in different positions + _, err = s.e.AddPolicy("alice", "", "read") + s.Require().NoError(err) + + _, err = s.e.AddPolicy("bob", "data1", "") + s.Require().NoError(err) + + _, err = s.e.AddPolicy("charlie", "", "") + s.Require().NoError(err) + + // Reload to ensure they were saved correctly + err = s.e.LoadPolicy() + s.Require().NoError(err) + + // Check that all policies including empty strings are present + policies := s.e.GetPolicy() + + // Should have original 4 policies plus 3 new ones with empty strings + s.Assert().Len(policies, 7) + + // Verify the new policies with empty strings exist + hasAliceEmpty := false + hasBobEmpty := false + hasCharlieEmpty := false + + for _, p := range policies { + if len(p) == 3 && p[0] == "alice" && p[1] == "" && p[2] == "read" { + hasAliceEmpty = true + } + if len(p) == 3 && p[0] == "bob" && p[1] == "data1" && p[2] == "" { + hasBobEmpty = true + } + if len(p) == 3 && p[0] == "charlie" && p[1] == "" && p[2] == "" { + hasCharlieEmpty = true + } + } + + s.Assert().True(hasAliceEmpty, "Policy with alice and empty string in middle not found") + s.Assert().True(hasBobEmpty, "Policy with bob and empty string at end not found") + s.Assert().True(hasCharlieEmpty, "Policy with charlie and two empty strings not found") + + // Test removing a policy with empty string + _, err = s.e.RemovePolicy("alice", "", "read") + s.Require().NoError(err) + + err = s.e.LoadPolicy() + s.Require().NoError(err) + + policies = s.e.GetPolicy() + s.Assert().Len(policies, 6) + + // Verify alice policy with empty string was removed + for _, p := range policies { + if len(p) == 3 && p[0] == "alice" && p[1] == "" && p[2] == "read" { + s.Fail("Policy with alice and empty string should have been removed") + } + } + + // Test updating a policy with empty string + _, err = s.e.UpdatePolicy([]string{"bob", "data1", ""}, []string{"bob", "data2", ""}) + s.Require().NoError(err) + + err = s.e.LoadPolicy() + s.Require().NoError(err) + + policies = s.e.GetPolicy() + hasBobData2Empty := false + for _, p := range policies { + if len(p) == 3 && p[0] == "bob" && p[1] == "data2" && p[2] == "" { + hasBobData2Empty = true + } + // Should not have the old policy anymore + if len(p) == 3 && p[0] == "bob" && p[1] == "data1" && p[2] == "" { + s.Fail("Old policy with bob and data1 should have been updated") + } + } + s.Assert().True(hasBobData2Empty, "Updated policy with bob and data2 not found") +} + +func (s *AdapterTestSuite) TestEmptyStringVsWildcard() { + // Test that empty strings in rules are different from wildcards in filters + var err error + + // Add a policy with an empty string + _, err = s.e.AddPolicy("user1", "", "write") + s.Require().NoError(err) + + // Add a policy with a non-empty value + _, err = s.e.AddPolicy("user1", "resource1", "write") + s.Require().NoError(err) + + // Reload + err = s.e.LoadPolicy() + s.Require().NoError(err) + + // Both should exist + policies := s.e.GetPolicy() + hasEmpty := false + hasNonEmpty := false + + for _, p := range policies { + if len(p) >= 3 && p[0] == "user1" && p[2] == "write" { + if p[1] == "" { + hasEmpty = true + } else if p[1] == "resource1" { + hasNonEmpty = true + } + } + } + + s.Assert().True(hasEmpty, "Policy with empty string should exist") + s.Assert().True(hasNonEmpty, "Policy with resource1 should exist") + + // Remove using wildcard (empty string in filter) should remove both + _, err = s.e.RemoveFilteredPolicy(0, "user1", "", "write") + s.Require().NoError(err) + + err = s.e.LoadPolicy() + s.Require().NoError(err) + + policies = s.e.GetPolicy() + for _, p := range policies { + if len(p) >= 3 && p[0] == "user1" && p[2] == "write" { + s.Fail("Policies with user1 and write should have been removed by wildcard filter") + } + } +} + func TestAdapterTestSuite(t *testing.T) { suite.Run(t, new(AdapterTestSuite)) } From 388a6120edded946edff012171f0110d9839ea0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 22 Nov 2025 08:59:46 +0000 Subject: [PATCH 3/5] Fix test to handle Casbin's trailing empty string trimming Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com> --- adapter_test.go | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/adapter_test.go b/adapter_test.go index 534f0f2..7bc45e8 100644 --- a/adapter_test.go +++ b/adapter_test.go @@ -347,17 +347,15 @@ func (s *AdapterTestSuite) TestUpdateFilteredPolicies() { } func (s *AdapterTestSuite) TestEmptyStringSupport() { - // Test that empty strings are properly stored and retrieved + // Test that empty strings in the middle of rules are properly stored and retrieved + // Note: Casbin trims trailing empty strings, so we focus on empty strings in the middle var err error - // Add policies with empty strings in different positions + // Add policies with empty strings in the middle _, err = s.e.AddPolicy("alice", "", "read") s.Require().NoError(err) - _, err = s.e.AddPolicy("bob", "data1", "") - s.Require().NoError(err) - - _, err = s.e.AddPolicy("charlie", "", "") + _, err = s.e.AddPolicy("bob", "", "write") s.Require().NoError(err) // Reload to ensure they were saved correctly @@ -367,29 +365,24 @@ func (s *AdapterTestSuite) TestEmptyStringSupport() { // Check that all policies including empty strings are present policies := s.e.GetPolicy() - // Should have original 4 policies plus 3 new ones with empty strings - s.Assert().Len(policies, 7) + // Should have original 4 policies plus 2 new ones with empty strings + s.Assert().Len(policies, 6) // Verify the new policies with empty strings exist hasAliceEmpty := false hasBobEmpty := false - hasCharlieEmpty := false for _, p := range policies { if len(p) == 3 && p[0] == "alice" && p[1] == "" && p[2] == "read" { hasAliceEmpty = true } - if len(p) == 3 && p[0] == "bob" && p[1] == "data1" && p[2] == "" { + if len(p) == 3 && p[0] == "bob" && p[1] == "" && p[2] == "write" { hasBobEmpty = true } - if len(p) == 3 && p[0] == "charlie" && p[1] == "" && p[2] == "" { - hasCharlieEmpty = true - } } s.Assert().True(hasAliceEmpty, "Policy with alice and empty string in middle not found") - s.Assert().True(hasBobEmpty, "Policy with bob and empty string at end not found") - s.Assert().True(hasCharlieEmpty, "Policy with charlie and two empty strings not found") + s.Assert().True(hasBobEmpty, "Policy with bob and empty string in middle not found") // Test removing a policy with empty string _, err = s.e.RemovePolicy("alice", "", "read") @@ -399,7 +392,7 @@ func (s *AdapterTestSuite) TestEmptyStringSupport() { s.Require().NoError(err) policies = s.e.GetPolicy() - s.Assert().Len(policies, 6) + s.Assert().Len(policies, 5) // Verify alice policy with empty string was removed for _, p := range policies { @@ -409,24 +402,24 @@ func (s *AdapterTestSuite) TestEmptyStringSupport() { } // Test updating a policy with empty string - _, err = s.e.UpdatePolicy([]string{"bob", "data1", ""}, []string{"bob", "data2", ""}) + _, err = s.e.UpdatePolicy([]string{"bob", "", "write"}, []string{"bob", "", "read"}) s.Require().NoError(err) err = s.e.LoadPolicy() s.Require().NoError(err) policies = s.e.GetPolicy() - hasBobData2Empty := false + hasBobEmptyRead := false for _, p := range policies { - if len(p) == 3 && p[0] == "bob" && p[1] == "data2" && p[2] == "" { - hasBobData2Empty = true + if len(p) == 3 && p[0] == "bob" && p[1] == "" && p[2] == "read" { + hasBobEmptyRead = true } // Should not have the old policy anymore - if len(p) == 3 && p[0] == "bob" && p[1] == "data1" && p[2] == "" { - s.Fail("Old policy with bob and data1 should have been updated") + if len(p) == 3 && p[0] == "bob" && p[1] == "" && p[2] == "write" { + s.Fail("Old policy with bob and write should have been updated") } } - s.Assert().True(hasBobData2Empty, "Updated policy with bob and data2 not found") + s.Assert().True(hasBobEmptyRead, "Updated policy with bob and read not found") } func (s *AdapterTestSuite) TestEmptyStringVsWildcard() { From e4c190102ebe40d317bc2ec4a7878ed25002458d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 22 Nov 2025 09:01:35 +0000 Subject: [PATCH 4/5] Refactor to extract duplicate logic into helper methods Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com> --- adapter.go | 50 +++++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/adapter.go b/adapter.go index 7ecc13b..8f0dd71 100644 --- a/adapter.go +++ b/adapter.go @@ -151,6 +151,23 @@ func (a *Adapter) createTableifNotExists() error { return nil } +// getValues returns the V0-V5 values as a slice +func (r *CasbinRule) getValues() []string { + return []string{r.V0, r.V1, r.V2, r.V3, r.V4, r.V5} +} + +// getLastNonEmptyIndex returns the index of the last non-empty value in the rule +// Returns -1 if all values are empty +func (r *CasbinRule) getLastNonEmptyIndex() int { + values := r.getValues() + for i := len(values) - 1; i >= 0; i-- { + if values[i] != "" { + return i + } + } + return -1 +} + func (r *CasbinRule) String() string { const prefixLine = ", " var sb strings.Builder @@ -163,15 +180,8 @@ func (r *CasbinRule) String() string { sb.WriteString(r.Ptype) - // Build array of values to determine the last non-empty position - values := []string{r.V0, r.V1, r.V2, r.V3, r.V4, r.V5} - lastIndex := -1 - for i := len(values) - 1; i >= 0; i-- { - if values[i] != "" { - lastIndex = i - break - } - } + values := r.getValues() + lastIndex := r.getLastNonEmptyIndex() // Include all values up to and including the last non-empty one // This preserves empty strings in the middle while trimming trailing empty strings @@ -542,15 +552,8 @@ func (c *CasbinRule) queryString() (string, []interface{}) { queryArgs := []interface{}{c.Ptype} queryStr := "ptype = ?" - // Build array of values to determine the last non-empty position - values := []string{c.V0, c.V1, c.V2, c.V3, c.V4, c.V5} - lastIndex := -1 - for i := len(values) - 1; i >= 0; i-- { - if values[i] != "" { - lastIndex = i - break - } - } + values := c.getValues() + lastIndex := c.getLastNonEmptyIndex() // Include all fields up to and including the last non-empty one // This ensures empty strings in the middle are matched explicitly @@ -569,15 +572,8 @@ func (c *CasbinRule) toStringPolicy() []string { policy = append(policy, c.Ptype) } - // Build array of values to determine the last non-empty position - values := []string{c.V0, c.V1, c.V2, c.V3, c.V4, c.V5} - lastIndex := -1 - for i := len(values) - 1; i >= 0; i-- { - if values[i] != "" { - lastIndex = i - break - } - } + values := c.getValues() + lastIndex := c.getLastNonEmptyIndex() // Include all values up to and including the last non-empty one // This preserves empty strings in the middle while trimming trailing empty strings From 4208bff32a09e5c5ebd863b57390b74a483b52ea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 22 Nov 2025 09:04:56 +0000 Subject: [PATCH 5/5] Optimize helper methods to avoid duplicate slice creation Co-authored-by: nomeguy <85475922+nomeguy@users.noreply.github.com> --- adapter.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/adapter.go b/adapter.go index 8f0dd71..402c412 100644 --- a/adapter.go +++ b/adapter.go @@ -156,10 +156,9 @@ func (r *CasbinRule) getValues() []string { return []string{r.V0, r.V1, r.V2, r.V3, r.V4, r.V5} } -// getLastNonEmptyIndex returns the index of the last non-empty value in the rule +// getLastNonEmptyIndex returns the index of the last non-empty value in the given slice // Returns -1 if all values are empty -func (r *CasbinRule) getLastNonEmptyIndex() int { - values := r.getValues() +func getLastNonEmptyIndex(values []string) int { for i := len(values) - 1; i >= 0; i-- { if values[i] != "" { return i @@ -181,7 +180,7 @@ func (r *CasbinRule) String() string { sb.WriteString(r.Ptype) values := r.getValues() - lastIndex := r.getLastNonEmptyIndex() + lastIndex := getLastNonEmptyIndex(values) // Include all values up to and including the last non-empty one // This preserves empty strings in the middle while trimming trailing empty strings @@ -553,7 +552,7 @@ func (c *CasbinRule) queryString() (string, []interface{}) { queryStr := "ptype = ?" values := c.getValues() - lastIndex := c.getLastNonEmptyIndex() + lastIndex := getLastNonEmptyIndex(values) // Include all fields up to and including the last non-empty one // This ensures empty strings in the middle are matched explicitly @@ -573,7 +572,7 @@ func (c *CasbinRule) toStringPolicy() []string { } values := c.getValues() - lastIndex := c.getLastNonEmptyIndex() + lastIndex := getLastNonEmptyIndex(values) // Include all values up to and including the last non-empty one // This preserves empty strings in the middle while trimming trailing empty strings