Fixed panic when parsing reserved ids (#675)

cedar-policy · Feb 28, 2024 · 20fce06 · 20fce06
1 parent fb09925
commit 20fce06
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 5 deletions.
diff --git a/cedar-policy-validator/src/human_schema/err.rs b/cedar-policy-validator/src/human_schema/err.rs
@@ -16,6 +16,8 @@ pub enum UserError {
     EmptyList,
     #[error("Invalid escape codes")]
     StringEscape(NonEmpty<UnescapeError>),
+    #[error("`{0}` is a reserved identifier")]
+    ReservedIdentifierUsed(SmolStr),
 }
 
 pub(crate) type RawLocation = usize;
@@ -99,10 +101,7 @@ impl Display for ParseError {
             } => write!(f, "extra token `{token}`"),
             OwnedRawParseError::User {
                 error: Node { node, .. },
-            } => match node {
-                UserError::EmptyList => write!(f, "expected a non-empty list"),
-                UserError::StringEscape(unescape_errs) => write!(f, "{}", unescape_errs.first()),
-            },
+            } => write!(f, "{node}"),
         }
     }
 }

diff --git a/cedar-policy-validator/src/human_schema/grammar.lalrpop b/cedar-policy-validator/src/human_schema/grammar.lalrpop
@@ -199,7 +199,13 @@ Ident: Node<Id> = {
     <l:@L> STRING <r:@R> 
         => Node::with_source_loc("String".parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
     <l:@L> <i:IDENTIFIER> <r:@R>
-        => Node::with_source_loc(i.parse().unwrap(), Loc::new(l..r, Arc::clone(src))),
+        =>? Id::from_str(i)
+        .map(|id : Id| Node::with_source_loc(id, Loc::new(l..r, Arc::clone(src))))
+        .map_err(|err : cedar_policy_core::parser::err::ParseErrors|
+            ParseError::User {
+                error: Node::with_source_loc(UserError::ReservedIdentifierUsed(i.to_smolstr()), Loc::new(l..r, Arc::clone(src)))
+            }
+        )
 }
 
 STR: Node<SmolStr> = {

diff --git a/cedar-policy-validator/src/human_schema/test.rs b/cedar-policy-validator/src/human_schema/test.rs
@@ -1422,4 +1422,54 @@ mod translator_tests {
         let foo = ns.entity_types.get("Foo").unwrap();
         assert_eq!(foo.member_of_types, vec!["String".to_smolstr()]);
     }
+
+    #[test]
+    fn entity_named_if() {
+        let src = r#"
+        entity if = {};
+        entity Foo in [if] = {};
+        "#;
+
+        assert!(SchemaFragment::from_str_natural(src).is_err());
+    }
+
+    #[test]
+    fn entity_named_like() {
+        let src = r#"
+        entity like = {};
+        entity Foo in [like] = {};
+        "#;
+
+        assert!(SchemaFragment::from_str_natural(src).is_err());
+    }
+
+    #[test]
+    fn entity_named_true() {
+        let src = r#"
+        entity true = {};
+        entity Foo in [true] = {};
+        "#;
+
+        assert!(SchemaFragment::from_str_natural(src).is_err());
+    }
+
+    #[test]
+    fn entity_named_false() {
+        let src = r#"
+        entity false = {};
+        entity Foo in [false] = {};
+        "#;
+
+        assert!(SchemaFragment::from_str_natural(src).is_err());
+    }
+
+    #[test]
+    fn entity_named_has() {
+        let src = r#"
+        entity has = {};
+        entity Foo in [has] = {};
+        "#;
+
+        assert!(SchemaFragment::from_str_natural(src).is_err());
+    }
 }