datastore: add error checking to Query.nextBatch
Currently, nextBatch will panic if the given query is somehow invalid.
This can happen when Filter is invoked with an invalid type:
```
type MyType int
var v MyType = 1
q := datastore.NewQuery("MyKind").Filter("fld=", v)
n, err := c.Count(ctx, q)
```
This happens because the type validation happens while running the query
and converting the query to protobuf (via toProto). If this fails, the
iterator will set an error, however nextBatch is not checking the error.
Therefore the iterator is in an invalid state and certain fields are
nil. When the user invokes Next again, the call panics (in nextBatch).
This CL fixes this problem by having nextBatch check the iterator's err
field.
Fixes #1306
Change-Id: I8cfe48a17258a1ab4a18580230bffc51e3b2c75c
Reviewed-on: https://code-review.googlesource.com/c/38170
Reviewed-by: Jean de Klerk <deklerk@google.com>
diff --git a/datastore/query.go b/datastore/query.go
index 19576cd..e41c222 100644
--- a/datastore/query.go
+++ b/datastore/query.go
@@ -670,6 +670,10 @@
// nextBatch makes a single call to the server for a batch of results.
func (t *Iterator) nextBatch() error {
+ if t.err != nil {
+ return t.err
+ }
+
if t.limit == 0 {
return iterator.Done // Short-circuits the zero-item response.
}
diff --git a/datastore/query_test.go b/datastore/query_test.go
index 1806065..c28723a 100644
--- a/datastore/query_test.go
+++ b/datastore/query_test.go
@@ -545,3 +545,26 @@
}
}
}
+
+func TestInvalidFilters(t *testing.T) {
+ client := &Client{
+ client: &fakeClient{
+ queryFn: func(req *pb.RunQueryRequest) (*pb.RunQueryResponse, error) {
+ return fakeRunQuery(req)
+ },
+ },
+ }
+
+ // Used for an invalid type
+ type MyType int
+ var v MyType = 1
+
+ for _, q := range []*Query{
+ NewQuery("SomeKey").Filter("", 0),
+ NewQuery("SomeKey").Filter("fld=", v),
+ } {
+ if _, err := client.Count(context.Background(), q); err == nil {
+ t.Errorf("%+v: got nil, wanted error", q)
+ }
+ }
+}
