Skip to content

Commit edbcccf

Browse files
geekphilosophyChris Simmons
authored and
muditchaudhary
committed
Improve toCedarExpr() correctness and add CedarStrings utility (cedar-policy#351)
Signed-off-by: Chris Simmons <simmonsc@amazon.com> Co-authored-by: Chris Simmons <simmonsc@amazon.com>
1 parent 668964a commit edbcccf

6 files changed

Lines changed: 263 additions & 3 deletions

File tree

‎CedarJava/src/main/java/com/cedarpolicy/serializer/ValueCedarSerializer.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,13 @@ public void serialize(
6868
} else if (value instanceof CedarMap) {
6969
jsonGenerator.writeStartObject();
7070
for (Map.Entry<String, Value> entry : ((CedarMap) value).entrySet()) {
71-
jsonGenerator.writeObjectField(entry.getKey(), entry.getValue());
71+
String key = entry.getKey();
72+
if (ENTITY_ESCAPE_SEQ.equals(key) || EXTENSION_ESCAPE_SEQ.equals(key)) {
73+
throw new InvalidValueSerializationException(
74+
"CedarMap key \"" + key + "\" is reserved by the Cedar JSON protocol"
75+
+ " and cannot be used as a record key.");
76+
}
77+
jsonGenerator.writeObjectField(key, entry.getValue());
7278
}
7379
jsonGenerator.writeEndObject();
7480
} else if (value instanceof IpAddress) {

‎CedarJava/src/main/java/com/cedarpolicy/value/CedarMap.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525

2626
/** Represents a Cedar Map value. Maps support mapping strings to arbitrary values. */
2727
public final class CedarMap extends Value implements Map<String, Value> {
28+
/** Reserved keys in the Cedar JSON protocol that cannot be used as record keys. */
29+
private static final String ENTITY_ESCAPE_SEQ = "__entity";
30+
private static final String EXTENSION_ESCAPE_SEQ = "__extn";
31+
2832
/** Internal map data. */
2933
private final java.util.Map<String, Value> map;
3034

@@ -34,6 +38,13 @@ public final class CedarMap extends Value implements Map<String, Value> {
3438
* @param source map to copy from
3539
*/
3640
public CedarMap(java.util.Map<String, Value> source) {
41+
for (String key : source.keySet()) {
42+
if (ENTITY_ESCAPE_SEQ.equals(key) || EXTENSION_ESCAPE_SEQ.equals(key)) {
43+
throw new IllegalArgumentException(
44+
"Key \"" + key + "\" is reserved by the Cedar JSON protocol"
45+
+ " and cannot be used as a record key.");
46+
}
47+
}
3748
this.map = new HashMap<>(source);
3849
}
3950

@@ -66,7 +77,7 @@ public int hashCode() {
6677
public String toCedarExpr() {
6778
return "{"
6879
+ map.entrySet().stream()
69-
.map(e -> '\"' + e.getKey() + "\": " + e.getValue().toCedarExpr())
80+
.map(e -> '\"' + CedarStrings.escape(e.getKey()) + "\": " + e.getValue().toCedarExpr())
7081
.collect(Collectors.joining(", "))
7182
+ "}";
7283
}
@@ -118,12 +129,24 @@ public Value put(String k, Value v) throws NullPointerException {
118129
if (v == null) {
119130
throw new NullPointerException("Attempt to put null value in CedarMap");
120131
}
132+
if (ENTITY_ESCAPE_SEQ.equals(k) || EXTENSION_ESCAPE_SEQ.equals(k)) {
133+
throw new IllegalArgumentException(
134+
"Key \"" + k + "\" is reserved by the Cedar JSON protocol"
135+
+ " and cannot be used as a record key.");
136+
}
121137

122138
return map.put(k, v);
123139
}
124140

125141
@Override
126142
public void putAll(Map<? extends String, ? extends Value> m) {
143+
for (String key : m.keySet()) {
144+
if (ENTITY_ESCAPE_SEQ.equals(key) || EXTENSION_ESCAPE_SEQ.equals(key)) {
145+
throw new IllegalArgumentException(
146+
"Key \"" + key + "\" is reserved by the Cedar JSON protocol"
147+
+ " and cannot be used as a record key.");
148+
}
149+
}
127150
map.putAll(m);
128151
}
129152

‎CedarJava/src/main/java/com/cedarpolicy/value/CedarStrings.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright Cedar Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.cedarpolicy.value;
18+
19+
/**
20+
* Utility methods for working with Cedar string literals.
21+
*/
22+
public final class CedarStrings {
23+
24+
private CedarStrings() { }
25+
26+
/**
27+
* Escape a string for safe inclusion in Cedar source code as a string literal.
28+
* Handles backslashes, double quotes, and control characters recognized by Cedar.
29+
*
30+
* @param s the raw string value
31+
* @return the escaped string (without surrounding quotes)
32+
*/
33+
public static String escape(String s) {
34+
StringBuilder sb = new StringBuilder(s.length());
35+
for (int i = 0; i < s.length(); i++) {
36+
char c = s.charAt(i);
37+
switch (c) {
38+
case '\\':
39+
sb.append("\\\\");
40+
break;
41+
case '"':
42+
sb.append("\\\"");
43+
break;
44+
case '\n':
45+
sb.append("\\n");
46+
break;
47+
case '\r':
48+
sb.append("\\r");
49+
break;
50+
case '\t':
51+
sb.append("\\t");
52+
break;
53+
case '\0':
54+
sb.append("\\0");
55+
break;
56+
default:
57+
sb.append(c);
58+
break;
59+
}
60+
}
61+
return sb.toString();
62+
}
63+
}

‎CedarJava/src/main/java/com/cedarpolicy/value/PrimString.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,6 @@ public String toString() {
6060
/** To Cedar expr that can be used in a Cedar policy. */
6161
@Override
6262
public String toCedarExpr() {
63-
return "\"" + value + "\"";
63+
return "\"" + CedarStrings.escape(value) + "\"";
6464
}
6565
}

‎CedarJava/src/test/java/com/cedarpolicy/CedarExprEscapingTests.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright Cedar Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.cedarpolicy;
18+
19+
import static org.junit.jupiter.api.Assertions.*;
20+
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import com.cedarpolicy.value.CedarMap;
27+
import com.cedarpolicy.value.PrimString;
28+
import com.cedarpolicy.value.Value;
29+
30+
public class CedarExprEscapingTests {
31+
32+
@Test
33+
public void testPrimStringEscapesDoubleQuotes() {
34+
PrimString s = new PrimString("x\" && false && \"x");
35+
assertEquals("\"x\\\" && false && \\\"x\"", s.toCedarExpr());
36+
}
37+
38+
@Test
39+
public void testPrimStringEscapesBackslash() {
40+
PrimString s = new PrimString("path\\to\\file");
41+
assertEquals("\"path\\\\to\\\\file\"", s.toCedarExpr());
42+
}
43+
44+
@Test
45+
public void testPrimStringEscapesControlCharacters() {
46+
PrimString s = new PrimString("line1\nline2\ttab\r\0");
47+
assertEquals("\"line1\\nline2\\ttab\\r\\0\"", s.toCedarExpr());
48+
}
49+
50+
@Test
51+
public void testPrimStringPlainStringUnchanged() {
52+
PrimString s = new PrimString("hello world");
53+
assertEquals("\"hello world\"", s.toCedarExpr());
54+
}
55+
56+
@Test
57+
public void testPrimStringEmptyString() {
58+
PrimString s = new PrimString("");
59+
assertEquals("\"\"", s.toCedarExpr());
60+
}
61+
62+
@Test
63+
public void testCedarMapEscapesKeys() {
64+
Map<String, Value> source = new HashMap<>();
65+
source.put("key\"injection", new PrimString("value"));
66+
CedarMap map = new CedarMap(source);
67+
String expr = map.toCedarExpr();
68+
assertTrue(expr.contains("key\\\"injection"));
69+
assertFalse(expr.contains("key\"injection"));
70+
}
71+
72+
}

‎CedarJava/src/test/java/com/cedarpolicy/CedarMapReservedKeyTests.java‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright Cedar Contributors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.cedarpolicy;
18+
19+
import static org.junit.jupiter.api.Assertions.*;
20+
21+
import java.util.HashMap;
22+
import java.util.Map;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import com.cedarpolicy.value.CedarMap;
27+
import com.cedarpolicy.value.PrimString;
28+
import com.cedarpolicy.value.Value;
29+
30+
public class CedarMapReservedKeyTests {
31+
32+
@Test
33+
public void testPutRejectsEntityKey() {
34+
CedarMap map = new CedarMap();
35+
assertThrows(IllegalArgumentException.class, () -> {
36+
map.put("__entity", new PrimString("spoofed"));
37+
});
38+
}
39+
40+
@Test
41+
public void testPutRejectsExtnKey() {
42+
CedarMap map = new CedarMap();
43+
assertThrows(IllegalArgumentException.class, () -> {
44+
map.put("__extn", new PrimString("spoofed"));
45+
});
46+
}
47+
48+
@Test
49+
public void testConstructorRejectsEntityKey() {
50+
Map<String, Value> source = new HashMap<>();
51+
source.put("__entity", new PrimString("spoofed"));
52+
assertThrows(IllegalArgumentException.class, () -> {
53+
new CedarMap(source);
54+
});
55+
}
56+
57+
@Test
58+
public void testConstructorRejectsExtnKey() {
59+
Map<String, Value> source = new HashMap<>();
60+
source.put("__extn", new PrimString("spoofed"));
61+
assertThrows(IllegalArgumentException.class, () -> {
62+
new CedarMap(source);
63+
});
64+
}
65+
66+
@Test
67+
public void testPutAllRejectsEntityKey() {
68+
CedarMap map = new CedarMap();
69+
Map<String, Value> source = new HashMap<>();
70+
source.put("safe", new PrimString("ok"));
71+
source.put("__entity", new PrimString("spoofed"));
72+
assertThrows(IllegalArgumentException.class, () -> {
73+
map.putAll(source);
74+
});
75+
}
76+
77+
@Test
78+
public void testPutAllRejectsExtnKey() {
79+
CedarMap map = new CedarMap();
80+
Map<String, Value> source = new HashMap<>();
81+
source.put("__extn", new PrimString("spoofed"));
82+
assertThrows(IllegalArgumentException.class, () -> {
83+
map.putAll(source);
84+
});
85+
}
86+
87+
@Test
88+
public void testNormalKeysStillWork() {
89+
CedarMap map = new CedarMap();
90+
assertDoesNotThrow(() -> {
91+
map.put("entity", new PrimString("ok"));
92+
map.put("__other", new PrimString("ok"));
93+
map.put("_entity", new PrimString("ok"));
94+
});
95+
}
96+
}

0 commit comments

Comments
 (0)