Fixes an issue where certain cedar policies couldn't be converted to JSON (#601)

Author: Aaron Eline

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

@@ -1011,25 +1011,35 @@ impl TryFrom<&Node<Option<cst::Unary>>> for Expr {
     type Error = ParseErrors;
     fn try_from(u: &Node<Option<cst::Unary>>) -> Result<Expr, ParseErrors> {
         let u_node = u.ok_or_missing()?;
-        let inner = (&u_node.item).try_into()?;
+
         match u_node.op {
-            Some(cst::NegOp::Bang(0)) => Ok(inner),
-            Some(cst::NegOp::Bang(1)) => Ok(Expr::not(inner)),
-            Some(cst::NegOp::Bang(2)) => {
-                // not safe to collapse !! to nothing
-                Ok(Expr::not(Expr::not(inner)))
-            }
-            Some(cst::NegOp::Bang(n)) => {
-                if n % 2 == 0 {
-                    // safe to collapse to !! but not to nothing
-                    Ok(Expr::not(Expr::not(inner)))
-                } else {
-                    // safe to collapse to !
-                    Ok(Expr::not(inner))
+            Some(cst::NegOp::Bang(num_bangs)) => {
+                let inner = (&u_node.item).try_into()?;
+                match num_bangs {
+                    0 => Ok(inner),
+                    1 => Ok(Expr::not(inner)),
+                    2 => Ok(Expr::not(Expr::not(inner))),
+                    3 => Ok(Expr::not(Expr::not(Expr::not(inner)))),
+                    4 => Ok(Expr::not(Expr::not(Expr::not(Expr::not(inner))))),
+                    _ => Err(u
+                        .to_ast_err(ToASTErrorKind::UnaryOpLimit(ast::UnaryOp::Not))
+                        .into()),
                 }
             }
-            Some(cst::NegOp::Dash(0)) => Ok(inner),
+            Some(cst::NegOp::Dash(0)) => Ok((&u_node.item).try_into()?),
             Some(cst::NegOp::Dash(mut num_dashes)) => {
+                let inner = match &u_node.item.to_lit() {
+                    Some(cst::Literal::Num(num))
+                        if num
+                            .checked_sub(1)
+                            .map(|y| y == InputInteger::MAX as u64)
+                            .unwrap_or(false) =>
+                    {
+                        num_dashes -= 1;
+                        Expr::ExprNoExt(ExprNoExt::Value(CedarValueJson::Long(InputInteger::MIN)))
+                    }
+                    _ => (&u_node.item).try_into()?,
+                };
                 let inner = match inner {
                     Expr::ExprNoExt(ExprNoExt::Value(CedarValueJson::Long(n)))
                         if n != InputInteger::MIN =>
@@ -1048,15 +1058,11 @@ impl TryFrom<&Node<Option<cst::Unary>>> for Expr {
                         // not safe to collapse `--` to nothing
                         Ok(Expr::neg(Expr::neg(inner)))
                     }
-                    n => {
-                        if n % 2 == 0 {
-                            // safe to collapse to `--` but not to nothing
-                            Ok(Expr::neg(Expr::neg(inner)))
-                        } else {
-                            // safe to collapse to -
-                            Ok(Expr::neg(inner))
-                        }
-                    }
+                    3 => Ok(Expr::neg(Expr::neg(Expr::neg(inner)))),
+                    4 => Ok(Expr::neg(Expr::neg(Expr::neg(Expr::neg(inner))))),
+                    _ => Err(u
+                        .to_ast_err(ToASTErrorKind::UnaryOpLimit(ast::UnaryOp::Neg))
+                        .into()),
                 }
             }
             Some(cst::NegOp::OverBang) => Err(u
@@ -1065,7 +1071,7 @@ impl TryFrom<&Node<Option<cst::Unary>>> for Expr {
             Some(cst::NegOp::OverDash) => Err(u
                 .to_ast_err(ToASTErrorKind::UnaryOpLimit(ast::UnaryOp::Neg))
                 .into()),
-            None => Ok(inner),
+            None => Ok((&u_node.item).try_into()?),
         }
     }
 }

cedar-policy-core/src/parser/cst_to_ast.rs

@@ -1574,11 +1574,11 @@ enum AstAccessor {
 }
 
 impl Node<Option<cst::Member>> {
-    // Try to convert `cst::Member` into a `cst::Literal`, i.e.
-    // match `Member(Primary(Literal(_), []))`.
-    // It does not match the `Expr` arm of `Primary`, which means expressions
-    // like `(1)` are not considered as literals on the CST level.
-    fn to_lit(&self) -> Option<&cst::Literal> {
+    /// Try to convert `cst::Member` into a `cst::Literal`, i.e.
+    /// match `Member(Primary(Literal(_), []))`.
+    /// It does not match the `Expr` arm of `Primary`, which means expressions
+    /// like `(1)` are not considered as literals on the CST level.
+    pub fn to_lit(&self) -> Option<&cst::Literal> {
         let m = self.as_ref().node.as_ref()?;
         if !m.access.is_empty() {
             return None;

cedar-policy/CHANGELOG.md

@@ -60,6 +60,7 @@ method checks the request against the schema provided and the
 - The entity type tested for by an `is` expression may be an identifier shared
   with a builtin variable. E.g., `... is principal` and `... is action` are now
   accepted by the Cedar parser. (#558)
+- Policies containing the literal `i64::MIN` can now be properly converted to JSON ESTs (#601, resolving #596)
 
 ## [3.0.1] - 2023-12-21
 Cedar Language Version: 3.0.0

cedar-policy/src/api.rs

@@ -3924,3 +3924,100 @@ mod partial_eval_test {
         assert_eq!(a.residuals(), &p);
     }
 }
+
+#[cfg(test)]
+// PANIC SAFETY: unit tests
+#[allow(clippy::unwrap_used)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_all_ints() {
+        test_single_int(0);
+        test_single_int(i64::MAX);
+        test_single_int(i64::MIN);
+        test_single_int(7);
+        test_single_int(-7);
+    }
+
+    fn test_single_int(x: i64) {
+        for i in 0..4 {
+            test_single_int_with_dashes(x, i);
+        }
+    }
+
+    fn test_single_int_with_dashes(x: i64, num_dashes: usize) {
+        let dashes = vec!['-'; num_dashes].into_iter().collect::<String>();
+        let src = format!(r#"permit(principal, action, resource) when {{ {dashes}{x} }};"#);
+        let p: Policy = src.parse().unwrap();
+        let json = p.to_json().unwrap();
+        let round_trip = Policy::from_json(None, json).unwrap();
+        let pretty_print = format!("{round_trip}");
+        assert!(pretty_print.contains(&x.to_string()));
+        if x != 0 {
+            let expected_dashes = if x < 0 { num_dashes + 1 } else { num_dashes };
+            assert_eq!(
+                pretty_print.chars().filter(|c| *c == '-').count(),
+                expected_dashes
+            );
+        }
+    }
+
+    // Serializing a valid 64-bit int that can't be represented in double precision float
+    #[test]
+    fn json_bignum_1() {
+        let src = r#"
+        permit(
+            principal,
+            action == Action::"action",
+            resource
+          ) when {
+            -9223372036854775808
+          };"#;
+        let p: Policy = src.parse().unwrap();
+        p.to_json().unwrap();
+    }
+
+    #[test]
+    fn json_bignum_1a() {
+        let src = r#"
+        permit(principal, action, resource) when { 
+            (true && (-90071992547409921)) && principal
+        };"#;
+        let p: Policy = src.parse().unwrap();
+        let v = p.to_json().unwrap();
+        let s = serde_json::to_string(&v).unwrap();
+        assert!(s.contains("90071992547409921"));
+    }
+
+    // Deserializing a valid 64-bit int that can't be represented in double precision float
+    #[test]
+    fn json_bignum_2() {
+        let src = r#"{"effect":"permit","principal":{"op":"All"},"action":{"op":"All"},"resource":{"op":"All"},"conditions":[{"kind":"when","body":{"==":{"left":{".":{"left":{"Var":"principal"},"attr":"x"}},"right":{"Value":90071992547409921}}}}]}"#;
+        let v: serde_json::Value = serde_json::from_str(src).unwrap();
+        let p = Policy::from_json(None, v).unwrap();
+        let pretty = format!("{p}");
+        // Ensure the number didn't get rounded
+        assert!(pretty.contains("90071992547409921"));
+    }
+
+    // Deserializing a valid 64-bit int that can't be represented in double precision float
+    #[test]
+    fn json_bignum_2a() {
+        let src = r#"{"effect":"permit","principal":{"op":"All"},"action":{"op":"All"},"resource":{"op":"All"},"conditions":[{"kind":"when","body":{"==":{"left":{".":{"left":{"Var":"principal"},"attr":"x"}},"right":{"Value":-9223372036854775808}}}}]}"#;
+        let v: serde_json::Value = serde_json::from_str(src).unwrap();
+        let p = Policy::from_json(None, v).unwrap();
+        let pretty = format!("{p}");
+        // Ensure the number didn't get rounded
+        assert!(pretty.contains("-9223372036854775808"));
+    }
+
+    // Deserializing a number that doesn't fit in 64 bit integer
+    // This _should_ fail, as there's no way to do this w/out loss of precision
+    #[test]
+    fn json_bignum_3() {
+        let src = r#"{"effect":"permit","principal":{"op":"All"},"action":{"op":"All"},"resource":{"op":"All"},"conditions":[{"kind":"when","body":{"==":{"left":{".":{"left":{"Var":"principal"},"attr":"x"}},"right":{"Value":9223372036854775808}}}}]}"#;
+        let v: serde_json::Value = serde_json::from_str(src).unwrap();
+        assert!(Policy::from_json(None, v).is_err());
+    }
+}