hidingmyname opened a new issue, #277: URL: https://github.com/apache/casbin-cpp/issues/277
**Location** (apache/casbin-cpp @ default branch `0ad64a7`): - [`casbin/util/escape_assertion.cpp:34-51`](https://github.com/apache/casbin-cpp/blob/0ad64a739bd841099c6dbbae4a1f60d12e6cbc0a/casbin/util/escape_assertion.cpp#L34-L51) — `EscapeAssertion` - [`include/casbin/util/util.h:37-43`](https://github.com/apache/casbin-cpp/blob/0ad64a739bd841099c6dbbae4a1f60d12e6cbc0a/include/casbin/util/util.h#L37-L43) — public declaration ```cpp std::string EscapeAssertion(std::string s) { std::regex regex_s("[a-zA-Z0-9. ]+"); std::sregex_iterator words_begin = std::sregex_iterator(s.begin(), s.end(), regex_s); auto words_end = std::sregex_iterator(); for (std::sregex_iterator k = words_begin; k != words_end; ++k) { std::smatch match = *k; std::string match_str = match.str(); int pos = static_cast<int>(match_str.find(".")); if (pos != -1) { std::string new_str = match_str.replace(pos, 1, "_"); s = s.replace(match.position(), match.str().length(), new_str); } } return s; } ``` **Problem** ```cpp casbin::EscapeAssertion("r.sub_data.attr.id == p.sub && r.act == p.act"); // actual: "r_sub_data_attr.id == p_sub && r_act == p_act" // expected: "r_sub_data.attr.id == p_sub && r_act == p_act" ``` The function tokenizes with `[a-zA-Z0-9. ]+`, which doesn't include `_`. In `r.sub_data.attr.id` the underscore ends the first match (`r.sub`), and `data.attr.id` becomes a second match whose first dot is replaced. Only the `r.` / `p.` separator should become `_`; the attribute path `.attr.id` should be left alone, which is what already happens for names without underscores: [`tests/util_test.cpp`](https://github.com/apache/casbin-cpp/blob/0ad64a739bd841099c6dbbae4a1f60d12e6cbc0a/tests/util_test.cpp#L29-L36) expects `r.attr.value == p.attr` → `r_attr.value == p_attr`. Scope: `EscapeAssertion` is declared in the public header `include/casbin/util/util.h` (included by `casbin/casbin.h`), documented as taking "the value of the matcher and effect assertions". In the current `Enforce` path it is only applied to the argument names of `eval(...)` ([`casbin/enforcer.cpp:120-124`](https://github.com/apache/casbin-cpp/blob/0ad64a739bd841099c6dbbae4a1f60d12e6cbc0a/casbin/enforcer.cpp#L120-L124)), so **this is not an enforcement failure**: `Enforce` results are not affected. This is about the result of the public utility for code that calls it directly (model tooling, converters). **Reproduction** Verbatim gtest output for the call above: ```text Expected equality of these values: my_res Which is: "r_sub_data_attr.id == p_sub && r_act == p_act" res Which is: "r_sub_data.attr.id == p_sub && r_act == p_act" ``` Environment: - Commit: [`0ad64a7`](https://github.com/apache/casbin-cpp/commit/0ad64a739bd841099c6dbbae4a1f60d12e6cbc0a) (master) - Toolchain: GCC 11.5.0 **Already fixed in apache/casbin-node-casbin** PR: https://github.com/apache/casbin-node-casbin/pull/394 — "fix: escape assersion bug" (merged 2022-10-04T14:04:04Z, merge commit `5a339d858a9`). node-casbin's `escapeAssertion` also rewrote dots inside the nested attribute path of `r.sub_data.attr.id` (its old regex replaced every `r.` / `p.`, including the one at the end of `attr.`). The fix only rewrites `r.` / `p.` when preceded by a space or `(`, i.e. at the start of an identifier, and the added example model uses the same matcher as above. In node-casbin the helper is on the enforcement path, so the bug there also changed `Enforce` results; in casbin-cpp only the public helper's output is affected. How node-casbin fixed it: ```diff --- a/src/util/util.ts +++ b/src/util/util.ts @@ -17,8 +17,10 @@ import * as fs from 'fs'; // escapeAssertion escapes the dots in the assertion, // because the expression evaluation doesn't support such variable names. function escapeAssertion(s: string): string { - s = s.replace(/r\./g, 'r_'); - s = s.replace(/p\./g, 'p_'); + s = ' ' + s; + s = s.replace(/(?<=[\(| ])r\./g, 'r_'); + s = s.replace(/(?<=[\(| ])p\./g, 'p_'); + s = s.trim(); return s; } --- a/examples/abac_attr_model.conf +++ b/examples/abac_attr_model.conf @@ -0,0 +1,11 @@ +[request_definition] +r = sub_data, act + +[policy_definition] +p = sub, act + +[policy_effect] +e = some(where (p.eft == allow)) + +[matchers] +m = r.sub_data.attr.id == p.sub && r.act == p.act \ No newline at end of file ``` **Suggested fix** Find the dot to rewrite by matching the whole identifier: an `r`/`p`-style prefix at an identifier boundary (`[A-Za-z_][A-Za-z0-9_]*`) followed by `.`, as node-casbin anchors its replacement. Rewrite only that dot, keep the rest of the path, and add `r.sub_data.attr.id` to `TestEscapeAssertion`. Not yet tested against this repository. Related: searched issues/PRs (all states) for underscore, ABAC attribute dot, escape assertion: no hits. Dots inside quoted string literals are a separate problem with a different cause and are reported separately. -- 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]
