casbin / casbin-rs

An authorization library that supports access control models like ACL, RBAC, ABAC in Rust.
https://casbin.org
Apache License 2.0
844 stars 69 forks source link

fix: possible endless loop when passing /{} or /:/ to keyMatch2 and 3 #283

Closed valkum closed 2 years ago

valkum commented 2 years ago

The go implementation uses a replace_all instead of this loop which avoids the possible endless loop. This PR changes the implementation of keyMatch2 and keyMatch3 to the one similar to the go implementation.

A benchmark with a single parameter /foo/{bar}/foo showed no meaningful impact. 15.09 ns to 15.1 ns per iteration.

Changes

/{} will no be replaced by /[^/]+ (/: accordingly), basically using them as unnamed url parts. Thus /{}/bar will now match /foo/bar

Fixes #282

casbin-bot commented 2 years ago

@smrpn @hackerchai @PsiACE @GopherJ please review

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

codecov[bot] commented 2 years ago

Codecov Report

Merging #283 (5a99c3f) into master (ec04f45) will increase coverage by 0.05%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
+ Coverage   80.99%   81.04%   +0.05%     
==========================================
  Files          23       23              
  Lines        3325     3334       +9     
==========================================
+ Hits         2693     2702       +9     
  Misses        632      632              
Impacted Files Coverage Δ
src/model/function_map.rs 94.44% <100.00%> (+0.50%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ec04f45...5a99c3f. Read the comment docs.

PsiACE commented 2 years ago

hi @valkum , pls make clippy happy. ty!

hsluoyz commented 2 years ago

@valkum plz fix:

image

image

valkum commented 2 years ago

I fixed the semantic commit message . Clippy is not happy because of some different deprecation warning of rhai. I guess that should be fixed in its own pull request.

PsiACE commented 2 years ago

Clippy is not happy because of some different deprecation warning of rhai. I guess that should be fixed in its own pull request.

You are right, I will be doing a fix later, probably tomorrow.