Skip to content

Commit c82e7c5

Browse files
committed
feat: fix the double quotes caused eval() bug
1 parent 8dd4940 commit c82e7c5

File tree

5 files changed

+74
-1
lines changed

5 files changed

+74
-1
lines changed
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
[request_definition]
2+
r = sub, obj, act
3+
4+
[policy_definition]
5+
p = sub_rule, obj_rule, act
6+
7+
[policy_effect]
8+
e = some(where (p.eft == allow))
9+
10+
[matchers]
11+
m = r.act == p.act && eval(p.sub_rule) && eval(p.obj_rule)

src/main/java/org/casbin/jcasbin/main/CoreEnforcer.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@
4242
import java.util.function.BiPredicate;
4343
import java.util.function.Function;
4444

45+
import static org.casbin.jcasbin.util.Util.hasEval;
46+
import static org.casbin.jcasbin.util.Util.splitCommaDelimitedList;
47+
4548
/**
4649
* CoreEnforcer defines the core functionality of an enforcer.
4750
*/
@@ -580,6 +583,7 @@ private EnforceResult enforce(String matcher, Object... rvals) {
580583
} else {
581584
expString = Util.removeComments(Util.escapeAssertion(matcher));
582585
}
586+
boolean hasEval = hasEval(expString);
583587

584588
// json process
585589
if (acceptJsonRequest) {
@@ -629,6 +633,9 @@ private EnforceResult enforce(String matcher, Object... rvals) {
629633

630634
for (int i = 0; i < policy.size(); i++) {
631635
List<String> pvals = policy.get(i);
636+
if (hasEval) {
637+
pvals = splitCommaDelimitedList(pvals);
638+
}
632639
Map<String, Object> parameters = new HashMap<>(rvals.length + pTokens.length);
633640
getPTokens(parameters, pType, pvals, pTokens);
634641
getRTokens(parameters, rType, rvals);

src/main/java/org/casbin/jcasbin/util/Util.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,21 @@ public static String[] splitCommaDelimited(String s) {
284284
return records;
285285
}
286286

287+
/**
288+
* splits each string in the given list by commas according to CSV format
289+
* and removes any extra double quotes
290+
* @param rule the rule to be modified
291+
* @return the modified rule
292+
*/
293+
public static List<String> splitCommaDelimitedList(List<String> rule) {
294+
List<String> modifiedRule = new ArrayList<>();
295+
for (String s : rule) {
296+
String[] strings = splitCommaDelimited(s);
297+
modifiedRule.add(strings[0]);
298+
}
299+
return modifiedRule;
300+
}
301+
287302
/**
288303
* setEquals determines whether two string sets are identical.
289304
*
@@ -314,7 +329,7 @@ public static boolean setEquals(List<String> a, List<String> b) {
314329
}
315330

316331
public static boolean hasEval(String exp) {
317-
return evalReg.matcher(exp).matches();
332+
return evalReg.matcher(exp).find();
318333
}
319334

320335
public static String replaceEval(String s, String replacement) {

src/test/java/org/casbin/jcasbin/main/AbacAPIUnitTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,15 @@
1616

1717
import org.casbin.jcasbin.util.Util;
1818
import org.junit.Test;
19+
20+
import java.util.ArrayList;
21+
import java.util.List;
1922
import java.util.Map;
2023
import java.util.HashMap;
2124

2225
import static org.casbin.jcasbin.main.TestUtil.testDomainEnforce;
2326
import static org.casbin.jcasbin.main.TestUtil.testEnforce;
27+
import static org.junit.Assert.*;
2428

2529
public class AbacAPIUnitTest {
2630
@Test
@@ -57,6 +61,34 @@ public void testEvalWithDomain() {
5761
testDomainEnforce(e, "bob", "domain2", "data2", "read", true);
5862
}
5963

64+
@Test
65+
public void testEvalWithComma() {
66+
Enforcer e = new Enforcer("examples/abac_rule_with_comma_model.conf");
67+
List<String> rule = new ArrayList<>();
68+
rule.add("true");
69+
rule.add("\"let test=seq.set('alice','bob');include(test,r.sub.name)\"");
70+
rule.add("read");
71+
List<String> newRule = new ArrayList<>();
72+
newRule.add("true");
73+
newRule.add("\"let test=seq.set('bob');include(test,r.sub.name)\"");
74+
newRule.add("read");
75+
assertTrue(e.addPolicy(rule));
76+
assertFalse(e.addPolicy(rule));
77+
78+
Map<String, Object> sub = new HashMap<>();
79+
sub.put("name", "alice");
80+
81+
testEnforce(e, sub, "data1", "read", true);
82+
83+
assertTrue(e.updatePolicy("p", "p", rule, newRule));
84+
testEnforce(e, sub, "data1", "read", false);
85+
sub.put("name", "bob");
86+
testEnforce(e, sub, "data1", "read", true);
87+
88+
assertTrue(e.removePolicy(newRule));
89+
testEnforce(e, sub, "data1", "read", false);
90+
}
91+
6092
@Test
6193
public void testABACMapRequest() {
6294
Enforcer e = new Enforcer("examples/abac_rule_map_model.conf");

src/test/java/org/casbin/jcasbin/main/UtilTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.io.IOException;
2828
import java.io.StringReader;
2929

30+
import static org.casbin.jcasbin.util.Util.hasEval;
3031
import static org.junit.Assert.*;
3132
import static org.mockito.ArgumentMatchers.*;
3233

@@ -84,6 +85,13 @@ public void testSplitCommaDelimited(){
8485
assertArrayEquals(new String[]{"a b", "c", "d"}, Util.splitCommaDelimited("\"a b\", c, d"));
8586
}
8687

88+
@Test
89+
public void testHasEval() {
90+
assertTrue(hasEval("eval(test)"));
91+
assertTrue(hasEval("r_act == p_act && eval(p_sub_rule) && eval(p_obj_rule)"));
92+
assertFalse(hasEval("evaltest"));
93+
}
94+
8795
@Test
8896
public void testReplaceEval() {
8997
Util.logPrint(Util.replaceEval("eval(test)", "testEval"));

0 commit comments

Comments
 (0)