hsluoyz commented on PR #1766:
URL: https://github.com/apache/casbin/pull/1766#issuecomment-6294031416

   Thanks for the report and the clear write-up. I fixed this on master in 
0a73027 with a broader change, so I'm closing this PR.
   
   The same panic also comes from keyMatch2/3/4/5 and keyGet2/3, since they 
build a regexp from the policy pattern too. It is worse when one of them is 
registered as a role-manager matching function (`AddNamedMatchingFunc`): there 
it runs outside `Enforce`'s recover, so `AddNamedGroupingPolicy("g2", 
"/book/(1", ...)` with `util.KeyMatch2` panics the whole process.
   
   What 0a73027 does:
   - A pattern that fails to compile is cached together with its error, so it 
is not recompiled on every call.
   - All the `*Func` matcher wrappers (regexMatch, keyMatch2-5, keyGet2/3) 
return that error, e.g. `regexMatch: error parsing regexp: missing closing ): ` 
+ "`(GET`" + `, so `Enforce` reports it without the `panic:` prefix and stack 
trace.
   - The bool/string APIs (`RegexMatch`, `KeyMatch2`, ...) no longer panic; an 
invalid pattern matches nothing (fail closed), which keeps role-manager 
matching safe.
   
   Your test cases are included, plus one for the role-manager path.
   


-- 
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]

Reply via email to