Skip to content

Commit 5a1987f

Browse files
committed
Protect against nil options in bundles
GODRIVER-493 Change-Id: I25d861227f95b7016cd78c35f389b7b04a962a72
1 parent eecffe0 commit 5a1987f

37 files changed

+551
-56
lines changed

mongo/aggregateopt/aggregateopt.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,10 @@ func (ab *AggregateBundle) bundleLength() int {
140140
}
141141

142142
bundleLen := 0
143-
for ; ab != nil && ab.option != nil; ab = ab.next {
143+
for ; ab != nil; ab = ab.next {
144+
if ab.option == nil {
145+
continue
146+
}
144147
if converted, ok := ab.option.(*AggregateBundle); ok {
145148
// nested bundle
146149
bundleLen += converted.bundleLength()
@@ -197,7 +200,11 @@ func (ab *AggregateBundle) unbundle() ([]option.AggregateOptioner, *session.Clie
197200
options := make([]option.AggregateOptioner, listLen)
198201
index := listLen - 1
199202

200-
for listHead := ab; listHead != nil && listHead.option != nil; listHead = listHead.next {
203+
for listHead := ab; listHead != nil; listHead = listHead.next {
204+
if listHead.option == nil {
205+
continue
206+
}
207+
201208
// if the current option is a nested bundle, Unbundle it and add its options to the current array
202209
if converted, ok := listHead.option.(*AggregateBundle); ok {
203210
nestedOptions, s, err := converted.unbundle()

mongo/aggregateopt/aggregateopt_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,23 @@ func TestAggregateOpt(t *testing.T) {
185185
}
186186
})
187187

188+
t.Run("Nil Option Bundle", func(t *testing.T) {
189+
sess := AggregateSessionOpt{}
190+
opts, _, err := BundleAggregate(AllowDiskUse(true), BundleAggregate(nil), sess, nil).unbundle()
191+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
192+
193+
if len(opts) != 1 {
194+
t.Errorf("expected bundle length 1. got: %d", len(opts))
195+
}
196+
197+
opts, _, err = BundleAggregate(nil, sess, BundleAggregate(nil), AllowDiskUse(true)).unbundle()
198+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
199+
200+
if len(opts) != 1 {
201+
t.Errorf("expected bundle length 1. got: %d", len(opts))
202+
}
203+
})
204+
188205
t.Run("MakeOptions", func(t *testing.T) {
189206
head := bundle1
190207

mongo/changestreamopt/changestreamopt.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,19 @@ func (csb *ChangeStreamBundle) bundleLength() int {
147147
}
148148

149149
bundleLen := 0
150-
for ; csb != nil && csb.option != nil; csb = csb.next {
150+
for ; csb != nil; csb = csb.next {
151+
if csb.option == nil {
152+
continue
153+
}
151154
if converted, ok := csb.option.(*ChangeStreamBundle); ok {
152155
// nested bundle
153156
bundleLen += converted.bundleLength()
154157
continue
155158
}
156159

157-
bundleLen++
160+
if _, ok := csb.option.(ChangeStreamSessionOpt); !ok {
161+
bundleLen++
162+
}
158163
}
159164

160165
return bundleLen
@@ -172,7 +177,11 @@ func (csb *ChangeStreamBundle) unbundle() ([]option.ChangeStreamOptioner, *sessi
172177
options := make([]option.ChangeStreamOptioner, listLen)
173178
index := listLen - 1
174179

175-
for listHead := csb; listHead != nil && listHead.option != nil; listHead = listHead.next {
180+
for listHead := csb; listHead != nil; listHead = listHead.next {
181+
if listHead.option == nil {
182+
continue
183+
}
184+
176185
// if the current option is a nested bundle, Unbundle it and add its options to the current array
177186
if converted, ok := listHead.option.(*ChangeStreamBundle); ok {
178187
nestedOptions, s, err := converted.unbundle()

mongo/changestreamopt/changestreamopt_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,23 @@ func TestChangeStreamOpt(t *testing.T) {
195195
}
196196
})
197197

198+
t.Run("Nil Option Bundle", func(t *testing.T) {
199+
sess := ChangeStreamSessionOpt{}
200+
opts, _, err := BundleChangeStream(BatchSize(1), BundleChangeStream(nil), sess, nil).unbundle()
201+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
202+
203+
if len(opts) != 1 {
204+
t.Errorf("expected bundle length 1. got: %d", len(opts))
205+
}
206+
207+
opts, _, err = BundleChangeStream(nil, sess, BundleChangeStream(nil), BatchSize(1)).unbundle()
208+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
209+
210+
if len(opts) != 1 {
211+
t.Errorf("expected bundle length 1. got: %d", len(opts))
212+
}
213+
})
214+
198215
t.Run("MakeOptions", func(t *testing.T) {
199216
head := bundle1
200217

mongo/countopt/countopt.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,19 @@ func (cb *CountBundle) bundleLength() int {
150150
}
151151

152152
bundleLen := 0
153-
for ; cb != nil && cb.option != nil; cb = cb.next {
153+
for ; cb != nil; cb = cb.next {
154+
if cb.option == nil {
155+
continue
156+
}
154157
if converted, ok := cb.option.(*CountBundle); ok {
155158
// nested bundle
156159
bundleLen += converted.bundleLength()
157160
continue
158161
}
159162

160-
bundleLen++
163+
if _, ok := cb.option.(CountSessionOpt); !ok {
164+
bundleLen++
165+
}
161166
}
162167

163168
return bundleLen
@@ -175,7 +180,11 @@ func (cb *CountBundle) unbundle() ([]option.CountOptioner, *session.Client, erro
175180
options := make([]option.CountOptioner, listLen)
176181
index := listLen - 1
177182

178-
for listHead := cb; listHead != nil && listHead.option != nil; listHead = listHead.next {
183+
for listHead := cb; listHead != nil; listHead = listHead.next {
184+
if listHead.option == nil {
185+
continue
186+
}
187+
179188
// if the current option is a nested bundle, Unbundle it and add its options to the current array
180189
if converted, ok := listHead.option.(*CountBundle); ok {
181190
nestedOptions, s, err := converted.unbundle()

mongo/countopt/countopt_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,23 @@ func TestCountOpt(t *testing.T) {
168168
}
169169
})
170170

171+
t.Run("Nil Option Bundle", func(t *testing.T) {
172+
sess := CountSessionOpt{}
173+
opts, _, err := BundleCount(Skip(1), BundleCount(nil), sess, nil).unbundle()
174+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
175+
176+
if len(opts) != 1 {
177+
t.Errorf("expected bundle length 1. got: %d", len(opts))
178+
}
179+
180+
opts, _, err = BundleCount(nil, sess, BundleCount(nil), Skip(1)).unbundle()
181+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
182+
183+
if len(opts) != 1 {
184+
t.Errorf("expected bundle length 1. got: %d", len(opts))
185+
}
186+
})
187+
171188
t.Run("MakeOptions", func(t *testing.T) {
172189
head := bundle1
173190

mongo/deleteopt/deleteopt.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,19 @@ func (db *DeleteBundle) bundleLength() int {
110110
}
111111

112112
bundleLen := 0
113-
for ; db != nil && db.option != nil; db = db.next {
113+
for ; db != nil; db = db.next {
114+
if db.option == nil {
115+
continue
116+
}
114117
if converted, ok := db.option.(*DeleteBundle); ok {
115118
// nested bundle
116119
bundleLen += converted.bundleLength()
117120
continue
118121
}
119122

120-
bundleLen++
123+
if _, ok := db.option.(DeleteSessionOpt); !ok {
124+
bundleLen++
125+
}
121126
}
122127

123128
return bundleLen
@@ -135,7 +140,11 @@ func (db *DeleteBundle) unbundle() ([]option.DeleteOptioner, *session.Client, er
135140
options := make([]option.DeleteOptioner, listLen)
136141
index := listLen - 1
137142

138-
for listHead := db; listHead != nil && listHead.option != nil; listHead = listHead.next {
143+
for listHead := db; listHead != nil; listHead = listHead.next {
144+
if listHead.option == nil {
145+
continue
146+
}
147+
139148
// if the current option is a nested bundle, Unbundle it and add its options to the current array
140149
if converted, ok := listHead.option.(*DeleteBundle); ok {
141150
nestedOptions, s, err := converted.unbundle()

mongo/deleteopt/deleteopt_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,23 @@ func TestDeleteOpt(t *testing.T) {
157157
}
158158
})
159159

160+
t.Run("Nil Option Bundle", func(t *testing.T) {
161+
sess := DeleteSessionOpt{}
162+
opts, _, err := BundleDelete(Collation(c), BundleDelete(nil), sess, nil).unbundle()
163+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
164+
165+
if len(opts) != 1 {
166+
t.Errorf("expected bundle length 1. got: %d", len(opts))
167+
}
168+
169+
opts, _, err = BundleDelete(nil, sess, BundleDelete(nil), Collation(c)).unbundle()
170+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
171+
172+
if len(opts) != 1 {
173+
t.Errorf("expected bundle length 1. got: %d", len(opts))
174+
}
175+
})
176+
160177
t.Run("MakeOptions", func(t *testing.T) {
161178
head := bundle1
162179

mongo/distinctopt/distinctopt.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,19 @@ func (db *DistinctBundle) bundleLength() int {
116116
}
117117

118118
bundleLen := 0
119-
for ; db != nil && db.option != nil; db = db.next {
119+
for ; db != nil; db = db.next {
120+
if db.option == nil {
121+
continue
122+
}
120123
if converted, ok := db.option.(*DistinctBundle); ok {
121124
// nested bundle
122125
bundleLen += converted.bundleLength()
123126
continue
124127
}
125128

126-
bundleLen++
129+
if _, ok := db.option.(DistinctSessionOpt); !ok {
130+
bundleLen++
131+
}
127132
}
128133

129134
return bundleLen
@@ -141,7 +146,11 @@ func (db *DistinctBundle) unbundle() ([]option.DistinctOptioner, *session.Client
141146
options := make([]option.DistinctOptioner, listLen)
142147
index := listLen - 1
143148

144-
for listHead := db; listHead != nil && listHead.option != nil; listHead = listHead.next {
149+
for listHead := db; listHead != nil; listHead = listHead.next {
150+
if listHead.option == nil {
151+
continue
152+
}
153+
145154
// if the current option is a nested bundle, Unbundle it and add its options to the current array
146155
if converted, ok := listHead.option.(*DistinctBundle); ok {
147156
nestedOptions, s, err := converted.unbundle()

mongo/distinctopt/distinctopt_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,23 @@ func TestDistinctOpt(t *testing.T) {
154154
}
155155
})
156156

157+
t.Run("Nil Option Bundle", func(t *testing.T) {
158+
sess := DistinctSessionOpt{}
159+
opts, _, err := BundleDistinct(MaxTime(1), BundleDistinct(nil), sess, nil).unbundle()
160+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
161+
162+
if len(opts) != 1 {
163+
t.Errorf("expected bundle length 1. got: %d", len(opts))
164+
}
165+
166+
opts, _, err = BundleDistinct(nil, sess, BundleDistinct(nil), MaxTime(1)).unbundle()
167+
testhelpers.RequireNil(t, err, "got non-nil error from unbundle: %s", err)
168+
169+
if len(opts) != 1 {
170+
t.Errorf("expected bundle length 1. got: %d", len(opts))
171+
}
172+
})
173+
157174
t.Run("MakeOptions", func(t *testing.T) {
158175
head := bundle1
159176

0 commit comments

Comments
 (0)