diff --git a/blob/blob.go b/blob/blob.go index d64ed22fc..459344804 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -128,12 +128,11 @@ func (r *Reader) Read(p []byte) (int, error) { // to (SeekEnd, 0) and use the return value to determine the size // of the data, then Seek back to (SeekStart, 0). saved := r.savedOffset - r.savedOffset = -1 if r.relativeOffset == saved { // Nope! We're at the same place we left off. + r.savedOffset = -1 } else { // Yep! We've changed the offset. Recreate the reader. - _ = r.r.Close() length := r.baseLength if length >= 0 { length -= r.relativeOffset @@ -142,11 +141,13 @@ func (r *Reader) Read(p []byte) (int, error) { return 0, gcerr.Newf(gcerr.Internal, nil, "blob: invalid Seek (base length %d, relative offset %d)", r.baseLength, r.relativeOffset) } } - var err error - r.r, err = r.b.NewRangeReader(r.ctx, r.key, r.baseOffset+r.relativeOffset, length, r.dopts) + newR, err := r.b.NewRangeReader(r.ctx, r.key, r.baseOffset+r.relativeOffset, length, r.dopts) if err != nil { return 0, wrapError(r.b, err, r.key) } + _ = r.r.Close() + r.savedOffset = -1 + r.r = newR } } n, err := r.r.Read(p) diff --git a/blob/blob_test.go b/blob/blob_test.go index 22bb8efee..97ca67ea5 100644 --- a/blob/blob_test.go +++ b/blob/blob_test.go @@ -265,6 +265,74 @@ func TestDownloader(t *testing.T) { } } +func TestSeekAfterReadFailure(t *testing.T) { + const filename = "f.txt" + + ctx := context.Background() + + bucket := NewBucket(&oneTimeReadBucket{first: true}) + defer bucket.Close() + + reader, err := bucket.NewRangeReader(ctx, filename, 0, 100, nil) + if err != nil { + t.Fatalf("failed NewRangeReader: %v", err) + } + defer reader.Close() + + b := make([]byte, 10) + + _, err = reader.Read(b) + if err != nil { + t.Fatalf("failed Read#1: %v", err) + } + + _, err = reader.Seek(0, io.SeekStart) + if err != nil { + t.Fatalf("failed Seek#1: %v", err) + } + + // This Read will force a recreation of the reader via NewRangeReader, + // which will fail. + _, err = reader.Read(b) + if err == nil { + t.Fatalf("unexpectedly succeeded Read#2: %v", err) + } + + _, err = reader.Seek(0, io.SeekStart) + if err != nil { + t.Fatalf("failed Seek#2: %v", err) + } +} + +// oneTimeReadBucket implements driver.Bucket for TestSeekAfterReadFailure. +// It returns a fake reader that succeeds once, then fails. +type oneTimeReadBucket struct { + driver.Bucket + first bool +} + +type workingReader struct { + driver.Reader +} + +func (r *workingReader) Read(p []byte) (int, error) { + return len(p), nil +} + +func (r *workingReader) Attributes() *driver.ReaderAttributes { return &driver.ReaderAttributes{} } +func (r *workingReader) Close() error { return nil } + +func (b *oneTimeReadBucket) NewRangeReader(ctx context.Context, key string, offset, length int64, opts *driver.ReaderOptions) (driver.Reader, error) { + if b.first { + b.first = false + return &workingReader{}, nil + } + return nil, errFake +} + +func (b *oneTimeReadBucket) ErrorCode(err error) gcerrors.ErrorCode { return gcerrors.Unknown } +func (b *oneTimeReadBucket) Close() error { return nil } + // erroringBucket implements driver.Bucket. All interface methods that return // errors are implemented, and return errFake. // In addition, when passed the key "work", NewRangeReader and NewTypedWriter