fix: proper concurrent access to field map cache

Author: Gilthoniel

internal_api.go

@@ -493,5 +493,5 @@ func (e *Enforcer) GetFieldIndex(ptype string, field string) (int, error) {
 
 func (e *Enforcer) SetFieldIndex(ptype string, field string, index int) {
 	assertion := e.model["p"][ptype]
-	assertion.FieldIndexMap[field] = index
+	assertion.FieldIndexMap.Store(field, index)
 }

model/assertion.go

@@ -17,6 +17,7 @@ package model
 import (
 	"errors"
 	"strings"
+	"sync"
 
 	"github.com/casbin/casbin/v2/log"
 	"github.com/casbin/casbin/v2/rbac"
@@ -33,11 +34,25 @@ type Assertion struct {
 	PolicyMap     map[string]int
 	RM            rbac.RoleManager
 	CondRM        rbac.ConditionalRoleManager
-	FieldIndexMap map[string]int
+	FieldIndexMap sync.Map
 
 	logger log.Logger
 }
 
+func (ast *Assertion) GetFieldIndex(field string) (idx int, ok bool) {
+	value, found := ast.FieldIndexMap.Load(field)
+	if found {
+		ok = true
+		idx = value.(int)
+	}
+	return idx, ok
+}
+
+func (ast *Assertion) GetFieldIndexOrZero(field string) (idx int) {
+	idx, _ = ast.GetFieldIndex(field)
+	return idx
+}
+
 func (ast *Assertion) buildIncrementalRoleLinks(rm rbac.RoleManager, op PolicyOp, rules [][]string) error {
 	ast.RM = rm
 	count := strings.Count(ast.Value, "_")
@@ -182,13 +197,17 @@ func (ast *Assertion) copy() *Assertion {
 	}
 
 	newAst := &Assertion{
-		Key:           ast.Key,
-		Value:         ast.Value,
-		PolicyMap:     policyMap,
-		Tokens:        tokens,
-		Policy:        policy,
-		FieldIndexMap: ast.FieldIndexMap,
+		Key:       ast.Key,
+		Value:     ast.Value,
+		PolicyMap: policyMap,
+		Tokens:    tokens,
+		Policy:    policy,
 	}
 
+	ast.FieldIndexMap.Range(func(key, value interface{}) bool {
+		newAst.FieldIndexMap.Store(key, value)
+		return true
+	})
+
 	return newAst
 }

model/model.go

@@ -76,7 +76,6 @@ func (model Model) AddDef(sec string, key string, value string) bool {
 	ast.Key = key
 	ast.Value = value
 	ast.PolicyMap = make(map[string]int)
-	ast.FieldIndexMap = make(map[string]int)
 	ast.setLogger(model.GetLogger())
 
 	if sec == "r" || sec == "p" {
@@ -419,7 +418,7 @@ func (model Model) Copy() Model {
 
 func (model Model) GetFieldIndex(ptype string, field string) (int, error) {
 	assertion := model["p"][ptype]
-	if index, ok := assertion.FieldIndexMap[field]; ok {
+	if index, ok := assertion.GetFieldIndex(field); ok {
 		return index, nil
 	}
 	pattern := fmt.Sprintf("%s_"+field, ptype)
@@ -433,6 +432,6 @@ func (model Model) GetFieldIndex(ptype string, field string) (int, error) {
 	if index == -1 {
 		return index, fmt.Errorf(field + " index is not set, please use enforcer.SetFieldIndex() to set index")
 	}
-	assertion.FieldIndexMap[field] = index
+	assertion.FieldIndexMap.Store(field, index)
 	return index, nil
 }

model/model_test.go

@@ -15,7 +15,7 @@
 package model
 
 import (
-	"io/ioutil"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -63,7 +63,7 @@ func TestNewModelFromFile(t *testing.T) {
 }
 
 func TestNewModelFromString(t *testing.T) {
-	modelBytes, _ := ioutil.ReadFile(basicExample)
+	modelBytes, _ := os.ReadFile(basicExample)
 	modelString := string(modelBytes)
 	m, err := NewModelFromString(modelString)
 	if err != nil {
@@ -125,7 +125,47 @@ func TestModel_AddDef(t *testing.T) {
 	}
 }
 
-func TestModelToTest(t *testing.T) {
+func TestModel_Copy(t *testing.T) {
+	m, err := NewModelFromFile(basicExample)
+	if err != nil {
+		t.Errorf("model failed to load from file: %s", err)
+	}
+
+	newModel := m.Copy()
+	if newModel.ToText() != m.ToText() {
+		t.Errorf("new model is not equal to original")
+	}
+}
+
+func TestModel_Copy_includesFieldMapInCopy(t *testing.T) {
+	m, err := NewModelFromFile(basicExample)
+	if err != nil {
+		t.Errorf("model failed to load from file: %s", err)
+	}
+
+	idx, _ := m.GetFieldIndex("p", "act")
+	if idx != 2 {
+		t.Errorf("unexpected field index: %d", idx)
+	}
+
+	newModel := m.Copy()
+	if newModel.ToText() != m.ToText() {
+		t.Error("new model is not equal to original")
+	}
+
+	assertion, err := newModel.GetAssertion("p", "p")
+	if err != nil {
+		t.Errorf("model failed to get assertion: %s", err)
+	}
+	if _, ok := assertion.GetFieldIndex("act"); !ok {
+		t.Errorf("model does not have the field index in cache")
+	}
+	if idx, err := newModel.GetFieldIndex("p", "act"); err != nil || idx != 2 {
+		t.Errorf("unexpected field index: %s - %d", err, idx)
+	}
+}
+
+func TestModel_ToText(t *testing.T) {
 	testModelToText(t, "r.sub == p.sub && r.obj == p.obj && r_func(r.act, p.act) && testr_func(r.act, p.act)", "r_sub == p_sub && r_obj == p_obj && r_func(r_act, p_act) && testr_func(r_act, p_act)")
 	testModelToText(t, "r.sub == p.sub && r.obj == p.obj && p_func(r.act, p.act) && testp_func(r.act, p.act)", "r_sub == p_sub && r_obj == p_obj && p_func(r_act, p_act) && testp_func(r_act, p_act)")
 }

model/policy.go

@@ -229,14 +229,15 @@ func (model Model) AddPolicy(sec string, ptype string, rule []string) error {
 	assertion.PolicyMap[strings.Join(rule, DefaultSep)] = len(model[sec][ptype].Policy) - 1
 
 	hasPriority := false
-	if _, ok := assertion.FieldIndexMap[constant.PriorityIndex]; ok {
+	if _, ok := assertion.FieldIndexMap.Load(constant.PriorityIndex); ok {
 		hasPriority = true
 	}
 	if sec == "p" && hasPriority {
-		if idxInsert, err := strconv.Atoi(rule[assertion.FieldIndexMap[constant.PriorityIndex]]); err == nil {
+		priorityIndex := assertion.GetFieldIndexOrZero(constant.PriorityIndex)
+		if idxInsert, err := strconv.Atoi(rule[priorityIndex]); err == nil {
 			i := len(assertion.Policy) - 1
 			for ; i > 0; i-- {
-				idx, err := strconv.Atoi(assertion.Policy[i-1][assertion.FieldIndexMap[constant.PriorityIndex]])
+				idx, err := strconv.Atoi(assertion.Policy[i-1][priorityIndex])
 				if err != nil || idx <= idxInsert {
 					break
 				}