Skip to content

Commit 6b415aa

Browse files
committed
fix(graphql): memoize fragment depth to bound depth-limit walk
1 parent bd54541 commit 6b415aa

2 files changed

Lines changed: 82 additions & 20 deletions

File tree

internal/serve/graphql/depth_limit.go

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func (d DepthLimit) MutateOperationContext(_ context.Context, opCtx *graphql.Ope
5050
return nil
5151
}
5252

53-
depth := selectionSetDepth(op.SelectionSet, opCtx.Doc.Fragments, map[string]bool{}, 0)
53+
depth := selectionSetDepth(op.SelectionSet, opCtx.Doc.Fragments)
5454
if depth > maxDepth {
5555
err := gqlerror.Errorf("operation has depth %d, which exceeds the limit of %d", depth, maxDepth)
5656
err.Extensions = map[string]interface{}{"code": errQueryTooDeep}
@@ -59,35 +59,57 @@ func (d DepthLimit) MutateOperationContext(_ context.Context, opCtx *graphql.Ope
5959
return nil
6060
}
6161

62-
// selectionSetDepth returns the maximum nesting depth reachable from set, starting at depth.
62+
// selectionSetDepth returns the maximum nesting depth reachable from set.
6363
// Every Field adds one level (whether or not it has children — a leaf scalar still counts as the
6464
// level it was selected at); InlineFragments are transparent (they don't add a level, matching
6565
// how they read in the query); FragmentSpreads are resolved against fragments and otherwise
6666
// treated the same as an InlineFragment.
67+
func selectionSetDepth(set ast.SelectionSet, fragments ast.FragmentDefinitionList) int {
68+
return selectionSetDepthMemo(set, fragments, map[string]bool{}, map[string]int{}, 0)
69+
}
70+
71+
// selectionSetDepthMemo is the recursive core of selectionSetDepth, threading two maps through the
72+
// walk: visitedFragments and fragmentDepths.
6773
//
6874
// visitedFragments tracks fragment names currently on the recursion stack, guarding against a
6975
// fragment that (directly or transitively) spreads itself: rather than recursing forever, a
7076
// repeated name is simply skipped.
71-
func selectionSetDepth(set ast.SelectionSet, fragments ast.FragmentDefinitionList, visitedFragments map[string]bool, depth int) int {
77+
//
78+
// A fragment spread adds no level of its own, so the depth a fragment contributes below its spread
79+
// point is independent of where it is spread (it's a pure additive offset onto the current depth).
80+
// fragmentDepths caches each fragment's depth, computed once relative to depth 0, so on any later
81+
// encounter we simply add the current depth. Without this cache, a fragment reachable through
82+
// multiple spreads is re-expanded on every path to it, so a document whose fragments fan out into
83+
// one another can force work that grows exponentially with the number of fragments even though the
84+
// document itself is small and its actual depth is shallow. Memoizing bounds the walk to
85+
// O(number of fragments + selections).
86+
func selectionSetDepthMemo(set ast.SelectionSet, fragments ast.FragmentDefinitionList, visitedFragments map[string]bool, fragmentDepths map[string]int, depth int) int {
7287
maxDepth := depth
7388
for _, sel := range set {
7489
var childDepth int
7590
switch sel := sel.(type) {
7691
case *ast.Field:
77-
childDepth = selectionSetDepth(sel.SelectionSet, fragments, visitedFragments, depth+1)
92+
childDepth = selectionSetDepthMemo(sel.SelectionSet, fragments, visitedFragments, fragmentDepths, depth+1)
7893
case *ast.InlineFragment:
79-
childDepth = selectionSetDepth(sel.SelectionSet, fragments, visitedFragments, depth)
94+
childDepth = selectionSetDepthMemo(sel.SelectionSet, fragments, visitedFragments, fragmentDepths, depth)
8095
case *ast.FragmentSpread:
8196
if visitedFragments[sel.Name] {
8297
continue
8398
}
84-
def := fragments.ForName(sel.Name)
85-
if def == nil {
86-
continue
99+
// Reuse the fragment's cached relative depth if we've expanded it before; otherwise
100+
// expand it once (from depth 0, guarded against cycles) and cache the result.
101+
relDepth, cached := fragmentDepths[sel.Name]
102+
if !cached {
103+
def := fragments.ForName(sel.Name)
104+
if def == nil {
105+
continue
106+
}
107+
visitedFragments[sel.Name] = true
108+
relDepth = selectionSetDepthMemo(def.SelectionSet, fragments, visitedFragments, fragmentDepths, 0)
109+
delete(visitedFragments, sel.Name)
110+
fragmentDepths[sel.Name] = relDepth
87111
}
88-
visitedFragments[sel.Name] = true
89-
childDepth = selectionSetDepth(def.SelectionSet, fragments, visitedFragments, depth)
90-
delete(visitedFragments, sel.Name)
112+
childDepth = depth + relDepth
91113
default:
92114
continue
93115
}

internal/serve/graphql/depth_limit_test.go

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package graphql
22

33
import (
44
"context"
5+
"fmt"
56
"testing"
67
"time"
78

@@ -19,7 +20,7 @@ func field(name string, sub ast.SelectionSet) *ast.Field {
1920
func TestSelectionSetDepth(t *testing.T) {
2021
t.Run("flat selection set is depth 1", func(t *testing.T) {
2122
set := ast.SelectionSet{field("hash", nil), field("ledgerNumber", nil)}
22-
assert.Equal(t, 1, selectionSetDepth(set, nil, map[string]bool{}, 0))
23+
assert.Equal(t, 1, selectionSetDepth(set, nil))
2324
})
2425

2526
t.Run("nested fields add one level each", func(t *testing.T) {
@@ -35,7 +36,7 @@ func TestSelectionSetDepth(t *testing.T) {
3536
}),
3637
}),
3738
}
38-
assert.Equal(t, 5, selectionSetDepth(set, nil, map[string]bool{}, 0))
39+
assert.Equal(t, 5, selectionSetDepth(set, nil))
3940
})
4041

4142
t.Run("inline fragment does not add a level", func(t *testing.T) {
@@ -49,7 +50,7 @@ func TestSelectionSetDepth(t *testing.T) {
4950
}),
5051
}
5152
// node=1, inline fragment transparent, balance=2
52-
assert.Equal(t, 2, selectionSetDepth(set, nil, map[string]bool{}, 0))
53+
assert.Equal(t, 2, selectionSetDepth(set, nil))
5354
})
5455

5556
t.Run("fragment spread is resolved against the document's fragments and adds no level itself", func(t *testing.T) {
@@ -70,7 +71,7 @@ func TestSelectionSetDepth(t *testing.T) {
7071
}),
7172
}
7273
// transactionByHash=1, spread transparent, operations=2, id=3
73-
assert.Equal(t, 3, selectionSetDepth(set, fragments, map[string]bool{}, 0))
74+
assert.Equal(t, 3, selectionSetDepth(set, fragments))
7475
})
7576

7677
t.Run("unresolvable fragment spread is skipped without panicking", func(t *testing.T) {
@@ -79,7 +80,7 @@ func TestSelectionSetDepth(t *testing.T) {
7980
&ast.FragmentSpread{Name: "DoesNotExist"},
8081
}),
8182
}
82-
assert.Equal(t, 1, selectionSetDepth(set, ast.FragmentDefinitionList{}, map[string]bool{}, 0))
83+
assert.Equal(t, 1, selectionSetDepth(set, ast.FragmentDefinitionList{}))
8384
})
8485

8586
t.Run("fragment cycle does not hang", func(t *testing.T) {
@@ -91,7 +92,7 @@ func TestSelectionSetDepth(t *testing.T) {
9192

9293
done := make(chan int, 1)
9394
go func() {
94-
done <- selectionSetDepth(set, fragments, map[string]bool{}, 0)
95+
done <- selectionSetDepth(set, fragments)
9596
}()
9697

9798
select {
@@ -102,16 +103,55 @@ func TestSelectionSetDepth(t *testing.T) {
102103
}
103104
})
104105

105-
t.Run("a fragment spread more than once (not a cycle) is still counted each time", func(t *testing.T) {
106+
t.Run("a fragment spread more than once (not a cycle) is counted correctly once", func(t *testing.T) {
106107
fragments := ast.FragmentDefinitionList{
107108
{Name: "Shared", SelectionSet: ast.SelectionSet{field("value", nil)}},
108109
}
109110
set := ast.SelectionSet{
110111
field("a", ast.SelectionSet{&ast.FragmentSpread{Name: "Shared"}}),
111112
field("b", ast.SelectionSet{&ast.FragmentSpread{Name: "Shared"}}),
112113
}
113-
// a=1/b=1, Shared transparent, value=2
114-
assert.Equal(t, 2, selectionSetDepth(set, fragments, map[string]bool{}, 0))
114+
// a=1/b=1, Shared transparent, value=2. Depth is a max, so spreading Shared twice yields
115+
// the same depth as once — the memo cache changes cost, not the result.
116+
assert.Equal(t, 2, selectionSetDepth(set, fragments))
117+
})
118+
119+
// A document whose fragments fan out into one another must stay cheap to measure. Here each
120+
// fragment spreads the next twice, so without per-fragment memoization the walk re-expands the
121+
// chain on every path and does O(2^N) work; at N=64 that would never finish. With memoization
122+
// it is linear and returns immediately, while still reporting the true (shallow) depth of 2.
123+
t.Run("fragment fan-out does not blow up the walk", func(t *testing.T) {
124+
const n = 64
125+
fragments := make(ast.FragmentDefinitionList, 0, n)
126+
for i := 0; i < n-1; i++ {
127+
fragments = append(fragments, &ast.FragmentDefinition{
128+
Name: fmt.Sprintf("f%d", i),
129+
SelectionSet: ast.SelectionSet{
130+
&ast.FragmentSpread{Name: fmt.Sprintf("f%d", i+1)},
131+
&ast.FragmentSpread{Name: fmt.Sprintf("f%d", i+1)},
132+
},
133+
})
134+
}
135+
fragments = append(fragments, &ast.FragmentDefinition{
136+
Name: fmt.Sprintf("f%d", n-1),
137+
SelectionSet: ast.SelectionSet{field("address", nil)},
138+
})
139+
set := ast.SelectionSet{
140+
field("accountByAddress", ast.SelectionSet{&ast.FragmentSpread{Name: "f0"}}),
141+
}
142+
143+
done := make(chan int, 1)
144+
go func() {
145+
done <- selectionSetDepth(set, fragments)
146+
}()
147+
148+
select {
149+
case depth := <-done:
150+
// accountByAddress=1, fragment chain transparent, address=2.
151+
assert.Equal(t, 2, depth)
152+
case <-time.After(5 * time.Second):
153+
t.Fatal("selectionSetDepth did not terminate on a binary fan-out fragment bomb; memoization is not working and the DoS is present")
154+
}
115155
})
116156
}
117157

0 commit comments

Comments
 (0)