From cccd29af3ea4a9d8254ec243c0a5994c6cb7d5d2 Mon Sep 17 00:00:00 2001
From: Eliad Peller <eliad.peller@wiz.io>
Date: Tue, 26 Jul 2022 17:25:49 +0300
Subject: [PATCH] consider missing fields as false

remove get_field_value_unchecked() function, and simply evaluate
missing fields as 'false' instead of panicking.

this allows a safe usage of optional fields.

Signed-off-by: Eliad Peller <eliad.peller@wiz.io>
---
 engine/src/ast/field_expr.rs    |  5 ++++-
 engine/src/ast/index_expr.rs    | 31 +++++++++++++++++--------------
 engine/src/execution_context.rs | 18 ------------------
 3 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/engine/src/ast/field_expr.rs b/engine/src/ast/field_expr.rs
index bdbda0d8..f4ab3307 100644
--- a/engine/src/ast/field_expr.rs
+++ b/engine/src/ast/field_expr.rs
@@ -248,7 +248,10 @@ impl<'s> IdentifierExpr<'s> {
     ) -> CompiledValueExpr<'s, C::U> {
         match self {
             IdentifierExpr::Field(f) => {
-                CompiledValueExpr::new(move |ctx| Ok(ctx.get_field_value_unchecked(f).as_ref()))
+                CompiledValueExpr::new(move |ctx| match ctx.get_field_value(f) {
+                    Some(value) => Ok(value.as_ref()),
+                    None => Err(f.get_type()),
+                })
             }
             IdentifierExpr::FunctionCallExpr(call) => compiler.compile_function_call_expr(call),
         }
diff --git a/engine/src/ast/index_expr.rs b/engine/src/ast/index_expr.rs
index 7a130d68..48404a81 100644
--- a/engine/src/ast/index_expr.rs
+++ b/engine/src/ast/index_expr.rs
@@ -104,11 +104,10 @@ impl<'s> ValueExpr<'s> for IndexExpr<'s> {
             // Average path
             match identifier {
                 IdentifierExpr::Field(f) => CompiledValueExpr::new(move |ctx| {
+                    let init_value = ctx.get_field_value(f).ok_or(ty)?;
                     indexes[..last]
                         .iter()
-                        .try_fold(ctx.get_field_value_unchecked(f), |value, index| {
-                            value.get(index).unwrap()
-                        })
+                        .try_fold(init_value, |value, index| value.get(index).unwrap())
                         .map(LhsValue::as_ref)
                         .ok_or(ty)
                 }),
@@ -130,7 +129,11 @@ impl<'s> ValueExpr<'s> for IndexExpr<'s> {
             match identifier {
                 IdentifierExpr::Field(f) => CompiledValueExpr::new(move |ctx| {
                     let mut iter = MapEachIterator::from_indexes(&indexes[..]);
-                    iter.reset(ctx.get_field_value_unchecked(f).as_ref());
+                    let value = match ctx.get_field_value(f) {
+                        Some(value) => value,
+                        None => return Err(f.get_type()),
+                    };
+                    iter.reset(value.as_ref());
                     Ok(LhsValue::Array(Array::try_from_iter(ty, iter).unwrap()))
                 }),
                 IdentifierExpr::FunctionCallExpr(call) => {
@@ -189,16 +192,13 @@ impl<'s> IndexExpr<'s> {
             }
             IdentifierExpr::Field(f) => {
                 if indexes.is_empty() {
-                    CompiledOneExpr::new(move |ctx| func(ctx.get_field_value_unchecked(f), ctx))
+                    CompiledOneExpr::new(move |ctx| match ctx.get_field_value(f) {
+                        Some(value) => func(value, ctx),
+                        None => false,
+                    })
                 } else {
                     CompiledOneExpr::new(move |ctx| {
-                        index_access_one!(
-                            indexes,
-                            Some(ctx.get_field_value_unchecked(f)),
-                            default,
-                            ctx,
-                            func
-                        )
+                        index_access_one!(indexes, ctx.get_field_value(f), default, ctx, func)
                     })
                 }
             }
@@ -226,7 +226,7 @@ impl<'s> IndexExpr<'s> {
                 })
             }
             IdentifierExpr::Field(f) => CompiledVecExpr::new(move |ctx| {
-                index_access_vec!(indexes, Some(ctx.get_field_value_unchecked(f)), ctx, func)
+                index_access_vec!(indexes, ctx.get_field_value(f), ctx, func)
             }),
         }
     }
@@ -246,7 +246,10 @@ impl<'s> IndexExpr<'s> {
         match identifier {
             IdentifierExpr::Field(f) => CompiledVecExpr::new(move |ctx| {
                 let mut iter = MapEachIterator::from_indexes(&indexes[..]);
-                iter.reset(ctx.get_field_value_unchecked(f).as_ref());
+                let Some(value) = ctx.get_field_value(f) else {
+                    return TypedArray::default();
+                };
+                iter.reset(value.as_ref());
                 TypedArray::from_iter(iter.map(|item| func(&item, ctx)))
             }),
             IdentifierExpr::FunctionCallExpr(call) => {
diff --git a/engine/src/execution_context.rs b/engine/src/execution_context.rs
index 4bec4ae6..8c5a7bb1 100644
--- a/engine/src/execution_context.rs
+++ b/engine/src/execution_context.rs
@@ -99,24 +99,6 @@ impl<'e, U> ExecutionContext<'e, U> {
         }
     }
 
-    #[inline]
-    pub(crate) fn get_field_value_unchecked(&self, field: Field<'_>) -> &LhsValue<'_> {
-        // This is safe because this code is reachable only from Filter::execute
-        // which already performs the scheme compatibility check, but check that
-        // invariant holds in the future at least in the debug mode.
-        debug_assert!(self.scheme() == field.scheme());
-
-        // For now we panic in this, but later we are going to align behaviour
-        // with wireshark: resolve all subexpressions that don't have RHS value
-        // to `false`.
-        self.values[field.index()].as_ref().unwrap_or_else(|| {
-            panic!(
-                "Field {} was registered but not given a value",
-                field.name()
-            );
-        })
-    }
-
     /// Get the value of a field.
     pub fn get_field_value(&self, field: Field<'_>) -> Option<&LhsValue<'_>> {
         assert!(self.scheme() == field.scheme());