Skip to content

Commit 7493937

Browse files
committed
fix: lazyBoundedReadCloser correctly close/seek
Signed-off-by: Keith Zantow <[email protected]>
1 parent 344e29f commit 7493937

File tree

2 files changed

+26
-13
lines changed

2 files changed

+26
-13
lines changed

pkg/file/lazy_bounded_read_closer.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,15 @@ import (
44
"errors"
55
"io"
66
"os"
7+
8+
"github.com/anchore/stereoscope/internal/log"
79
)
810

9-
var _ io.ReadCloser = (*lazyBoundedReadCloser)(nil)
10-
var _ io.ReaderAt = (*lazyBoundedReadCloser)(nil)
11-
var _ io.Seeker = (*lazyBoundedReadCloser)(nil)
11+
var _ interface {
12+
io.ReadCloser
13+
io.ReaderAt
14+
io.Seeker
15+
} = (*lazyBoundedReadCloser)(nil)
1216

1317
// lazyBoundedReadCloser is a "lazy" read closer, allocating a file descriptor for the given path only upon the first Read() call.
1418
// Only part of the file is allowed to be read, starting at a given position.
@@ -40,10 +44,10 @@ func (d *lazyBoundedReadCloser) Read(b []byte) (int, error) {
4044

4145
n, err := d.reader.Read(b)
4246
if err != nil && errors.Is(err, io.EOF) {
43-
// we've reached the end of the file, force a release of the file descriptor. If the file has already been
44-
// closed, ignore the error.
45-
if closeErr := d.file.Close(); !errors.Is(closeErr, os.ErrClosed) {
46-
return n, closeErr
47+
// we've reached the end of the file, release of the file descriptor. continue to return EOF
48+
// IMPORTANT: call d.Close instead of d.reader.Close to reset internal state
49+
if closeErr := d.Close(); closeErr != nil {
50+
log.Tracef("unable to close: %v: %v", d.path, closeErr)
4751
}
4852
}
4953
return n, err
@@ -80,10 +84,10 @@ func (d *lazyBoundedReadCloser) ReadAt(b []byte, off int64) (n int, err error) {
8084

8185
n, err = d.reader.ReadAt(b, off)
8286
if err != nil && errors.Is(err, io.EOF) {
83-
// we've reached the end of the file, force a release of the file descriptor. If the file has already been
84-
// closed, ignore the error.
85-
if closeErr := d.file.Close(); !errors.Is(closeErr, os.ErrClosed) {
86-
return n, closeErr
87+
// we've reached the end of the file, release of the file descriptor. continue to return EOF
88+
// IMPORTANT: call d.Close instead of d.reader.Close to reset internal state
89+
if closeErr := d.Close(); closeErr != nil {
90+
log.Tracef("unable to close: %v: %v", d.path, closeErr)
8791
}
8892
}
8993
return n, err

pkg/file/lazy_bounded_read_closer_test.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,16 @@ func TestDeferredPartialReadCloser(t *testing.T) {
2828
require.NoError(t, err)
2929

3030
require.Equal(t, contents, actualContents)
31-
require.NotNil(t, dReader.file)
31+
require.Nil(t, dReader.file) // file is closed at EOF
32+
33+
_, err = dReader.Seek(0, io.SeekStart)
34+
require.NoError(t, err)
35+
require.NotNil(t, dReader.file) // file is reopened
36+
37+
secondReadContents, err := io.ReadAll(dReader)
38+
require.NoError(t, err)
39+
require.Equal(t, contents, secondReadContents)
40+
require.Nil(t, dReader.file) // file is closed again at EOF
3241

3342
require.NoError(t, dReader.Close())
3443
require.Nil(t, dReader.file, "should not have a file, but we do somehow")
@@ -49,7 +58,7 @@ func TestDeferredPartialReadCloser_Seek(t *testing.T) {
4958
require.NoError(t, err)
5059

5160
require.Equal(t, content[int(off):], actualContent)
52-
require.NotNil(t, dReader.file)
61+
require.Nil(t, dReader.file) // file is closed at EOF
5362

5463
require.NoError(t, dReader.Close())
5564
require.Nil(t, dReader.file, "should not have a file, but we do somehow")

0 commit comments

Comments
 (0)