From f52629f539aa960381f233fdb4f1ae61f42277b3 Mon Sep 17 00:00:00 2001 From: "aneesh.reddy" Date: Wed, 2 Oct 2024 16:19:43 +0900 Subject: [PATCH 1/4] [AIM-1482] limit number of aliases per query --- .../rules/limit_number_of_aliases.rs | 156 ++++++++++++++++++ juniper/src/validation/rules/mod.rs | 4 +- 2 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 juniper/src/validation/rules/limit_number_of_aliases.rs diff --git a/juniper/src/validation/rules/limit_number_of_aliases.rs b/juniper/src/validation/rules/limit_number_of_aliases.rs new file mode 100644 index 000000000..e973ab267 --- /dev/null +++ b/juniper/src/validation/rules/limit_number_of_aliases.rs @@ -0,0 +1,156 @@ +use crate::{ + ast::{Field, Operation}, + parser::Spanning, + validation::{ValidatorContext, Visitor}, + value::ScalarValue, +}; + +pub struct Aliases { + alias_count: u8, + max_allowed: u8, +} + +pub fn factory<'a>() -> Aliases { + Aliases { + alias_count: 0, + max_allowed: 1, + } +} + +impl<'a, S> Visitor<'a, S> for Aliases +where + S: ScalarValue, +{ + fn enter_operation_definition( + &mut self, + _ctx: &mut ValidatorContext<'a, S>, + _op: &'a Spanning>, + ) { + self.alias_count = 0; // Reset for each operation + } + + fn enter_field( + &mut self, + ctx: &mut ValidatorContext<'a, S>, + field: &'a Spanning>, + ) { + let alias_name = &field.item.alias; // Get the Spanning for the alias + + if let Some(alias) = alias_name { + self.alias_count += 1; + if self.alias_count > self.max_allowed { + ctx.report_error(&error_message(&alias.item), &[alias.start]); + } + } + } +} + +fn error_message(alias_name: &str) -> String { + format!("There can only be one alias, {} is not allowed", alias_name) +} + +#[cfg(test)] +mod tests { + use super::{error_message, factory}; + + use crate::{ + parser::SourcePosition, + validation::{expect_fails_rule, expect_passes_rule, RuleError}, + value::DefaultScalarValue, + }; + + #[test] + fn single_alias_allowed() { + expect_passes_rule::<_, _, DefaultScalarValue>( + factory, + r#" + mutation CreateLiquidToken { + liquidApplication: createLiquidSdkApplication { + token + applicantId + } + } + "#, + ); + + expect_passes_rule::<_, _, DefaultScalarValue>( + factory, + r#" + mutation UpdateLiquidToken { + liquidApplication: createLiquidSdkUpdateApplication { + token + applicantId + } + } + "#, + ); + } + + #[test] + fn multiple_field_aliases_not_allowed() { + expect_fails_rule::<_, _, DefaultScalarValue>( + factory, + r#" + query MyQuery { + myField1: my_field, + myField2: my_field, + myField3: my_field + } + "#, + &[ + RuleError::new( + &error_message("myField2"), + &[ + SourcePosition::new(69, 3, 12), // includes whitespaces + ], + ), + RuleError::new( + &error_message("myField3"), + &[SourcePosition::new(101, 4, 12)], + ), + ], + ); + } + + #[test] + fn multiple_query_aliases_not_allowed() { + expect_fails_rule::<_, _, DefaultScalarValue>( + factory, + r#" + query Hero { + empireHero: hero(episode: EMPIRE) { + name + } + jediHero: hero(episode: JEDI) { + name + } + republicHero: hero(episode: REPUBLIC) { + name + } + } + "#, + &[ + RuleError::new( + &error_message("jediHero"), + &[SourcePosition::new(117, 5, 12)], + ), + RuleError::new( + &error_message("republicHero"), + &[SourcePosition::new(194, 8, 12)], + ), + ], + ); + } + + #[test] + fn no_aliases() { + expect_passes_rule::<_, _, DefaultScalarValue>( + factory, + r#" + query MyQuery { + my_field + } + "#, + ); + } +} diff --git a/juniper/src/validation/rules/mod.rs b/juniper/src/validation/rules/mod.rs index 60a38d13d..50b2c3ada 100644 --- a/juniper/src/validation/rules/mod.rs +++ b/juniper/src/validation/rules/mod.rs @@ -6,6 +6,7 @@ mod known_argument_names; mod known_directives; mod known_fragment_names; mod known_type_names; +mod limit_number_of_aliases; mod lone_anonymous_operation; mod no_fragment_cycles; mod no_undefined_variables; @@ -65,7 +66,8 @@ where .with(self::unique_operation_names::factory()) .with(self::unique_variable_names::factory()) .with(self::variables_are_input_types::factory()) - .with(self::variables_in_allowed_position::factory()); + .with(self::variables_in_allowed_position::factory()) + .with(self::limit_number_of_aliases::factory()); visit(&mut stage1, ctx, doc); if ctx.has_errors() { return; From b5c7c2e3feb972210c0ab0e9798e9a472009c0f6 Mon Sep 17 00:00:00 2001 From: "aneesh.reddy" Date: Mon, 7 Oct 2024 16:02:10 +0900 Subject: [PATCH 2/4] [AIM-1482] increase allowed alias count to 3 --- .../rules/limit_number_of_aliases.rs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/juniper/src/validation/rules/limit_number_of_aliases.rs b/juniper/src/validation/rules/limit_number_of_aliases.rs index e973ab267..934ed7ad2 100644 --- a/juniper/src/validation/rules/limit_number_of_aliases.rs +++ b/juniper/src/validation/rules/limit_number_of_aliases.rs @@ -13,7 +13,7 @@ pub struct Aliases { pub fn factory<'a>() -> Aliases { Aliases { alias_count: 0, - max_allowed: 1, + max_allowed: 3, } } @@ -94,19 +94,19 @@ mod tests { query MyQuery { myField1: my_field, myField2: my_field, - myField3: my_field + myField3: my_field, + myField4: my_field, + myField5: my_field } "#, &[ RuleError::new( - &error_message("myField2"), - &[ - SourcePosition::new(69, 3, 12), // includes whitespaces - ], + &error_message("myField4"), + &[SourcePosition::new(133, 5, 12)], ), RuleError::new( - &error_message("myField3"), - &[SourcePosition::new(101, 4, 12)], + &error_message("myField5"), + &[SourcePosition::new(165, 6, 12)], ), ], ); @@ -127,16 +127,22 @@ mod tests { republicHero: hero(episode: REPUBLIC) { name } + republicH3ro: hero(episode: REPUBLIC) { + name + } + republicH4ro: hero(episode: REPUBLIC) { + name + } } "#, &[ RuleError::new( - &error_message("jediHero"), - &[SourcePosition::new(117, 5, 12)], + &error_message("republicH3ro"), + &[SourcePosition::new(268, 8, 12)], ), RuleError::new( - &error_message("republicHero"), - &[SourcePosition::new(194, 8, 12)], + &error_message("republicH4ro"), + &[SourcePosition::new(342, 8, 12)], ), ], ); From d8f344b1be2da9435887a2226095943807e38338 Mon Sep 17 00:00:00 2001 From: "aneesh.reddy" Date: Tue, 8 Oct 2024 13:21:49 +0900 Subject: [PATCH 3/4] [AIM-1482] update error msg, add test case --- .../rules/limit_number_of_aliases.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/juniper/src/validation/rules/limit_number_of_aliases.rs b/juniper/src/validation/rules/limit_number_of_aliases.rs index 934ed7ad2..71dae75e0 100644 --- a/juniper/src/validation/rules/limit_number_of_aliases.rs +++ b/juniper/src/validation/rules/limit_number_of_aliases.rs @@ -46,7 +46,7 @@ where } fn error_message(alias_name: &str) -> String { - format!("There can only be one alias, {} is not allowed", alias_name) + format!("Illegal number of aliases, {} is not allowed", alias_name) } #[cfg(test)] @@ -84,6 +84,22 @@ mod tests { } "#, ); + + expect_passes_rule::<_, _, DefaultScalarValue>( + factory, + r#" + query Hero { + empireHero: hero(episode: EMPIRE) { + name + } + jediHero: hero(episode: JEDI) { + name + } + republicHero: hero(episode: REPUBLIC) { + name + } + "#, + ); } #[test] From b48a12641d9f13f0c68a9f03fd64fc0f6c70e1bc Mon Sep 17 00:00:00 2001 From: "aneesh.reddy" Date: Tue, 8 Oct 2024 13:22:29 +0900 Subject: [PATCH 4/4] [AIM-1482] update error msg, add test case --- juniper/src/validation/rules/limit_number_of_aliases.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/juniper/src/validation/rules/limit_number_of_aliases.rs b/juniper/src/validation/rules/limit_number_of_aliases.rs index 71dae75e0..1aa6736a2 100644 --- a/juniper/src/validation/rules/limit_number_of_aliases.rs +++ b/juniper/src/validation/rules/limit_number_of_aliases.rs @@ -84,7 +84,10 @@ mod tests { } "#, ); + } + #[test] + fn multiple_alias_allowed() { expect_passes_rule::<_, _, DefaultScalarValue>( factory, r#"