From 87a4c6e3c1f036a2da6e0b709341d2e4e252900c Mon Sep 17 00:00:00 2001 From: Aaron Eline Date: Wed, 28 Feb 2024 18:24:49 +0000 Subject: [PATCH 1/2] Fixing 2 bugs with IDs Signed-off-by: Aaron Eline --- .../src/human_schema/grammar.lalrpop | 4 +- .../src/human_schema/test.rs | 155 ++++++++++++++++++ 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/cedar-policy-validator/src/human_schema/grammar.lalrpop b/cedar-policy-validator/src/human_schema/grammar.lalrpop index a5a0363917..9ba1a4f77e 100644 --- a/cedar-policy-validator/src/human_schema/grammar.lalrpop +++ b/cedar-policy-validator/src/human_schema/grammar.lalrpop @@ -178,10 +178,8 @@ Ident: Node = { => Node::with_source_loc("namespace".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), ENTITY => Node::with_source_loc("entity".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), - IN - => Node::with_source_loc("in".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), SET - => Node::with_source_loc("set".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), + => Node::with_source_loc("Set".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), APPLIESTO => Node::with_source_loc("appliesTo".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), PRINCIPAL diff --git a/cedar-policy-validator/src/human_schema/test.rs b/cedar-policy-validator/src/human_schema/test.rs index 2c64ff7098..c70b80404c 100644 --- a/cedar-policy-validator/src/human_schema/test.rs +++ b/cedar-policy-validator/src/human_schema/test.rs @@ -948,6 +948,7 @@ mod parser_tests { #[cfg(test)] mod translator_tests { use cedar_policy_core::FromNormalizedStr; + use smol_str::ToSmolStr; use crate::{SchemaError, SchemaFragment, SchemaTypeVariant, TypeOfAttribute, ValidatorSchema}; @@ -1267,4 +1268,158 @@ mod translator_tests { ) ); } + + #[test] + fn entity_named_namespace() { + let src = r#" + entity namespace = {}; + entity Foo in [namespace] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["namespace".to_smolstr()]); + } + + #[test] + fn entity_named_in() { + // This fails because `in` is reserved + let src = r#" + entity in = {}; + entity Foo in [in] = {}; + "#; + + assert!(SchemaFragment::from_str_natural(src).is_err()); + } + + #[test] + fn entity_named_set() { + let src = r#" + entity Set = {}; + entity Foo in [Set] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["Set".to_smolstr()]); + } + + #[test] + fn entity_named_applies_to() { + let src = r#" + entity appliesTo = {}; + entity Foo in [appliesTo] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["appliesTo".to_smolstr()]); + } + + #[test] + fn entity_named_principal() { + let src = r#" + entity principal = {}; + entity Foo in [principal ] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["principal".to_smolstr()]); + } + + #[test] + fn entity_named_resource() { + let src = r#" + entity resource= {}; + entity Foo in [resource] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["resource".to_smolstr()]); + } + + #[test] + fn entity_named_action() { + let src = r#" + entity action= {}; + entity Foo in [action] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["action".to_smolstr()]); + } + + #[test] + fn entity_named_context() { + let src = r#" + entity context= {}; + entity Foo in [context] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["context".to_smolstr()]); + } + + #[test] + fn entity_named_attributes() { + let src = r#" + entity attributes= {}; + entity Foo in [attributes] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["attributes".to_smolstr()]); + } + + #[test] + fn entity_named_bool() { + let src = r#" + entity Bool= {}; + entity Foo in [Bool] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["Bool".to_smolstr()]); + } + + #[test] + fn entity_named_long() { + let src = r#" + entity Long= {}; + entity Foo in [Long] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["Long".to_smolstr()]); + } + + #[test] + fn entity_named_string() { + let src = r#" + entity String= {}; + entity Foo in [String] = {}; + "#; + + let (schema, _) = SchemaFragment::from_str_natural(src).unwrap(); + let ns = schema.0.get("").unwrap(); + let foo = ns.entity_types.get("Foo").unwrap(); + assert_eq!(foo.member_of_types, vec!["String".to_smolstr()]); + } } From f3859038012d7e8ecfd7f583765330ece74fff01 Mon Sep 17 00:00:00 2001 From: Aaron Eline Date: Wed, 28 Feb 2024 19:20:10 +0000 Subject: [PATCH 2/2] Fixing another bug with ids Signed-off-by: Aaron Eline --- .../src/human_schema/err.rs | 7 ++- .../src/human_schema/grammar.lalrpop | 8 ++- .../src/human_schema/test.rs | 50 +++++++++++++++++++ 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/cedar-policy-validator/src/human_schema/err.rs b/cedar-policy-validator/src/human_schema/err.rs index 0248c9dc11..03a7777646 100644 --- 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), + #[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 index 9ba1a4f77e..7ca3ec77f7 100644 --- a/cedar-policy-validator/src/human_schema/grammar.lalrpop +++ b/cedar-policy-validator/src/human_schema/grammar.lalrpop @@ -199,7 +199,13 @@ Ident: Node = { STRING => Node::with_source_loc("String".parse().unwrap(), Loc::new(l..r, Arc::clone(src))), - => 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 = { diff --git a/cedar-policy-validator/src/human_schema/test.rs b/cedar-policy-validator/src/human_schema/test.rs index c70b80404c..f41ae79c3a 100644 --- 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()); + } }