Repository: casbin/casbin (go)

Issue:
model.conf has grammatical errors Enforce func return err is nil

## demo code
main.go

``` go
package main

import (
	"log"
	"net/http"

	"github.com/casbin/casbin/v2"
)

func main() {
	e, err := casbin.NewEnforcer("model.conf", "policy.csv")
	if err != nil {
		log.Fatal(err)
	}
	e.EnableLog(true)
	ok, err := e.Enforce("admin", "/v1/admin/manager/1", http.MethodPut)
	if err != nil {
		log.Fatal(err)
	}
	log.Println(ok)
}
```

model.conf

```env
[request_definition]
r = sub, obj, act

[policy_definition]
p = sub, obj, act

[role_definition]
g = _, _

[policy_effect]
e = some(where (p.eft == allow))

[matchers]
m = g(r.sub, p.sub) && keyMatch2(r.obj == p.obj) && regexMatch(r.act == p.act)
```

policy.csv

``` csv
p, common, /v1/admin/manager/:id, PUT
g, admin, common
```

casbin use version: `v2.2.1` [demo](https://github.com/zeromake/casbin-demo/tree/master)
``` bash
$ go run .\main.go
2020/03/18 16:35:24 false
```
switch casbin version: `v2.1.2`  [demo](https://github.com/zeromake/casbin-demo/tree/v2.1.2)
```bash
$ go run .\main.go
panic: interface conversion: interface {} is bool, not string

goroutine 1 [running]:
github.com/Knetic/govaluate.makeFunctionStage.func1(0x0, 0x0, 0x707f80, 0xa47ac7, 0x7ee3e0, 0xc000044ed0, 0x0, 0x0, 0x4a, 0xc0000505a0)
        C:/Users/coolz/go/pkg/mod/github.com/!knetic/govaluate@v3.0.1-0.20171022003610-9aa49832a739+incompatible/evaluationStage.go:245 +0x106
github.com/Knetic/govaluate.EvaluableExpression.evaluateStage(0x784bde, 0x22, 0x1, 0xc0000ce000, 0x14, 0x20, 0xc000050820, 0xc0000142d0, 0x4a, 0xc0000507d0, ...)
        C:/Users/coolz/go/pkg/mod/github.com/!knetic/govaluate@v3.0.1-0.20171022003610-9aa49832a739+incompatible/EvaluableExpression.go:205 +0x451
github.com/Knetic/govaluate.EvaluableExpression.evaluateStage(0x784bde, 0x22, 0x1, 0xc0000ce000, 0x14, 0x20, 0xc000050820, 0xc0000142d0, 0x4a, 0xc000050820, ...)
        C:/Users/coolz/go/pkg/mod/github.com/!knetic/govaluate@v3.0.1-0.20171022003610-9aa49832a739+incompatible/EvaluableExpression.go:172 +0x6b4
github.com/Knetic/govaluate.EvaluableExpression.Eval(0x784bde, 0x22, 0x1, 0xc0000ce000, 0x14, 0x20, 0xc000050820, 0xc0000142d0, 0x4a, 0x7eec60, ...)
        C:/Users/coolz/go/pkg/mod/github.com/!knetic/govaluate@v3.0.1-0.20171022003610-9aa49832a739+incompatible/EvaluableExpression.go:163 +0xc0
github.com/casbin/casbin/v2.(*Enforcer).enforce(0xc0000ba000, 0x0, 0x0, 0xc000065650, 0x3, 0x3, 0x44431e, 0x9db720, 0xc00003ff78)
        C:/Users/coolz/go/pkg/mod/github.com/casbin/casbin/v2@v2.1.2/enforcer.go:406 +0xae6
github.com/casbin/casbin/v2.(*Enforcer).Enforce(...)
        C:/Users/coolz/go/pkg/mod/github.com/casbin/casbin/v2@v2.1.2/enforcer.go:495
        D:/project/casbin-demo/main.go:16 +0x173
```

on enforcer.go#L350 `fmt.Errorf("panic: %v", err)` not log

https://github.com/casbin/casbin/blob/53116d0ba1f07f0277b06a471922180977bc062c/enforcer.go#L350





Pull request:
Return errors in builtin function
fixes #384 

Patch:
diff --git a/enforcer_interface.go b/enforcer_interface.go
index e6f3101..38d4191 100644
--- a/enforcer_interface.go
+++ b/enforcer_interface.go
@@ -15,27 +15,23 @@
 package casbin
 
 import (
-	"github.com/Knetic/govaluate"
 	"github.com/casbin/casbin/v2/effect"
 	"github.com/casbin/casbin/v2/model"
 	"github.com/casbin/casbin/v2/persist"
 	"github.com/casbin/casbin/v2/rbac"
 )
 
-var _ IEnforcer = &Enforcer{}
-
-// IEnforcer is the API interface of Enforcer
 type IEnforcer interface {
 	/* Enforcer API */
-	InitWithFile(modelPath string, policyPath string) error
-	InitWithAdapter(modelPath string, adapter persist.Adapter) error
-	InitWithModelAndAdapter(m model.Model, adapter persist.Adapter) error
-	LoadModel() error
+	InitWithFile(modelPath string, policyPath string)
+	InitWithAdapter(modelPath string, adapter persist.Adapter)
+	InitWithModelAndAdapter(m model.Model, adapter persist.Adapter)
+	LoadModel()
 	GetModel() model.Model
 	SetModel(m model.Model)
 	GetAdapter() persist.Adapter
 	SetAdapter(adapter persist.Adapter)
-	SetWatcher(watcher persist.Watcher) error
+	SetWatcher(watcher persist.Watcher)
 	SetRoleManager(rm rbac.RoleManager)
 	SetEffector(eft effect.Effector)
 	ClearPolicy()
@@ -47,27 +43,27 @@ type IEnforcer interface {
 	EnableLog(enable bool)
 	EnableAutoSave(autoSave bool)
 	EnableAutoBuildRoleLinks(autoBuildRoleLinks bool)
-	BuildRoleLinks() error
-	Enforce(rvals ...interface{}) (bool, error)
+	BuildRoleLinks()
+	Enforce(rvals ...interface{}) bool
 
 	/* RBAC API */
 	GetRolesForUser(name string) ([]string, error)
 	GetUsersForRole(name string) ([]string, error)
 	HasRoleForUser(name string, role string) (bool, error)
-	AddRoleForUser(user string, role string) (bool, error)
-	AddPermissionForUser(user string, permission ...string) (bool, error)
-	DeletePermissionForUser(user string, permission ...string) (bool, error)
-	DeletePermissionsForUser(user string) (bool, error)
+	AddRoleForUser(user string, role string) bool
+	AddPermissionForUser(user string, permission ...string) bool
+	DeletePermissionForUser(user string, permission ...string) bool
+	DeletePermissionsForUser(user string) bool
 	GetPermissionsForUser(user string) [][]string
 	HasPermissionForUser(user string, permission ...string) bool
-	GetImplicitRolesForUser(name string, domain ...string) ([]string, error)
-	GetImplicitPermissionsForUser(user string, domain ...string) ([][]string, error)
-	GetImplicitUsersForPermission(permission ...string) ([]string, error)
-	DeleteRoleForUser(user string, role string) (bool, error)
-	DeleteRolesForUser(user string) (bool, error)
-	DeleteUser(user string) (bool, error)
-	DeleteRole(role string) (bool, error)
-	DeletePermission(permission ...string) (bool, error)
+	GetImplicitRolesForUser(name string, domain ...string) []string
+	GetImplicitPermissionsForUser(user string, domain ...string) [][]string
+	GetImplicitUsersForPermission(permission ...string) []string
+	DeleteRoleForUser(user string, role string) bool
+	DeleteRolesForUser(user string) bool
+	DeleteUser(user string) bool
+	DeleteRole(role string)
+	DeletePermission(permission ...string) bool
 
 	/* Management API */
 	GetAllSubjects() []string
@@ -88,19 +84,19 @@ type IEnforcer interface {
 	GetFilteredNamedGroupingPolicy(ptype string, fieldIndex int, fieldValues ...string) [][]string
 	HasPolicy(params ...interface{}) bool
 	HasNamedPolicy(ptype string, params ...interface{}) bool
-	AddPolicy(params ...interface{}) (bool, error)
-	AddNamedPolicy(ptype string, params ...interface{}) (bool, error)
-	RemovePolicy(params ...interface{}) (bool, error)
-	RemoveFilteredPolicy(fieldIndex int, fieldValues ...string) (bool, error)
-	RemoveNamedPolicy(ptype string, params ...interface{}) (bool, error)
-	RemoveFilteredNamedPolicy(ptype string, fieldIndex int, fieldValues ...string) (bool, error)
+	AddPolicy(params ...interface{}) bool
+	AddNamedPolicy(ptype string, params ...interface{}) bool
+	RemovePolicy(params ...interface{}) bool
+	RemoveFilteredPolicy(fieldIndex int, fieldValues ...string) bool
+	RemoveNamedPolicy(ptype string, params ...interface{}) bool
+	RemoveFilteredNamedPolicy(ptype string, fieldIndex int, fieldValues ...string) bool
 	HasGroupingPolicy(params ...interface{}) bool
 	HasNamedGroupingPolicy(ptype string, params ...interface{}) bool
-	AddGroupingPolicy(params ...interface{}) (bool, error)
-	AddNamedGroupingPolicy(ptype string, params ...interface{}) (bool, error)
-	RemoveGroupingPolicy(params ...interface{}) (bool, error)
-	RemoveFilteredGroupingPolicy(fieldIndex int, fieldValues ...string) (bool, error)
-	RemoveNamedGroupingPolicy(ptype string, params ...interface{}) (bool, error)
-	RemoveFilteredNamedGroupingPolicy(ptype string, fieldIndex int, fieldValues ...string) (bool, error)
-	AddFunction(name string, function govaluate.ExpressionFunction)
+	AddGroupingPolicy(params ...interface{}) bool
+	AddNamedGroupingPolicy(ptype string, params ...interface{}) bool
+	RemoveGroupingPolicy(params ...interface{}) bool
+	RemoveFilteredGroupingPolicy(fieldIndex int, fieldValues ...string) bool
+	RemoveNamedGroupingPolicy(ptype string, params ...interface{}) bool
+	RemoveFilteredNamedGroupingPolicy(ptype string, fieldIndex int, fieldValues ...string) bool
+	AddFunction(name string, function func(args ...interface{}) (interface{}, error))
 }
diff --git a/enforcer_synced.go b/enforcer_synced.go
index a7b50d9..2c694de 100644
--- a/enforcer_synced.go
+++ b/enforcer_synced.go
@@ -102,13 +102,6 @@ func (e *SyncedEnforcer) LoadPolicy() error {
 	return e.Enforcer.LoadPolicy()
 }
 
-// LoadFilteredPolicy reloads a filtered policy from file/database.
-func (e *SyncedEnforcer) LoadFilteredPolicy(filter interface{}) error {
-	e.m.Lock()
-	defer e.m.Unlock()
-	return e.Enforcer.LoadFilteredPolicy(filter)
-}
-
 // SavePolicy saves the current policy (usually after changed with Casbin API) back to file/database.
 func (e *SyncedEnforcer) SavePolicy() error {
 	e.m.RLock()
diff --git a/util/builtin_operators.go b/util/builtin_operators.go
index bc4356d..c13aa36 100644
--- a/util/builtin_operators.go
+++ b/util/builtin_operators.go
@@ -16,6 +16,7 @@ package util
 
 import (
 	"errors"
+	"fmt"
 	"net"
 	"regexp"
 	"strings"
@@ -24,6 +25,22 @@ import (
 	"github.com/casbin/casbin/v2/rbac"
 )
 
+// validate the variadic parameter size and type as string
+func validateVariadicArgs(expectedLen int, args ...interface{}) error {
+	if len(args) != expectedLen {
+		return errors.New(fmt.Sprintf("Expected %d arguments, but got %d", expectedLen, len(args)))
+	}
+
+	for _, p := range args {
+		_, ok := p.(string)
+		if !ok {
+			return errors.New("Argument must be a string")
+		}
+	}
+
+	return nil
+}
+
 // KeyMatch determines whether key1 matches the pattern of key2 (similar to RESTful path), key2 can contain a *.
 // For example, "/foo/bar" matches "/foo/*"
 func KeyMatch(key1 string, key2 string) bool {
@@ -40,6 +57,10 @@ func KeyMatch(key1 string, key2 string) bool {
 
 // KeyMatchFunc is the wrapper for KeyMatch.
 func KeyMatchFunc(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "keyMatch", err))
+	}
+
 	name1 := args[0].(string)
 	name2 := args[1].(string)
 
@@ -65,6 +86,10 @@ func KeyMatch2(key1 string, key2 string) bool {
 
 // KeyMatch2Func is the wrapper for KeyMatch2.
 func KeyMatch2Func(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "keyMatch2", err))
+	}
+
 	name1 := args[0].(string)
 	name2 := args[1].(string)
 
@@ -90,6 +115,10 @@ func KeyMatch3(key1 string, key2 string) bool {
 
 // KeyMatch3Func is the wrapper for KeyMatch3.
 func KeyMatch3Func(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "keyMatch3", err))
+	}
+
 	name1 := args[0].(string)
 	name2 := args[1].(string)
 
@@ -158,6 +187,10 @@ func KeyMatch4(key1 string, key2 string) bool {
 
 // KeyMatch4Func is the wrapper for KeyMatch4.
 func KeyMatch4Func(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "keyMatch4", err))
+	}
+
 	name1 := args[0].(string)
 	name2 := args[1].(string)
 
@@ -175,6 +208,10 @@ func RegexMatch(key1 string, key2 string) bool {
 
 // RegexMatchFunc is the wrapper for RegexMatch.
 func RegexMatchFunc(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "regexMatch", err))
+	}
+
 	name1 := args[0].(string)
 	name2 := args[1].(string)
 
@@ -204,6 +241,10 @@ func IPMatch(ip1 string, ip2 string) bool {
 
 // IPMatchFunc is the wrapper for IPMatch.
 func IPMatchFunc(args ...interface{}) (interface{}, error) {
+	if err := validateVariadicArgs(2, args...); err != nil {
+		return false, errors.New(fmt.Sprintf("%s: %s", "ipMatch", err))
+	}
+
 	ip1 := args[0].(string)
 	ip2 := args[1].(string)
 


Test patch:
diff --git a/util/builtin_operators_test.go b/util/builtin_operators_test.go
index fb4358d..1ba838f 100644
--- a/util/builtin_operators_test.go
+++ b/util/builtin_operators_test.go
@@ -179,3 +179,163 @@ func TestIPMatch(t *testing.T) {
 	testIPMatch(t, "10.0.0.11", "10.0.0.0/8", true)
 	testIPMatch(t, "11.0.0.123", "10.0.0.0/8", false)
 }
+
+func testRegexMatchFunc(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := RegexMatchFunc(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func testKeyMatchFunc(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := KeyMatchFunc(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func testKeyMatch2Func(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := KeyMatch2Func(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func testKeyMatch3Func(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := KeyMatch3Func(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func testKeyMatch4Func(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := KeyMatch4Func(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func testIPMatchFunc(t *testing.T, res bool, err string, args ...interface{}) {
+	t.Helper()
+	myRes, myErr := IPMatchFunc(args...)
+	myErrStr := ""
+
+	if myErr != nil {
+		myErrStr = myErr.Error()
+	}
+
+	if myRes != res || err != myErrStr {
+		t.Errorf("%v returns %v %v, supposed to be %v %v", args, myRes, myErr, res, err)
+	}
+}
+
+func TestRegexMatchFunc(t *testing.T) {
+	testRegexMatchFunc(t, false, "regexMatch: Expected 2 arguments, but got 1", "/topic/create")
+	testRegexMatchFunc(t, false, "regexMatch: Expected 2 arguments, but got 3", "/topic/create/123", "/topic/create", "/topic/update")
+	testRegexMatchFunc(t, false, "regexMatch: Argument must be a string", "/topic/create", false)
+	testRegexMatchFunc(t, true, "", "/topic/create/123", "/topic/create")
+}
+
+func TestKeyMatchFunc(t *testing.T) {
+	testKeyMatchFunc(t, false, "keyMatch: Expected 2 arguments, but got 1", "/foo")
+	testKeyMatchFunc(t, false, "keyMatch: Expected 2 arguments, but got 3", "/foo/create/123", "/foo/*", "/foo/update/123")
+	testKeyMatchFunc(t, false, "keyMatch: Argument must be a string", "/foo", true)
+	testKeyMatchFunc(t, false, "", "/foo/bar", "/foo")
+	testKeyMatchFunc(t, true, "", "/foo/bar", "/foo/*")
+	testKeyMatchFunc(t, true, "", "/foo/bar", "/foo*")
+}
+
+func TestKeyMatch2Func(t *testing.T) {
+	testKeyMatch2Func(t, false, "keyMatch2: Expected 2 arguments, but got 1", "/")
+	testKeyMatch2Func(t, false, "keyMatch2: Expected 2 arguments, but got 3", "/foo/create/123", "/*", "/foo/update/123")
+	testKeyMatch2Func(t, false, "keyMatch2: Argument must be a string", "/foo", true)
+
+	testKeyMatch2Func(t, false, "", "/", "/:resource")
+	testKeyMatch2Func(t, true, "", "/resource1", "/:resource")
+
+	testKeyMatch2Func(t, true, "", "/foo", "/foo")
+	testKeyMatch2Func(t, true, "", "/foo", "/foo*")
+	testKeyMatch2Func(t, false, "", "/foo", "/foo/*")
+}
+
+func TestKeyMatch3Func(t *testing.T) {
+	testKeyMatch3Func(t, false, "keyMatch3: Expected 2 arguments, but got 1", "/")
+	testKeyMatch3Func(t, false, "keyMatch3: Expected 2 arguments, but got 3", "/foo/create/123", "/*", "/foo/update/123")
+	testKeyMatch3Func(t, false, "keyMatch3: Argument must be a string", "/foo", true)
+
+	testKeyMatch3Func(t, true, "", "/foo", "/foo")
+	testKeyMatch3Func(t, true, "", "/foo", "/foo*")
+	testKeyMatch3Func(t, false, "", "/foo", "/foo/*")
+	testKeyMatch3Func(t, false, "", "/foo/bar", "/foo")
+	testKeyMatch3Func(t, false, "", "/foo/bar", "/foo*")
+	testKeyMatch3Func(t, true, "", "/foo/bar", "/foo/*")
+	testKeyMatch3Func(t, false, "", "/foobar", "/foo")
+	testKeyMatch3Func(t, false, "", "/foobar", "/foo*")
+	testKeyMatch3Func(t, false, "", "/foobar", "/foo/*")
+
+	testKeyMatch3Func(t, false, "", "/", "/{resource}")
+	testKeyMatch3Func(t, true, "", "/resource1", "/{resource}")
+	testKeyMatch3Func(t, false, "", "/myid", "/{id}/using/{resId}")
+	testKeyMatch3Func(t, true, "", "/myid/using/myresid", "/{id}/using/{resId}")
+
+	testKeyMatch3Func(t, false, "", "/proxy/myid", "/proxy/{id}/*")
+	testKeyMatch3Func(t, true, "", "/proxy/myid/", "/proxy/{id}/*")
+	testKeyMatch3Func(t, true, "", "/proxy/myid/res", "/proxy/{id}/*")
+	testKeyMatch3Func(t, true, "", "/proxy/myid/res/res2", "/proxy/{id}/*")
+	testKeyMatch3Func(t, true, "", "/proxy/myid/res/res2/res3", "/proxy/{id}/*")
+	testKeyMatch3Func(t, false, "", "/proxy/", "/proxy/{id}/*")
+}
+
+func TestKeyMatch4Func(t *testing.T) {
+	testKeyMatch4Func(t, false, "keyMatch4: Expected 2 arguments, but got 1", "/parent/123/child/123")
+	testKeyMatch4Func(t, false, "keyMatch4: Expected 2 arguments, but got 3", "/parent/123/child/123", "/parent/{id}/child/{id}", true)
+	testKeyMatch4Func(t, false, "keyMatch4: Argument must be a string", "/parent/123/child/123", true)
+
+	testKeyMatch4Func(t, true, "", "/parent/123/child/123", "/parent/{id}/child/{id}")
+	testKeyMatch4Func(t, false, "", "/parent/123/child/456", "/parent/{id}/child/{id}")
+
+	testKeyMatch4Func(t, true, "", "/parent/123/child/123", "/parent/{id}/child/{another_id}")
+	testKeyMatch4Func(t, true, "", "/parent/123/child/456", "/parent/{id}/child/{another_id}")
+
+}
+
+func TestIPMatchFunc(t *testing.T) {
+	testIPMatchFunc(t, false, "ipMatch: Expected 2 arguments, but got 1", "192.168.2.123")
+	testIPMatchFunc(t, false, "ipMatch: Argument must be a string", "192.168.2.123", 128)
+	testIPMatchFunc(t, true, "", "192.168.2.123", "192.168.2.0/24")
+}