Skip to content

Commit 5288511

Browse files
authored
fix(bigquery): address possible panic due to offset checking in handleInsertErrors (#3524)
* fix: address possible panic due to offset checking in handleInsertErrors External reporter identified case where improper bounds checking can cause panic when comparing the index value from structured error response. Fixes: #3519
1 parent eee3d54 commit 5288511

File tree

2 files changed

+34
-11
lines changed

2 files changed

+34
-11
lines changed

Diff for: bigquery/inserter.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ func handleInsertErrors(ierrs []*bq.TableDataInsertAllResponseInsertErrors, rows
229229
}
230230
var errs PutMultiError
231231
for _, e := range ierrs {
232-
if int(e.Index) > len(rows) {
232+
if int(e.Index) >= len(rows) {
233233
return fmt.Errorf("internal error: unexpected row index: %v", e.Index)
234234
}
235235
rie := RowInsertionError{

Diff for: bigquery/inserter_test.go

+33-10
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package bigquery
1616

1717
import (
1818
"errors"
19+
"fmt"
1920
"strconv"
2021
"testing"
2122

@@ -113,22 +114,27 @@ func TestHandleInsertErrors(t *testing.T) {
113114
{InsertId: "b"},
114115
}
115116
for _, test := range []struct {
116-
in []*bq.TableDataInsertAllResponseInsertErrors
117-
want error
117+
description string
118+
in []*bq.TableDataInsertAllResponseInsertErrors
119+
want error
118120
}{
119121
{
120-
in: nil,
121-
want: nil,
122+
description: "nil error",
123+
in: nil,
124+
want: nil,
122125
},
123126
{
124-
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
125-
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
127+
description: "single error last row",
128+
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
129+
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
126130
},
127131
{
128-
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
129-
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
132+
description: "single error first row",
133+
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 0}},
134+
want: PutMultiError{RowInsertionError{InsertID: "a", RowIndex: 0}},
130135
},
131136
{
137+
description: "errors with extended message",
132138
in: []*bq.TableDataInsertAllResponseInsertErrors{
133139
{Errors: []*bq.ErrorProto{{Message: "m0"}}, Index: 0},
134140
{Errors: []*bq.ErrorProto{{Message: "m1"}}, Index: 1},
@@ -138,11 +144,28 @@ func TestHandleInsertErrors(t *testing.T) {
138144
RowInsertionError{InsertID: "b", RowIndex: 1, Errors: []error{&Error{Message: "m1"}}},
139145
},
140146
},
147+
{
148+
description: "invalid index",
149+
in: []*bq.TableDataInsertAllResponseInsertErrors{
150+
{Errors: []*bq.ErrorProto{{Message: "m0"}}, Index: 2},
151+
},
152+
want: fmt.Errorf("internal error: unexpected row index: 2"),
153+
},
141154
} {
142155
got := handleInsertErrors(test.in, rows)
143-
if !testutil.Equal(got, test.want) {
144-
t.Errorf("%#v:\ngot\n%#v\nwant\n%#v", test.in, got, test.want)
156+
if _, ok := got.(PutMultiError); ok {
157+
// compare structure of the PutMultiError
158+
if !testutil.Equal(got, test.want) {
159+
t.Errorf("(case: %s)\nin %#v\ngot\n%#v\nwant\n%#v", test.description, test.in, got, test.want)
160+
}
161+
continue
162+
}
163+
164+
if got != nil && test.want != nil && got.Error() != test.want.Error() {
165+
// check matching error messages
166+
t.Errorf("(case: %s)\nin %#v:\ngot\n%#v\nwant\n%#v", test.description, test.in, got, test.want)
145167
}
168+
146169
}
147170
}
148171

0 commit comments

Comments
 (0)