Skip to content

Commit 8222050

Browse files
cdisselkoenshaobo-he-aws
cdisselkoen
authored and
shaobo-he-aws
committed
improve error when null occurs in entity json data (#751)
Signed-off-by: Craig Disselkoen <[email protected]>
1 parent 5b7d4b6 commit 8222050

File tree

5 files changed

+254
-0
lines changed

5 files changed

+254
-0
lines changed

‎cedar-policy-core/src/entities.rs‎

Lines changed: 239 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1341,6 +1341,245 @@ mod json_parsing_tests {
13411341
});
13421342
}
13431343

1344+
/// Test that `null` is properly rejected, with a sane error message, in
1345+
/// various positions
1346+
#[test]
1347+
fn null_failures() {
1348+
let eparser: EntityJsonParser<'_, '_> =
1349+
EntityJsonParser::new(None, Extensions::all_available(), TCComputation::ComputeNow);
1350+
1351+
let json = serde_json::json!(
1352+
[
1353+
{
1354+
"uid": null,
1355+
"attrs": {},
1356+
"parents": [],
1357+
}
1358+
]
1359+
);
1360+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1361+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1362+
"in uid field of <unknown entity>, expected a literal entity reference, but got `null`",
1363+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1364+
));
1365+
});
1366+
1367+
let json = serde_json::json!(
1368+
[
1369+
{
1370+
"uid": { "type": null, "id": "bar" },
1371+
"attrs": {},
1372+
"parents": [],
1373+
}
1374+
]
1375+
);
1376+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1377+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1378+
r#"in uid field of <unknown entity>, expected a literal entity reference, but got `{"id":"bar","type":null}`"#,
1379+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1380+
));
1381+
});
1382+
1383+
let json = serde_json::json!(
1384+
[
1385+
{
1386+
"uid": { "type": "foo", "id": null },
1387+
"attrs": {},
1388+
"parents": [],
1389+
}
1390+
]
1391+
);
1392+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1393+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1394+
r#"in uid field of <unknown entity>, expected a literal entity reference, but got `{"id":null,"type":"foo"}`"#,
1395+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1396+
));
1397+
});
1398+
1399+
let json = serde_json::json!(
1400+
[
1401+
{
1402+
"uid": { "type": "foo", "id": "bar" },
1403+
"attrs": null,
1404+
"parents": [],
1405+
}
1406+
]
1407+
);
1408+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1409+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1410+
"invalid type: null, expected a map"
1411+
));
1412+
});
1413+
1414+
let json = serde_json::json!(
1415+
[
1416+
{
1417+
"uid": { "type": "foo", "id": "bar" },
1418+
"attrs": { "attr": null },
1419+
"parents": [],
1420+
}
1421+
]
1422+
);
1423+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1424+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1425+
r#"in attribute `attr` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1426+
));
1427+
});
1428+
1429+
let json = serde_json::json!(
1430+
[
1431+
{
1432+
"uid": { "type": "foo", "id": "bar" },
1433+
"attrs": { "attr": { "subattr": null } },
1434+
"parents": [],
1435+
}
1436+
]
1437+
);
1438+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1439+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1440+
r#"in attribute `attr` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1441+
));
1442+
});
1443+
1444+
let json = serde_json::json!(
1445+
[
1446+
{
1447+
"uid": { "type": "foo", "id": "bar" },
1448+
"attrs": { "attr": [ 3, null ] },
1449+
"parents": [],
1450+
}
1451+
]
1452+
);
1453+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1454+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1455+
r#"in attribute `attr` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1456+
));
1457+
});
1458+
1459+
let json = serde_json::json!(
1460+
[
1461+
{
1462+
"uid": { "type": "foo", "id": "bar" },
1463+
"attrs": { "attr": [ 3, { "subattr" : null } ] },
1464+
"parents": [],
1465+
}
1466+
]
1467+
);
1468+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1469+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1470+
r#"in attribute `attr` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1471+
));
1472+
});
1473+
1474+
let json = serde_json::json!(
1475+
[
1476+
{
1477+
"uid": { "type": "foo", "id": "bar" },
1478+
"attrs": { "__extn": { "fn": null, "args": [] } },
1479+
"parents": [],
1480+
}
1481+
]
1482+
);
1483+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1484+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1485+
r#"in attribute `__extn` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1486+
));
1487+
});
1488+
1489+
let json = serde_json::json!(
1490+
[
1491+
{
1492+
"uid": { "type": "foo", "id": "bar" },
1493+
"attrs": { "__extn": { "fn": "ip", "args": null } },
1494+
"parents": [],
1495+
}
1496+
]
1497+
);
1498+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1499+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1500+
r#"in attribute `__extn` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1501+
));
1502+
});
1503+
1504+
let json = serde_json::json!(
1505+
[
1506+
{
1507+
"uid": { "type": "foo", "id": "bar" },
1508+
"attrs": { "__extn": { "fn": "ip", "args": [ null ] } },
1509+
"parents": [],
1510+
}
1511+
]
1512+
);
1513+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1514+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1515+
r#"in attribute `__extn` on `foo::"bar"`, found a `null`; JSON `null`s are not allowed in Cedar"#,
1516+
));
1517+
});
1518+
1519+
let json = serde_json::json!(
1520+
[
1521+
{
1522+
"uid": { "type": "foo", "id": "bar" },
1523+
"attrs": { "attr": 2 },
1524+
"parents": null,
1525+
}
1526+
]
1527+
);
1528+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1529+
expect_err(&json, &e, &ExpectedErrorMessage::error(
1530+
"invalid type: null, expected a sequence"
1531+
));
1532+
});
1533+
1534+
let json = serde_json::json!(
1535+
[
1536+
{
1537+
"uid": { "type": "foo", "id": "bar" },
1538+
"attrs": { "attr": 2 },
1539+
"parents": [ null ],
1540+
}
1541+
]
1542+
);
1543+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1544+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1545+
r#"in parents field of `foo::"bar"`, expected a literal entity reference, but got `null`"#,
1546+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1547+
));
1548+
});
1549+
1550+
let json = serde_json::json!(
1551+
[
1552+
{
1553+
"uid": { "type": "foo", "id": "bar" },
1554+
"attrs": { "attr": 2 },
1555+
"parents": [ { "type": "foo", "id": null } ],
1556+
}
1557+
]
1558+
);
1559+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1560+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1561+
r#"in parents field of `foo::"bar"`, expected a literal entity reference, but got `{"id":null,"type":"foo"}`"#,
1562+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1563+
));
1564+
});
1565+
1566+
let json = serde_json::json!(
1567+
[
1568+
{
1569+
"uid": { "type": "foo", "id": "bar" },
1570+
"attrs": { "attr": 2 },
1571+
"parents": [ { "type": "foo", "id": "parent" }, null ],
1572+
}
1573+
]
1574+
);
1575+
assert_matches!(eparser.from_json_value(json.clone()), Err(EntitiesError::Deserialization(e)) => {
1576+
expect_err(&json, &e, &ExpectedErrorMessage::error_and_help(
1577+
r#"in parents field of `foo::"bar"`, expected a literal entity reference, but got `null`"#,
1578+
r#"literal entity references can be made with `{ "type": "SomeType", "id": "SomeId" }`"#,
1579+
));
1580+
});
1581+
}
1582+
13441583
/// helper function to round-trip an Entities (with no schema-based parsing)
13451584
fn roundtrip(entities: &Entities) -> Result<Entities> {
13461585
let mut buf = Vec::new();

‎cedar-policy-core/src/entities/json/err.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,9 @@ pub enum JsonDeserializationError {
233233
#[error("{0}, the `__expr` escape is no longer supported")]
234234
#[diagnostic(help("to create an entity reference, use `__entity`; to create an extension value, use `__extn`; and for all other values, use JSON directly"))]
235235
ExprTag(Box<JsonDeserializationErrorContext>),
236+
/// Raised when the input JSON contains a `null`
237+
#[error("{0}, found a `null`; JSON `null`s are not allowed in Cedar")]
238+
Null(Box<JsonDeserializationErrorContext>),
236239
}
237240

238241
/// Errors thrown during serialization to JSON

‎cedar-policy-core/src/entities/json/value.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ pub enum CedarValueJson {
8686
/// JSON object => Cedar record; must have string keys, but values
8787
/// can be any `CedarValueJson`s, even heterogeneously
8888
Record(JsonRecord),
89+
/// JSON null, which is never valid, but we put this here in order to
90+
/// provide a better error message.
91+
Null,
8992
}
9093

9194
/// Structure representing a Cedar record in JSON
@@ -243,6 +246,7 @@ impl CedarValueJson {
243246
)),
244247
Self::ExtnEscape { __extn: extn } => extn.into_expr(ctx),
245248
Self::ExprEscape { .. } => Err(JsonDeserializationError::ExprTag(Box::new(ctx()))),
249+
Self::Null => Err(JsonDeserializationError::Null(Box::new(ctx()))),
246250
}
247251
}
248252

‎cedar-policy-core/src/est/expr.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,6 +1400,10 @@ fn display_cedarvaluejson(f: &mut std::fmt::Formatter<'_>, v: &CedarValueJson) -
14001400
write!(f, "}}")?;
14011401
Ok(())
14021402
}
1403+
CedarValueJson::Null => {
1404+
write!(f, "null")?;
1405+
Ok(())
1406+
}
14031407
}
14041408
}
14051409

‎cedar-policy-validator/src/schema/namespace_def.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,10 @@ impl ValidatorNamespaceDef {
371371
"extension function escape (`__extn`)".to_owned(),
372372
))
373373
}
374+
CedarValueJson::Null => Err(SchemaError::UnsupportedActionAttribute(
375+
action_id.clone(),
376+
"null".to_owned(),
377+
)),
374378
}
375379
}
376380

0 commit comments

Comments
 (0)