Skip to content

Commit dabc07b

Browse files
amzn-mdamineamzn-mdamine
committed
fix 437, properly handle unknowns in the context.
Signed-off-by: Mohamed Amine Ouali <[email protected]>
1 parent 197763e commit dabc07b

File tree

1 file changed

+196
-7
lines changed

1 file changed

+196
-7
lines changed

‎cedar-policy-validator/src/types.rs‎

Lines changed: 196 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -514,9 +514,6 @@ impl Type {
514514
/// it does typecheck for some permissible substitutions of the unknowns,
515515
/// but does not typecheck for other permissible substitutions), this is
516516
/// reported as an error.
517-
///
518-
/// TODO(#437): Handling of `Unknown`s is not yet complete and doesn't
519-
/// properly behave according to the above description, as of this writing.
520517
pub(crate) fn typecheck_partial_value(
521518
&self,
522519
value: &PartialValue,
@@ -526,7 +523,7 @@ impl Type {
526523
PartialValue::Value(value) => self.typecheck_value(value, extensions),
527524
PartialValue::Residual(expr) => match BorrowedRestrictedExpr::new(expr) {
528525
Ok(rexpr) => self.typecheck_restricted_expr(rexpr, extensions),
529-
Err(_) => Ok(false), // TODO(#437): instead of just reporting typecheck fails for all nontrivial residuals, we should do something more intelligent
526+
Err(_) => Ok(false),
530527
},
531528
}
532529
}
@@ -544,14 +541,14 @@ impl Type {
544541
}
545542

546543
/// Does the given `BorrowedRestrictedExpr` have this validator type?
547-
///
548-
/// TODO(#437): Handling of restricted exprs containing `Unknown`s is not
549-
/// yet complete or correct, as of this writing.
550544
pub(crate) fn typecheck_restricted_expr(
551545
&self,
552546
restricted_expr: BorrowedRestrictedExpr<'_>,
553547
extensions: &Extensions<'_>,
554548
) -> Result<bool, ExtensionFunctionLookupError> {
549+
if restricted_expr.as_unknown().is_some() {
550+
return Ok(true);
551+
}
555552
match self {
556553
Type::Never => Ok(false), // no expr has type Never
557554
Type::Primitive {
@@ -2520,6 +2517,198 @@ mod test {
25202517
.expect("Expected valid schema")
25212518
}
25222519

2520+
#[test]
2521+
#[cfg(feature = "partial-eval")]
2522+
fn test_typecheck_partial_value_with_unknown() {
2523+
use cedar_policy_core::ast::Context;
2524+
let context = Context::from_json_value(serde_json::json!({
2525+
"a": {
2526+
"__extn": {
2527+
"fn": "unknown",
2528+
"arg": "test_arg"
2529+
}
2530+
},
2531+
}))
2532+
.unwrap();
2533+
2534+
let typeValidator = Type::record_with_attributes(
2535+
[
2536+
(
2537+
"a".into(),
2538+
AttributeType::optional_attribute(Type::primitive_boolean()),
2539+
),
2540+
(
2541+
"b".into(),
2542+
AttributeType::optional_attribute(Type::primitive_boolean()),
2543+
),
2544+
(
2545+
"c".into(),
2546+
AttributeType::optional_attribute(Type::primitive_boolean()),
2547+
),
2548+
],
2549+
OpenTag::ClosedAttributes,
2550+
);
2551+
2552+
let result = typeValidator
2553+
.typecheck_partial_value(&context.clone().into(), Extensions::all_available());
2554+
assert_eq!(result.unwrap(), true);
2555+
}
2556+
2557+
#[test]
2558+
#[cfg(feature = "partial-eval")]
2559+
fn test_typecheck_partial_value_with_embedded_unknown() {
2560+
use cedar_policy_core::ast::Context;
2561+
let context = Context::from_json_value(serde_json::json!({
2562+
"a": {
2563+
"b":{
2564+
"__extn": {
2565+
"fn": "unknown",
2566+
"arg": "test_arg"
2567+
}
2568+
}
2569+
},
2570+
}))
2571+
.unwrap();
2572+
2573+
let embedded_attributes = Type::record_with_attributes(
2574+
[
2575+
(
2576+
"b".into(),
2577+
AttributeType::optional_attribute(Type::primitive_boolean()),
2578+
),
2579+
(
2580+
"c".into(),
2581+
AttributeType::optional_attribute(Type::primitive_boolean()),
2582+
),
2583+
],
2584+
OpenTag::ClosedAttributes,
2585+
);
2586+
2587+
let typeValidator = Type::record_with_attributes(
2588+
[(
2589+
"a".into(),
2590+
AttributeType::optional_attribute(embedded_attributes),
2591+
)],
2592+
OpenTag::ClosedAttributes,
2593+
);
2594+
2595+
let result = typeValidator
2596+
.typecheck_partial_value(&context.clone().into(), Extensions::all_available());
2597+
2598+
// c is optional attribute it is not required to provide it.
2599+
assert_eq!(result.unwrap(), true);
2600+
}
2601+
2602+
#[test]
2603+
#[cfg(feature = "partial-eval")]
2604+
fn test_typecheck_partial_value_with_unknown1() {
2605+
use cedar_policy_core::ast::Context;
2606+
let context = Context::from_json_value(serde_json::json!({
2607+
"foo": 1,
2608+
"bar": {
2609+
"__extn": {
2610+
"fn": "unknown",
2611+
"arg": "test_arg"
2612+
}
2613+
},
2614+
}))
2615+
.unwrap();
2616+
2617+
let typeValidator = Type::record_with_attributes(
2618+
[
2619+
(
2620+
"foo".into(),
2621+
AttributeType::required_attribute(Type::primitive_long()),
2622+
),
2623+
(
2624+
"bar".into(),
2625+
AttributeType::required_attribute(Type::primitive_string()),
2626+
),
2627+
],
2628+
OpenTag::ClosedAttributes,
2629+
);
2630+
2631+
let result = typeValidator
2632+
.typecheck_partial_value(&context.clone().into(), Extensions::all_available());
2633+
//success both foo and bar are provided
2634+
assert_eq!(result.unwrap(), true);
2635+
}
2636+
2637+
#[test]
2638+
#[cfg(feature = "partial-eval")]
2639+
fn test_typecheck_partial_value_with_unknown2() {
2640+
use cedar_policy_core::ast::Context;
2641+
let context = Context::from_json_value(serde_json::json!({
2642+
"bar": {
2643+
"__extn": {
2644+
"fn": "unknown",
2645+
"arg": "test_arg"
2646+
}
2647+
},
2648+
"foo": 1,
2649+
}))
2650+
.unwrap();
2651+
2652+
let typeValidator = Type::record_with_attributes(
2653+
[
2654+
(
2655+
"foo".into(),
2656+
AttributeType::required_attribute(Type::primitive_string()),
2657+
),
2658+
(
2659+
"bar".into(),
2660+
AttributeType::required_attribute(Type::primitive_string()),
2661+
),
2662+
],
2663+
OpenTag::ClosedAttributes,
2664+
);
2665+
2666+
let result = typeValidator
2667+
.typecheck_partial_value(&context.clone().into(), Extensions::all_available());
2668+
//fail foo is string in schema not long
2669+
assert_eq!(result.unwrap(), false);
2670+
}
2671+
2672+
#[test]
2673+
#[cfg(feature = "partial-eval")]
2674+
fn test_typecheck_partial_value_with_unknown3() {
2675+
use cedar_policy_core::ast::Context;
2676+
let context = Context::from_json_value(serde_json::json!({
2677+
"bar": {
2678+
"__extn": {
2679+
"fn": "unknown",
2680+
"arg": "test_arg"
2681+
}
2682+
},
2683+
"foo": 1,
2684+
}))
2685+
.unwrap();
2686+
2687+
let typeValidator = Type::record_with_attributes(
2688+
[
2689+
(
2690+
"foo".into(),
2691+
AttributeType::required_attribute(Type::primitive_long()),
2692+
),
2693+
(
2694+
"bar".into(),
2695+
AttributeType::required_attribute(Type::primitive_string()),
2696+
),
2697+
(
2698+
"baz".into(),
2699+
AttributeType::required_attribute(Type::primitive_string()),
2700+
),
2701+
],
2702+
OpenTag::ClosedAttributes,
2703+
);
2704+
2705+
let result = typeValidator
2706+
.typecheck_partial_value(&context.clone().into(), Extensions::all_available());
2707+
2708+
//fail baz is required in the schema
2709+
assert_eq!(result.unwrap(), false);
2710+
}
2711+
25232712
/// Test cases with entity type Action are interesting because Action
25242713
/// does not need to be declared in the entity type list.
25252714
#[test]

0 commit comments

Comments
 (0)