Skip to content

Commit 9e8decc

Browse files
committed
limit stack recursion to 50
1 parent 274d7df commit 9e8decc

File tree

3 files changed

+153
-9
lines changed

3 files changed

+153
-9
lines changed

src/resolve.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ where
8585
.map(|x| x.0);
8686

8787
for fd in &fragment_defs {
88-
match detect_fragment_cycles(fd, &mut HashSet::new(), &fragment_defs) {
88+
match detect_fragment_cycles(fd, &mut HashSet::new(), &fragment_defs, 1) {
8989
Ok(()) => {}
9090
Err(message) => {
9191
return GraphQLResponse {
@@ -516,14 +516,22 @@ where
516516
}
517517
}
518518

519+
const STACK_DEPTH_LIMIT: u32 = 50;
520+
519521
fn detect_fragment_cycles<'a, 'b, T>(
520522
fragment_definition: &'b FragmentDefinition<'a, T>,
521523
visited: &mut HashSet<&'b str>,
522524
fragment_definitions: &'b [FragmentDefinition<'a, T>],
525+
stack_depth: u32,
523526
) -> Result<(), String>
524527
where
525528
T: Text<'a>,
526529
{
530+
if stack_depth > STACK_DEPTH_LIMIT {
531+
return Err(format!(
532+
"Fragment cycle depth is greater than {STACK_DEPTH_LIMIT}"
533+
));
534+
}
527535
if visited.contains(fragment_definition.name.as_ref()) {
528536
return Err("Found a cycle between fragments".to_string());
529537
} else {
@@ -533,6 +541,7 @@ where
533541
&fragment_definition.selection_set,
534542
visited,
535543
fragment_definitions,
544+
stack_depth + 1,
536545
)?;
537546

538547
visited.remove(fragment_definition.name.as_ref());
@@ -543,23 +552,30 @@ fn detect_fragment_cycles_in_selection_set<'a, 'b, T>(
543552
selection_set: &'b SelectionSet<'a, T>,
544553
visited: &mut HashSet<&'b str>,
545554
fragment_definitions: &'b [FragmentDefinition<'a, T>],
555+
stack_depth: u32,
546556
) -> Result<(), String>
547557
where
548558
T: Text<'a>,
549559
{
560+
if stack_depth > STACK_DEPTH_LIMIT {
561+
return Err(format!(
562+
"Fragment cycle depth is greater than {STACK_DEPTH_LIMIT}"
563+
));
564+
}
550565
for selection in &selection_set.items {
551566
match selection {
552567
Selection::Field(field) => {
553568
detect_fragment_cycles_in_selection_set(
554569
&field.selection_set,
555570
visited,
556571
fragment_definitions,
572+
stack_depth + 1,
557573
)?;
558574
}
559575
Selection::FragmentSpread(fragment_spread) => {
560576
for fd in fragment_definitions {
561577
if fd.name == fragment_spread.fragment_name {
562-
detect_fragment_cycles(fd, visited, fragment_definitions)?;
578+
detect_fragment_cycles(fd, visited, fragment_definitions, stack_depth + 1)?;
563579
break;
564580
}
565581
}
@@ -569,6 +585,7 @@ where
569585
&inline_fragment.selection_set,
570586
visited,
571587
fragment_definitions,
588+
stack_depth + 1,
572589
)?;
573590
}
574591
}

test/expected/issue_fragment_spread_cycles.out

Lines changed: 69 additions & 1 deletion
Large diffs are not rendered by default.

test/sql/issue_fragment_spread_cycles.sql

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,13 @@ begin;
3636
}
3737
$$);
3838

39+
-- example from dockerfile
3940
create table account(
4041
id serial primary key,
4142
email varchar(255) not null,
4243
created_at timestamp not null
4344
);
4445

45-
4646
create table blog(
4747
id serial primary key,
4848
owner_id integer not null references account(id) on delete cascade,
@@ -51,10 +51,8 @@ begin;
5151
created_at timestamp not null
5252
);
5353

54-
5554
create type blog_post_status as enum ('PENDING', 'RELEASED');
5655

57-
5856
create table blog_post(
5957
id uuid not null default gen_random_uuid() primary key,
6058
blog_id integer not null references blog(id) on delete cascade,
@@ -64,8 +62,6 @@ begin;
6462
created_at timestamp not null
6563
);
6664

67-
68-
-- 5 Accounts
6965
insert into public.account(email, created_at)
7066
values
7167
('aardvark@x.com', now()),
@@ -81,7 +77,6 @@ begin;
8177
((select id from account where email ilike 'a%'), 'A: Blog 3', 'a desc3', now()),
8278
((select id from account where email ilike 'b%'), 'B: Blog 3', 'b desc1', now());
8379

84-
8580
comment on schema public is '@graphql({"inflect_names": true})';
8681

8782
select graphql.resolve($$
@@ -112,4 +107,68 @@ begin;
112107
}
113108
$$);
114109

110+
select graphql.resolve($$
111+
{
112+
blogCollection {
113+
edges {
114+
node {
115+
... blogFragment
116+
}
117+
}
118+
}
119+
}
120+
121+
fragment blogFragment on Blog {
122+
owner {
123+
blogCollection {
124+
edges {
125+
node {
126+
owner {
127+
blogCollection {
128+
edges {
129+
node {
130+
owner {
131+
blogCollection {
132+
edges {
133+
node {
134+
owner {
135+
blogCollection {
136+
edges {
137+
node {
138+
owner {
139+
blogCollection {
140+
edges {
141+
node {
142+
owner {
143+
blogCollection {
144+
edges {
145+
node {
146+
id
147+
}
148+
}
149+
}
150+
}
151+
}
152+
}
153+
}
154+
}
155+
}
156+
}
157+
}
158+
}
159+
}
160+
}
161+
}
162+
}
163+
}
164+
}
165+
}
166+
}
167+
}
168+
}
169+
}
170+
}
171+
}
172+
$$);
173+
115174
rollback;

0 commit comments

Comments
 (0)