From edd0ccca5bb34810fe5992d55092b8c16c1ec925 Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi <35780660+anakrish@users.noreply.github.com> Date: Thu, 29 Aug 2024 09:47:45 -0700 Subject: [PATCH] fix: Issues #302, #303 (#304) Handle undefined values correctly in ordered-else. Previously an undefined value in one of the blocks could cause the entire rule to evaluate to undefined. Handle undefined values correctly in generic rule refs to prevent them from propagating to output. Signed-off-by: Anand Krishnamoorthi --- src/interpreter.rs | 12 ++++++--- tests/interpreter/cases/rule/else.yaml | 33 +++++++++++++++++------ tests/interpreter/cases/rule/generic.yaml | 26 ++++++++++++++++++ 3 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 tests/interpreter/cases/rule/generic.yaml diff --git a/src/interpreter.rs b/src/interpreter.rs index 956612c..9453f9c 100644 --- a/src/interpreter.rs +++ b/src/interpreter.rs @@ -1558,6 +1558,10 @@ impl Interpreter { let mut obj = &mut self.data; let len = path.len(); for (idx, p) in path.into_iter().enumerate() { + // Stop at the first undefined component in the path + if p == Value::Undefined { + break; + } if idx == len - 1 { // last key. if is_set { @@ -1692,6 +1696,7 @@ impl Interpreter { } if output == Value::Undefined || !comps_defined { + ctx.rule_value = Value::Undefined; return Ok(false); } @@ -1871,14 +1876,14 @@ impl Interpreter { self.hoist_loops_impl(oe, &mut loops); } - self.eval_output_expr_in_loop(&loops[..])?; + let r = self.eval_output_expr_in_loop(&loops[..])?; let ctx = self.get_current_context()?; if let Some(_oe) = &ctx.output_expr { // Ensure that at least one output was generated. - Ok(ctx.value != Value::Undefined) + Ok(ctx.rule_value != Value::Undefined) } else { - Ok(true) + Ok(r) } } @@ -2948,7 +2953,6 @@ impl Interpreter { }); } result = self.eval_query(&body.query); - if matches!(&result, Ok(true) | Err(_)) { break; } diff --git a/tests/interpreter/cases/rule/else.yaml b/tests/interpreter/cases/rule/else.yaml index 8d4a7a3..d5aa4f8 100644 --- a/tests/interpreter/cases/rule/else.yaml +++ b/tests/interpreter/cases/rule/else.yaml @@ -2,17 +2,34 @@ # Licensed under the MIT License. cases: - - note: else without body + # - note: else without body + # data: {} + # modules: + # - | + # package test + # x = 4 { + # false + # } else = 5 + + # y = 6 + # query: data.test + # want_result: + # x: 5 + # y: 6 + - note: undefined values being assigned data: {} modules: - | package test - x = 4 { - false - } else = 5 - - y = 6 + + import rego.v1 + + x := data.y if { + true + } else := 2 if { + true + } query: data.test want_result: - x: 5 - y: 6 + x: 2 + diff --git a/tests/interpreter/cases/rule/generic.yaml b/tests/interpreter/cases/rule/generic.yaml new file mode 100644 index 0000000..55514c9 --- /dev/null +++ b/tests/interpreter/cases/rule/generic.yaml @@ -0,0 +1,26 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +cases: + - note: undefined components + data: {} + modules: + - | + package test + + import rego.v1 + + principal := input.principal + action := input.action + + p[principal][action] := 1 if { + some a in [] + } + + q[principal][action] contains 1 if { + some a in [] + } + query: data.test + want_result: + p: {} + q: {}