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 956612c3..9453f9c3 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 8d4a7a32..d5aa4f8e 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 00000000..55514c9c --- /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: {}