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

fix(bigquery): address possible panic due to offset checking in handleInsertErrors #3524

Merged
merged 8 commits into from Jan 12, 2021
2 changes: 1 addition & 1 deletion bigquery/inserter.go
Expand Up @@ -229,7 +229,7 @@ func handleInsertErrors(ierrs []*bq.TableDataInsertAllResponseInsertErrors, rows
}
var errs PutMultiError
for _, e := range ierrs {
if int(e.Index) > len(rows) {
if int(e.Index) >= len(rows) {
return fmt.Errorf("internal error: unexpected row index: %v", e.Index)
}
rie := RowInsertionError{
Expand Down
45 changes: 35 additions & 10 deletions bigquery/inserter_test.go
Expand Up @@ -16,6 +16,7 @@ package bigquery

import (
"errors"
"fmt"
"strconv"
"testing"

Expand Down Expand Up @@ -113,22 +114,27 @@ func TestHandleInsertErrors(t *testing.T) {
{InsertId: "b"},
}
for _, test := range []struct {
in []*bq.TableDataInsertAllResponseInsertErrors
want error
description string
in []*bq.TableDataInsertAllResponseInsertErrors
want error
}{
{
in: nil,
want: nil,
description: "nil error",
in: nil,
want: nil,
},
{
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
description: "single error last row",
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
},
{
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 1}},
want: PutMultiError{RowInsertionError{InsertID: "b", RowIndex: 1}},
description: "single error first row",
in: []*bq.TableDataInsertAllResponseInsertErrors{{Index: 0}},
want: PutMultiError{RowInsertionError{InsertID: "a", RowIndex: 0}},
},
{
description: "errors with extended message",
in: []*bq.TableDataInsertAllResponseInsertErrors{
{Errors: []*bq.ErrorProto{{Message: "m0"}}, Index: 0},
{Errors: []*bq.ErrorProto{{Message: "m1"}}, Index: 1},
Expand All @@ -138,10 +144,29 @@ func TestHandleInsertErrors(t *testing.T) {
RowInsertionError{InsertID: "b", RowIndex: 1, Errors: []error{&Error{Message: "m1"}}},
},
},
{
description: "invalid index",
in: []*bq.TableDataInsertAllResponseInsertErrors{
{Errors: []*bq.ErrorProto{{Message: "m0"}}, Index: 2},
},
want: fmt.Errorf("internal error: unexpected row index: 2"),
},
} {
got := handleInsertErrors(test.in, rows)
if !testutil.Equal(got, test.want) {
t.Errorf("%#v:\ngot\n%#v\nwant\n%#v", test.in, got, test.want)
_, ok := got.(PutMultiError)
if ok {
shollyman marked this conversation as resolved.
Show resolved Hide resolved
// compare structure of the PutMultiError
if !testutil.Equal(got, test.want) {
t.Errorf("(case: %s)\nin %#v\ngot\n%#v\nwant\n%#v", test.description, test.in, got, test.want)
}
} else {
shollyman marked this conversation as resolved.
Show resolved Hide resolved
if got != nil && test.want != nil && got.Error() != test.want.Error() {
// check matching error messages
t.Errorf("(case: %s)\nin %#v:\ngot\n%#v\nwant\n%#v", test.description, test.in, got, test.want)
} else {
// mismatched nils
t.Errorf("(case: %s)\nin %#v:\ngot\n%#v\nwant\n%#v", test.description, test.in, got, test.want)
}
}
}
}
Expand Down