feat: quote CSV fields containing commas in SavePolicy

Author: hsluoyz

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]

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")
 		}
 	}

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")
 		}
 	}

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]