Quoted fields in CSV policy files keep their quote characters, so quoted policy rules never match
还没有人认领这个 Issue。
评估
- 难度
- 2/5
- 预计耗时
- 1-3 小时
- 新手友好度
- 85/100
- Issue 类型
- 缺陷
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 技术栈
- cpp
调研方向
从 casbin/persist/adapter.cpp 中的 LoadPolicyLine 开始,将其按逗号拆分的行为与文档中说明的 CSV 引号处理方式进行对比。用一条包含带引号的 admin 值的策略行,以及另一条包含带引号的逗号的策略行复现问题,然后运行项目的 GoogleTest 测试套件,必要时添加一个针对性测试用例。当带引号的字段加载后不含引号,而不带引号的行仍保持原有行为时,即告完成。
由索引模型根据 Issue 内容生成。
描述
Location (apache/casbin-cpp @ default branch 0ad64a7):
casbin/persist/adapter.cpp:28-43—LoadPolicyLine, used by the file, filtered-file and string adapters
void LoadPolicyLine(const std::string& line, const std::shared_ptr<Model>& model) {
if (line.empty() || line.find('#') == 0)
return;
std::vector<std::string> tokens = Split(line, ",", -1);
for (int i = 0; i < tokens.size(); i++)
tokens[i] = Trim(tokens[i]);
std::string key = tokens[0];
std::string sec = key.substr(0, 1);
std::vector<std::string> new_tokens(tokens.begin() + 1, tokens.end());
Problem
The Casbin Policy Storage docs describe quoted CSV fields:
If a field contains commas, wrap it in double quotes … If a field contains both commas and double quotes, wrap it in double quotes and escape internal quotes by doubling them
LoadPolicyLine does Split(line, ",") and then Trim on each token. Trim only removes whitespace, so "admin" keeps its quotes and the quote characters are stored as part of the value. A request for admin therefore doesn't match a policy line written as "admin", and Enforce returns false. For the same reason a quoted comma ("data1,data2") is split into two fields (from reading the code; not run).
Expected: a quoted field is unquoted when loading: "admin" → admin, "data1,data2" → one field data1,data2, "" inside a quoted field → ". Spaces inside the quotes stay part of the value.
Reproduction
quoted_policy.csv:
p, admin, /, GET
p, "admin", /, POST
#include <iostream>
#include <casbin/casbin.h>
int main() {
casbin::Enforcer e("examples/basic_model.conf", "quoted_policy.csv");
std::cout << std::boolalpha
<< e.Enforce({"admin", "/", "GET"}) << "\n" // true
<< e.Enforce({"admin", "/", "POST"}) << "\n"; // false, expected true
}
Observed with a gtest:
Expected equality of these values:
e.Enforce({"admin", "/", "POST"})
Which is: false
true
The unquoted GET line in the same file is enforced as expected.
Environment:
- Commit:
0ad64a7(master) - Toolchain: GCC 11.5.0, Linux x86_64, default CMake build
Already fixed in apache/casbin-node-casbin
PR: https://github.com/apache/casbin-node-casbin/pull/494 — "feat: add BracketAwareCsvParser to parse CSV file better" (merged 2024-11-22T10:12:57Z, merge commit 4c738831ebb). node-casbin's loadPolicyLine now strips the outer double quotes from the section key and from every policy value and turns "" into ", so a quoted rule loads as the plain value. The PR also adds bracket-aware token joining for policy expressions; the unquoting in PolicyLoader.loadPolicyLine is the part that addresses this behaviour. (node-casbin also trims whitespace inside the quotes; the scope proposed below keeps it.)
How node-casbin fixed it:
--- a/src/persist/helper.ts
+++ b/src/persist/helper.ts
@@ -1,23 +1,83 @@
...
+ loadPolicyLine(line: string, model: Model): void {
+ const tokens = this.parser.parse(line);
if (!tokens || !tokens[0]) {
return;
}
- const key = tokens[0][0];
+ let key = tokens[0][0].trim();
+ if (key.startsWith('"') && key.endsWith('"')) {
+ key = key.slice(1, -1);
+ }
+
...
@@ -28,6 +88,22 @@ export class Helper {
if (!policy) {
return;
}
- policy.policy.push(tokens[0].slice(1));
+
+ const values = tokens[0].slice(1).map((v) => {
+ if (v.startsWith('"') && v.endsWith('"')) {
+ v = v.slice(1, -1);
+ }
+ return v.replace(/""/g, '"').trim();
+ });
+
+ policy.policy.push(values);
+ }
...
Suggested fix
Replace the Split + Trim tokenizer in LoadPolicyLine with a small CSV-aware splitter: a field wrapped in double quotes may contain commas, "" inside it becomes ", whitespace inside the quotes is kept, and unquoted fields are trimmed as before:
diff --git a/casbin/persist/adapter.cpp b/casbin/persist/adapter.cpp
--- a/casbin/persist/adapter.cpp
+++ b/casbin/persist/adapter.cpp
@@ -24,14 +24,44 @@
namespace casbin {
+namespace {
+ // SplitPolicyLine splits a CSV policy line into fields. Unquoted fields are trimmed. A field wrapped in double
+ // quotes may contain commas, "" inside it stands for one double quote, and whitespace inside the quotes is kept.
+ std::vector<std::string> SplitPolicyLine(const std::string& line) {
+ std::vector<std::string> tokens;
+ std::string token;
+ bool in_quotes = false;
+ bool quoted = false;
+ for (size_t i = 0; i < line.size(); i++) {
+ char c = line[i];
+ if (in_quotes) {
+ if (c != '"')
+ token += c;
+ else if (i + 1 < line.size() && line[i + 1] == '"')
+ token += line[++i];
+ else
+ in_quotes = false;
+ } else if (c == ',') {
+ tokens.push_back(quoted ? token : Trim(token));
+ token.clear();
+ quoted = false;
+ } else if (c == '"' && !quoted && Trim(token).empty()) {
+ in_quotes = quoted = true;
+ } else if (!quoted || !std::isspace(static_cast<unsigned char>(c))) {
+ token += c;
+ }
+ }
+ tokens.push_back(quoted ? token : Trim(token));
+ return tokens;
+ }
+} // namespace
+
// LoadPolicyLine loads a text line as a policy rule to model.
void LoadPolicyLine(const std::string& line, const std::shared_ptr<Model>& model) {
if (line.empty() || line.find('#') == 0)
return;
- std::vector<std::string> tokens = Split(line, ",", -1);
- for (int i = 0; i < tokens.size(); i++)
- tokens[i] = Trim(tokens[i]);
+ std::vector<std::string> tokens = SplitPolicyLine(line);
std::string key = tokens[0];
std::string sec = key.substr(0, 1);
Verified on 0ad64a7 (GCC 11.4.0, GoogleTest 1.14.0): the test above, extended with a quoted-comma line (p, "admin", "/a,b", PUT) and a doubled-quote line, fails without this patch and passes with it; no other test results change.
I can submit a PR for this.
Related: searched issues/PRs for LoadPolicyLine, quotes/quoted CSV, double quotes; the only hit, #132, is about RBAC role links after LoadPolicyLine, not quoting.
- 主要语言
- C++
- 星标
- 257
- 派生
- 67
- 平均合并
- 10 小时 23 分钟
- 30 天内合并 PR
- 1
环境准备
- 没有 Dockerfile 或 Docker Compose 文件
- 没有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
apache/casbin-cpp 的其他 Issue
-
ci: automated PR performance benchmark comparison可能重新可做 @knyk-dev 于 298 天前认领,目前没有进行中的 PR。 未关闭enhancement
apache/casbin-cpp#266 · 已指派 1 人 ·
-
Porting Casbin-CPP to Real-Time Operating Systems (RTOS)可能重新可做 @hsluoyz 于 1276 天前认领,目前没有进行中的 PR。 未关闭enhancement
apache/casbin-cpp#229 · 1 条评论 · 已指派 1 人 ·
-
Use the same performance benchmark tests for all languages可能重新可做 @hsluoyz 于 1575 天前认领,目前没有进行中的 PR。 未关闭enhancement
apache/casbin-cpp#207 · 3 条评论 · 已指派 1 人 ·
-
Benchmarking pycasbin's performance to compare it with Python's casbin implementation可能重新可做 @hsluoyz 于 1747 天前认领,目前没有进行中的 PR。 未关闭enhancement
apache/casbin-cpp#181 · 1 条评论 · 已指派 1 人 ·
-
Make an authorization middleware for Envoy based on Casbin可能重新可做 @hsluoyz 于 1951 天前认领,目前没有进行中的 PR。 未关闭enhancement
apache/casbin-cpp#115 · 1 条评论 · 已指派 1 人 ·
查看 apache/casbin-cpp 的全部 Issue
相似的 Issue
-
Issue-Bug Needs-Triage
难度 2/5 1-3 小时 新手友好度 62/100
microsoft/winget-cli#6645 ·
维护者通常 2 天内回复
-
cleanup help wanted
难度 2/5 1-3 小时 新手友好度 62/100
ESPresense/ESPresense#2602 ·
维护者通常 1 天内回复
-
devex good first issue
难度 2/5 1-3 小时 新手友好度 62/100
维护者通常 1 天内回复
-
artifact:simulation-scenario domain:physics good first issue help wanted role:validation testing
难度 2/5 1-3 小时 新手友好度 75/100
-
难度 2/5 1-3 小时 新手友好度 62/100
维护者通常 3 天内回复