Quoted fields in CSV policy files keep their quote characters, so quoted policy rules never match
Personne n'a encore pris cette issue.
Évaluation
- Difficulté
- 2/5
- Temps estimé
- 1-3 heures
- Accessibilité débutants
- 85/100
- Type d'issue
- Bug
- Clarté
- Clairement spécifiée
- Activité
- Active
- Stack technique
- cpp
- Domaine
- authorization
Piste de recherche
Commence dans casbin/persist/adapter.cpp au niveau de LoadPolicyLine et compare sa découpe sur les virgules avec le comportement documenté du quoting CSV. Reproduis le problème avec une ligne de politique contenant une valeur admin entre guillemets et une autre contenant une virgule entre guillemets. Ensuite, exécute la suite GoogleTest du projet et ajoute un cas ciblé si nécessaire. C’est terminé lorsque les champs entre guillemets sont chargés sans guillemets et que les lignes sans guillemets se comportent comme auparavant.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Description
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.
- Langage dominant
- C++
- Étoiles
- 257
- Forks
- 67
- Merge moyen
- 10 h 23 min
- PR mergées (30 j)
- 1
Préparer son environnement
- Aucun Dockerfile ni fichier Docker Compose
- Aucun modèle de pull request
- Lire le guide de contribution
Par où commencer
- Lisez l'issue en entier, puis le guide de contribution du projet.
- Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
- Forkez le dépôt et travaillez sur une branche.
- Ouvrez une pull request qui référence le numéro de l'issue.
Autres issues de apache/casbin-cpp
-
ci: automated PR performance benchmark comparisonPeut-être à nouveau libre @knyk-dev l’a pris il y a 298 jours, et aucune pull request n’est ouverte. Ouverteenhancement
apache/casbin-cpp#266 · 1 personne assignée ·
-
Porting Casbin-CPP to Real-Time Operating Systems (RTOS)Peut-être à nouveau libre @hsluoyz l’a pris il y a 1277 jours, et aucune pull request n’est ouverte. Ouverteenhancement
apache/casbin-cpp#229 · 1 commentaire · 1 personne assignée ·
-
Use the same performance benchmark tests for all languagesPeut-être à nouveau libre @hsluoyz l’a pris il y a 1575 jours, et aucune pull request n’est ouverte. Ouverteenhancement
apache/casbin-cpp#207 · 3 commentaires · 1 personne assignée ·
-
Benchmarking pycasbin's performance to compare it with Python's casbin implementationPeut-être à nouveau libre @hsluoyz l’a pris il y a 1747 jours, et aucune pull request n’est ouverte. Ouverteenhancement
apache/casbin-cpp#181 · 1 commentaire · 1 personne assignée ·
-
Make an authorization middleware for Envoy based on CasbinPeut-être à nouveau libre @hsluoyz l’a pris il y a 1951 jours, et aucune pull request n’est ouverte. Ouverteenhancement
apache/casbin-cpp#115 · 1 commentaire · 1 personne assignée ·
Toutes les issues de apache/casbin-cpp
Issues similaires
-
Difficulté 2/5 1-3 heures Accessibilité débutants 72/100
Les mainteneurs répondent en général sous 1 jour
-
bug
Difficulté 2/5 1-3 heures Accessibilité débutants 62/100
pocoproject/poco#5626 ·
Les mainteneurs répondent en général sous 2 jours
-
Difficulté 2/5 1-3 heures Accessibilité débutants 78/100
-
Issue-Bug Needs-Triage
Difficulté 2/5 1-3 heures Accessibilité débutants 62/100
microsoft/winget-cli#6645 ·
Les mainteneurs répondent en général sous 2 jours
-
cleanup help wanted
Difficulté 2/5 1-3 heures Accessibilité débutants 62/100
ESPresense/ESPresense#2602 ·
Les mainteneurs répondent en général sous 1 jour