vlieven opened a new issue, #435:
URL: https://github.com/apache/casbin-pycasbin/issues/435
## Description
`RoleManager.add_link` only loops over `all_roles `when a matching function
is configured:
```python
if self.matching_func != None:
for r in self.all_roles.values():
...
```
`RoleManager.delete_link` has the same loop, but without that check:
```python
for r in self.all_roles.values():
if r.name != user.name and self._matching_fn(user.name, r.name,
MatchOrder.PATTERN_STR):
r.remove_role(role)
if r.name != role.name and self._matching_fn(role.name, r.name,
MatchOrder.PATTERN_STR):
role.remove_role(r)
```
When `matching_func` is `None`, every `_matching_fn` call ends up in
`match_error_handler(None, ...)`. That raises a TypeError, which the bare
except swallows and turns into False. The result is correct, but removing a
single link costs two raised and caught exceptions per role.
This shows up when you combine `add_named_matching_func("g", None)` with a
domain matching function, for example to use `*` as a domain wildcard (`g,
alice, admin, *`, as described in the RBAC with pattern docs). Disabling the
role matcher is needed there, because `DomainManagerBase` installs an equality
lambda by default. That lambda makes `add_link` scan all roles on every insert,
which makes rebuilding the per-domain role managers quadratic. But a wildcard
link is present in every cached domain's role manager, so removing it runs the
unguarded loop once per domain.
## Reproduction
```python
import time
import casbin
from casbin.model import Model
from casbin.util import key_match
MODEL = """
[request_definition]
r = sub, dom, obj, act
[policy_definition]
p = sub, act
[role_definition]
g = _, _, _
[policy_effect]
e = some(where (p.eft == allow))
[matchers]
m = g(r.sub, p.sub, r.obj) && r.act == p.act
"""
model = Model()
model.load_model_from_text(MODEL)
e = casbin.Enforcer(model)
e.add_named_matching_func("g", None)
e.add_named_domain_matching_func("g", key_match)
e.add_policy("reader", "read")
e.add_named_grouping_policies(
"g",
[[f"group{i % 30}", "reader", f"resource{i}"] for i in range(1000)]
+ [[f"user{i}", f"group{i % 30}", "*"] for i in range(450)],
)
# Warm up the per-domain role managers
for i in range(1000):
e.enforce("user0", "", f"resource{i}", "read")
start = time.perf_counter()
e.remove_named_grouping_policy("g", "user1", "group1", "*")
print(f"remove_named_grouping_policy took {(time.perf_counter() - start) *
1000:.0f} ms")
```
On pycasbin 2.8.0 with Python 3.13, removing one link takes about 356 ms.
With the guard described below, it takes about 3 ms. With a SyncedEnforcer,
this runs while holding the write lock, so every enforce call waits for it.
## Expected behavior
`delete_link` skips the pattern loop when no matching function is set, the
same way `add_link` does.
## Proposed fix
Wrap the loop in `delete_link` in the same check `add_link` uses:
```python
if self.matching_func is not None:
for r in self.all_roles.values():
...
```
## Environment
- pycasbin 2.8.0 (the code is the same on master)
- Python 3.13
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
