casbin / Casbin.NET

An authorization library that supports access control models like ACL, RBAC, ABAC in .NET (C#)
https://casbin.org
Apache License 2.0
1.13k stars 110 forks source link

[BUG] White space is included in policy effect in preview branch #263

Closed AsakusaRinne closed 1 year ago

AsakusaRinne commented 2 years ago

The environment

In branch preview, windows 11, .NET 6

The description

In the preview branch, the loaded policy effect includes white space, while the PermConstants.PolicyEffect does not.

For instance, we add some white spaces to the basic_model.conf in the examples of unit tests, as shown below.

[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act 

[policy_effect]
e = some(where (p.eft == allow))    # Before this comment there are some white spaces.

[matchers]
m = r.sub == p.sub && r.obj == p.obj && r.act == p.act

Then the expression of policy effect cannot be matched, which throws an exception.

If it's an unexpected behavior, I would like to solve it soon.

casbin-bot commented 2 years ago

@sagilio @xcaptain @huazhikui

hsluoyz commented 2 years ago

@sagilio

hsluoyz commented 2 years ago

@AsakusaRinne plz see the supported comment format in Golang, I'm not sure if this inline comment is supported or not.

hsluoyz commented 2 years ago

@AsakusaRinne is this issue the same one as: https://github.com/casbin/Casbin.NET/issues/258 ? Why open two duplicated issues?

AsakusaRinne commented 2 years ago

@AsakusaRinne is this issue the same one as: #258 ? Why open two duplicated issues?

Not really, #258 is the lack of process of comment symbol, while this issue is raised because the expression includes white symbol. I agree that they are similar, I would like to solve it with #258 in the same PR if it's indeed an unexpected behavior.

hsluoyz commented 2 years ago

@AsakusaRinne I agree with you, and both are commenting style problems. For this issue my comment is already offered at: https://github.com/casbin/Casbin.NET/pull/262#issuecomment-1159721651 . They can be tackled in one PR.

sagilio commented 1 year ago

Fixed by #262