Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add query complexity and depth support in server directive #2828

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
9edd1d0
- added transforms for rules.
laststylebender14 Sep 16, 2024
6a550b3
- impl rules
laststylebender14 Sep 16, 2024
000f018
- move calculations to specific rules
laststylebender14 Sep 16, 2024
cb109b5
- remove un-used test
laststylebender14 Sep 16, 2024
7bec760
- removed
laststylebender14 Sep 16, 2024
35da5b8
- lint formatting
laststylebender14 Sep 16, 2024
48f2e4e
- used associated types
laststylebender14 Sep 16, 2024
bbdcb49
- added test cases
laststylebender14 Sep 16, 2024
2adc5f4
- fix the Value to ConstValue
laststylebender14 Sep 16, 2024
d756b26
- fix constvalue and value issue.
laststylebender14 Sep 16, 2024
3aa2424
- take query complexity and depth from server directive.
laststylebender14 Sep 16, 2024
5db2d92
- add when operator to rule
laststylebender14 Sep 16, 2024
8054dd9
- apply rules conditionally.
laststylebender14 Sep 16, 2024
72bb868
- auto updated
laststylebender14 Sep 16, 2024
3fc1b4f
- updated snap: added server to index.
laststylebender14 Sep 16, 2024
5ed4208
- made rules generic over T.
laststylebender14 Sep 16, 2024
a864282
Merge branch 'main' into feat/add-query-complexity-and-depth
laststylebender14 Sep 16, 2024
f06a19d
- added default to ensure only applied setting is executed.
laststylebender14 Sep 16, 2024
b5fd2cb
- auto generated.
laststylebender14 Sep 16, 2024
ff3844f
- updated error msg.
laststylebender14 Sep 16, 2024
c9d3518
- apply setting only when flag is true.
laststylebender14 Sep 16, 2024
d335da0
- lint fixes
laststylebender14 Sep 16, 2024
6ed10b6
Merge branch 'main' into feat/add-query-complexity-and-depth
laststylebender14 Sep 16, 2024
03ce3f4
- rename trait to ExecutionRule
laststylebender14 Sep 16, 2024
1969d62
- fix ci tests.
laststylebender14 Sep 16, 2024
80c9965
- lint fixes
laststylebender14 Sep 16, 2024
ebbd47f
- added integration test for query complexity
laststylebender14 Sep 16, 2024
2ef01f2
- added integration test for query depth
laststylebender14 Sep 16, 2024
b182614
- lint fixes
laststylebender14 Sep 16, 2024
8d011d7
Merge branch 'main' into feat/add-query-complexity-and-depth
laststylebender14 Sep 16, 2024
ef198aa
- instead of converting use validationerror as variant in build error…
laststylebender14 Sep 17, 2024
0d40854
- iterate over iterator instead of converting to vec.
laststylebender14 Sep 17, 2024
9b707f8
- removed pub
laststylebender14 Sep 17, 2024
5cd51b8
- clean up - use iter_only to iterate over field.
laststylebender14 Sep 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions generated/.tailcallrc.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,18 @@ directive @server(
"""
port: Int
"""
`queryComplexity` sets the maximum allowed complexity for a GraphQL query. It helps
prevent resource-intensive queries by limiting their complexity. If set, queries
exceeding this complexity will be rejected.
"""
queryComplexity: Int
"""
`queryDepth` sets the maximum allowed depth for a GraphQL query. It helps prevent
deeply nested queries that could potentially overload the server. If set, queries
exceeding this depth will be rejected.
"""
queryDepth: Int
"""
`queryValidation` checks incoming GraphQL queries against the schema, preventing
errors from invalid queries. Can be disabled for performance. @default `false`.
"""
Expand Down
18 changes: 18 additions & 0 deletions generated/.tailcallrc.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,24 @@
"format": "uint16",
"minimum": 0.0
},
"queryComplexity": {
"description": "`queryComplexity` sets the maximum allowed complexity for a GraphQL query. It helps prevent resource-intensive queries by limiting their complexity. If set, queries exceeding this complexity will be rejected.",
"type": [
"integer",
"null"
],
"format": "uint",
"minimum": 0.0
},
"queryDepth": {
"description": "`queryDepth` sets the maximum allowed depth for a GraphQL query. It helps prevent deeply nested queries that could potentially overload the server. If set, queries exceeding this depth will be rejected.",
"type": [
"integer",
"null"
],
"format": "uint",
"minimum": 0.0
},
"queryValidation": {
"description": "`queryValidation` checks incoming GraphQL queries against the schema, preventing errors from invalid queries. Can be disabled for performance. @default `false`.",
"type": [
Expand Down
23 changes: 20 additions & 3 deletions src/core/blueprint/index.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use indexmap::IndexMap;

use crate::core::blueprint::{
Blueprint, Definition, FieldDefinition, InputFieldDefinition, SchemaDefinition,
Blueprint, Definition, FieldDefinition, InputFieldDefinition, SchemaDefinition, Server,
};
use crate::core::scalar;

Expand All @@ -13,6 +13,7 @@ use crate::core::scalar;
pub struct Index {
map: IndexMap<String, (Definition, IndexMap<String, QueryField>)>,
schema: SchemaDefinition,
server: Server,
}

#[derive(Debug)]
Expand Down Expand Up @@ -78,6 +79,14 @@ impl Index {
false
}
}

pub fn query_complexity(&self) -> Option<usize> {
self.server.query_complexity
}

pub fn query_depth(&self) -> Option<usize> {
self.server.query_depth
}
}

impl From<&Blueprint> for Index {
Expand Down Expand Up @@ -169,7 +178,11 @@ impl From<&Blueprint> for Index {
}
}

Self { map, schema: blueprint.schema.to_owned() }
Self {
map,
schema: blueprint.schema.to_owned(),
server: blueprint.server.to_owned(),
}
}
}

Expand All @@ -183,7 +196,11 @@ mod test {
fn setup() -> Index {
let config = include_config!("./fixture/all-constructs.graphql").unwrap();
let cfg_module = ConfigModule::from(config);
let blueprint = Blueprint::try_from(&cfg_module).unwrap();
let mut blueprint = Blueprint::try_from(&cfg_module).unwrap();
// Set a fixed number of workers to ensure consistent snapshots across different
// environments This is necessary because the number of workers might
// vary on different CI systems
blueprint.server = blueprint.server.worker(4);

Index::from(&blueprint)
}
Expand Down
4 changes: 4 additions & 0 deletions src/core/blueprint/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub struct Server {
pub experimental_headers: HashSet<HeaderName>,
pub auth: Option<Auth>,
pub dedupe: bool,
pub query_complexity: Option<usize>,
pub query_depth: Option<usize>,
}

/// Mimic of mini_v8::Script that's wasm compatible
Expand Down Expand Up @@ -154,6 +156,8 @@ impl TryFrom<crate::core::config::ConfigModule> for Server {
cors,
auth,
dedupe: config_server.get_dedupe(),
query_complexity: config_server.get_query_complexity(),
query_depth: config_server.get_query_depth(),
}
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1200,4 +1200,30 @@ Index {
},
],
},
server: Server {
enable_jit: true,
enable_apollo_tracing: false,
enable_cache_control_header: false,
enable_set_cookie_header: false,
enable_introspection: true,
enable_query_validation: false,
enable_response_validation: false,
enable_batch_requests: false,
enable_showcase: false,
global_response_timeout: 0,
worker: 4,
port: 8000,
hostname: 127.0.0.1,
vars: {},
response_headers: {},
http: HTTP1,
pipeline_flush: true,
script: None,
cors: None,
experimental_headers: {},
auth: None,
dedupe: false,
query_complexity: None,
query_depth: None,
},
}
19 changes: 19 additions & 0 deletions src/core/config/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,19 @@ pub struct Server {
/// `workers` sets the number of worker threads. @default the number of
/// system cores.
pub workers: Option<usize>,

#[serde(default, skip_serializing_if = "is_default")]
/// `queryComplexity` sets the maximum allowed complexity for a GraphQL
/// query. It helps prevent resource-intensive queries by limiting their
/// complexity. If set, queries exceeding this complexity will be
/// rejected.
pub query_complexity: Option<usize>,

#[serde(default, skip_serializing_if = "is_default")]
/// `queryDepth` sets the maximum allowed depth for a GraphQL query.
/// It helps prevent deeply nested queries that could potentially overload
/// the server. If set, queries exceeding this depth will be rejected.
pub query_depth: Option<usize>,
}

fn merge_right_vars(mut left: Vec<KeyValue>, right: Vec<KeyValue>) -> Vec<KeyValue> {
Expand Down Expand Up @@ -231,6 +244,12 @@ impl Server {
pub fn enable_jit(&self) -> bool {
self.enable_jit.unwrap_or(true)
}
pub fn get_query_complexity(&self) -> Option<usize> {
self.query_complexity
}
pub fn get_query_depth(&self) -> Option<usize> {
self.query_depth
}
}

#[cfg(test)]
Expand Down
18 changes: 17 additions & 1 deletion src/core/jit/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ use async_graphql_value::{ConstValue, Value};

use super::input_resolver::InputResolver;
use super::model::{Directive as JitDirective, *};
use super::rules::{ExecutionRule, QueryComplexity, QueryDepth, RuleOps, Rules};
use super::BuildError;
use crate::core::blueprint::{Blueprint, Index, QueryField};
use crate::core::counter::{Count, Counter};
use crate::core::jit::model::OperationPlan;
use crate::core::merge_right::MergeRight;
use crate::core::valid::Validator;
use crate::core::Type;

#[derive(PartialEq, strum_macros::Display)]
Expand Down Expand Up @@ -360,12 +362,26 @@ impl Builder {
is_introspection_query,
);

// perform the rule check.
Rules
.pipe(
QueryComplexity::new(self.index.query_complexity().unwrap_or_default())
.when(self.index.query_complexity().is_some()),
)
.pipe(
QueryDepth::new(self.index.query_depth().unwrap_or_default())
.when(self.index.query_depth().is_some()),
)
.validate(&plan)
.to_result()?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store the computed query depth in the plan. The builder should not fail if depth or complexity thresholds are exceeded.


// TODO: operation from [ExecutableDocument] could contain definitions for
// default values of arguments. That info should be passed to
// [InputResolver] to resolve defaults properly
let input_resolver = InputResolver::new(plan);
let plan = input_resolver.resolve_input(variables)?;

Ok(input_resolver.resolve_input(variables)?)
Ok(plan)
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/core/jit/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ use async_graphql::parser::types::OperationType;
use async_graphql::{ErrorExtensions, ServerError};
use thiserror::Error;

#[derive(Error, Debug, Clone, PartialEq, Eq)]
use crate::core::valid::ValidationError as CoreValidationError;

#[derive(Error, Debug, Clone, PartialEq)]
#[error("Error while building the plan")]
pub enum BuildError {
#[error("Root Operation type not defined for {operation}")]
Expand All @@ -13,6 +15,8 @@ pub enum BuildError {
OperationNotFound(String),
#[error("Operation name required in request")]
OperationNameRequired,
#[error("{0}")]
ValidationError(#[from] CoreValidationError<String>),
}

#[derive(Error, Debug, Clone, PartialEq, Eq)]
Expand Down
1 change: 1 addition & 0 deletions src/core/jit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ mod exec_const;
mod input_resolver;
mod request;
mod response;
mod rules;

// NOTE: Only used in tests and benchmarks
mod builder;
Expand Down
2 changes: 1 addition & 1 deletion src/core/jit/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ impl Flat {
/// Store field relationships in a nested structure like a tree where each field
/// links to its children.
#[derive(Clone, Debug)]
pub struct Nested<Input>(Vec<Field<Nested<Input>, Input>>);
pub struct Nested<Input>(pub Vec<Field<Nested<Input>, Input>>);
laststylebender14 marked this conversation as resolved.
Show resolved Hide resolved

#[derive(Clone)]
pub struct OperationPlan<Input> {
Expand Down
59 changes: 59 additions & 0 deletions src/core/jit/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
use super::OperationPlan;
use crate::core::valid::{Valid, Validator};

mod query_complexity;
mod query_depth;

pub use query_complexity::QueryComplexity;
pub use query_depth::QueryDepth;

pub trait ExecutionRule {
fn validate<T: std::fmt::Debug>(&self, plan: &OperationPlan<T>) -> Valid<(), String>;
}

pub trait RuleOps: Sized + ExecutionRule {
fn pipe<Other: ExecutionRule>(self, other: Other) -> Pipe<Self, Other> {
Pipe(self, other)
}
fn when(self, cond: bool) -> When<Self> {
When(self, cond)
}
}

impl<T: ExecutionRule> RuleOps for T {}

pub struct Pipe<A, B>(A, B);

impl<A, B> ExecutionRule for Pipe<A, B>
where
A: ExecutionRule,
B: ExecutionRule,
{
fn validate<T: std::fmt::Debug>(&self, plan: &OperationPlan<T>) -> Valid<(), String> {
self.0.validate(plan).and_then(|_| self.1.validate(plan))
}
}

pub struct When<A>(A, bool);

impl<A> ExecutionRule for When<A>
where
A: ExecutionRule,
{
fn validate<T: std::fmt::Debug>(&self, plan: &OperationPlan<T>) -> Valid<(), String> {
if self.1 {
self.0.validate(plan)
} else {
Valid::succeed(())
}
}
}

#[derive(Default)]
pub struct Rules;

impl ExecutionRule for Rules {
fn validate<T: std::fmt::Debug>(&self, _: &OperationPlan<T>) -> Valid<(), String> {
Valid::succeed(())
}
}
Loading