This is an automated email from the ASF dual-hosted git repository. hsluoyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/casbin.git
commit f64a3c0bb196de76f4621fdd829298dfc1e2bba1 Author: Yang Luo <[email protected]> AuthorDate: Tue Jun 16 21:46:33 2026 +0800 feat: quote CSV fields containing commas in SavePolicy --- persist/adapter.go | 18 +++++++++ persist/file-adapter/adapter.go | 15 +++++--- persist/string-adapter/adapter.go | 15 +++++--- persist/string-adapter/adapter_test.go | 69 ++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/persist/adapter.go b/persist/adapter.go index 800e757..c15a4c8 100644 --- a/persist/adapter.go +++ b/persist/adapter.go @@ -15,6 +15,7 @@ package persist import ( + "bytes" "encoding/csv" "strings" @@ -40,6 +41,23 @@ func LoadPolicyLine(line string, m model.Model) error { return LoadPolicyArray(tokens, m) } +// PolicyLineToCsv serializes a policy type and its rule fields into a CSV-safe line. +// Fields containing commas are properly quoted so the line can be round-tripped +// through LoadPolicyLine without corruption. +func PolicyLineToCsv(ptype string, rule []string) (string, error) { + record := append([]string{ptype}, rule...) + var buf bytes.Buffer + w := csv.NewWriter(&buf) + if err := w.Write(record); err != nil { + return "", err + } + w.Flush() + if err := w.Error(); err != nil { + return "", err + } + return strings.TrimRight(buf.String(), "\r\n"), nil +} + // LoadPolicyArray loads a policy rule to model. func LoadPolicyArray(rule []string, m model.Model) error { key := rule[0] diff --git a/persist/file-adapter/adapter.go b/persist/file-adapter/adapter.go index 454b2d1..223581c 100644 --- a/persist/file-adapter/adapter.go +++ b/persist/file-adapter/adapter.go @@ -23,7 +23,6 @@ import ( "github.com/casbin/casbin/v3/model" "github.com/casbin/casbin/v3/persist" - "github.com/casbin/casbin/v3/util" ) // Adapter is the file adapter for Casbin. @@ -68,16 +67,22 @@ func (a *Adapter) SavePolicy(model model.Model) error { for ptype, ast := range model["p"] { for _, rule := range ast.Policy { - tmp.WriteString(ptype + ", ") - tmp.WriteString(util.ArrayToString(rule)) + line, err := persist.PolicyLineToCsv(ptype, rule) + if err != nil { + return err + } + tmp.WriteString(line) tmp.WriteString("\n") } } for ptype, ast := range model["g"] { for _, rule := range ast.Policy { - tmp.WriteString(ptype + ", ") - tmp.WriteString(util.ArrayToString(rule)) + line, err := persist.PolicyLineToCsv(ptype, rule) + if err != nil { + return err + } + tmp.WriteString(line) tmp.WriteString("\n") } } diff --git a/persist/string-adapter/adapter.go b/persist/string-adapter/adapter.go index c9655ac..388dc01 100644 --- a/persist/string-adapter/adapter.go +++ b/persist/string-adapter/adapter.go @@ -21,7 +21,6 @@ import ( "github.com/casbin/casbin/v3/model" "github.com/casbin/casbin/v3/persist" - "github.com/casbin/casbin/v3/util" ) // Adapter is the string adapter for Casbin. @@ -58,16 +57,22 @@ func (a *Adapter) SavePolicy(model model.Model) error { var tmp bytes.Buffer for ptype, ast := range model["p"] { for _, rule := range ast.Policy { - tmp.WriteString(ptype + ", ") - tmp.WriteString(util.ArrayToString(rule)) + line, err := persist.PolicyLineToCsv(ptype, rule) + if err != nil { + return err + } + tmp.WriteString(line) tmp.WriteString("\n") } } for ptype, ast := range model["g"] { for _, rule := range ast.Policy { - tmp.WriteString(ptype + ", ") - tmp.WriteString(util.ArrayToString(rule)) + line, err := persist.PolicyLineToCsv(ptype, rule) + if err != nil { + return err + } + tmp.WriteString(line) tmp.WriteString("\n") } } diff --git a/persist/string-adapter/adapter_test.go b/persist/string-adapter/adapter_test.go index 5e8727b..d41fe77 100644 --- a/persist/string-adapter/adapter_test.go +++ b/persist/string-adapter/adapter_test.go @@ -15,10 +15,12 @@ package stringadapter import ( + "strings" "testing" "github.com/casbin/casbin/v3" "github.com/casbin/casbin/v3/model" + "github.com/casbin/casbin/v3/persist" ) func Test_KeyMatchRbac(t *testing.T) { @@ -61,6 +63,73 @@ g, alice, data_group_admin } } +// Test_SavePolicyRoundTripWithCommas verifies that a policy rule whose fields contain +// commas (e.g. ABAC condition expressions) survives a SavePolicy → LoadPolicy round trip +// without corruption. See https://github.com/apache/casbin/issues/1733. +func Test_SavePolicyRoundTripWithCommas(t *testing.T) { + conf := ` +[request_definition] +r = sub, obj, act, cond + +[policy_definition] +p = sub, obj, act, cond + +[policy_effect] +e = some(where (p.eft == allow)) + +[matchers] +m = r.sub == p.sub && r.obj == p.obj && r.act == p.act +` + // cond field intentionally contains commas (as in ABAC expressions). + condWithComma := `r.attrs in ('val1','val2')` + line := "p, alice, data1, read, " + `"` + condWithComma + `"` + + a := NewAdapter(line) + m := model.NewModel() + if err := m.LoadModelFromText(conf); err != nil { + t.Fatalf("load model: %v", err) + } + e, err := casbin.NewEnforcer(m, a) + if err != nil { + t.Fatalf("new enforcer: %v", err) + } + + // SavePolicy serialises in-memory rules back to the adapter string. + err = e.SavePolicy() + if err != nil { + t.Fatalf("SavePolicy: %v", err) + } + + // The saved line must be a properly quoted CSV so that re-loading produces + // exactly one rule with the comma-containing cond field intact. + saved := a.Line + reloaded := model.NewModel() + err = reloaded.LoadModelFromText(conf) + if err != nil { + t.Fatalf("reload model: %v", err) + } + for _, l := range strings.Split(saved, "\n") { + if l == "" { + continue + } + err = persist.LoadPolicyLine(l, reloaded) + if err != nil { + t.Fatalf("LoadPolicyLine on saved line %q: %v", l, err) + } + } + + rules, err := reloaded.GetPolicy("p", "p") + if err != nil { + t.Fatalf("GetPolicy: %v", err) + } + if len(rules) != 1 { + t.Fatalf("expected 1 rule after round-trip, got %d (saved: %q)", len(rules), saved) + } + if rules[0][3] != condWithComma { + t.Errorf("cond field corrupted after round-trip: got %q, want %q", rules[0][3], condWithComma) + } +} + func Test_StringRbac(t *testing.T) { conf := ` [request_definition] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
