From d3310a18300cc5e89fa560f3e3cee032c6284b49 Mon Sep 17 00:00:00 2001 From: William Murphy Date: Fri, 26 Apr 2024 10:21:38 -0400 Subject: [PATCH] fix: re-use embedded union reader if possible (#2814) * fix: re-use embedded union reader if possible Previously, because file.LocationReadCloser embeds a ReadCloser that might be a UnionReader, but doesn't implement the interface itself, the type assertion would fall and Syft would fall back to io.ReadAll to enable seeking on the underlying reader, resulting in a potentially large extra allocation. Instead, check whether the passed ReadCloser is a file.LocationReadCloser, and if so, try to use the embedded ReadCloser as a UnionReader. Signed-off-by: Will Murphy * lint fix Signed-off-by: Will Murphy * Assert that underlying reader is returned Signed-off-by: Will Murphy --------- Signed-off-by: Will Murphy --- syft/internal/unionreader/union_reader.go | 12 +++++++ .../internal/unionreader/union_reader_test.go | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/syft/internal/unionreader/union_reader.go b/syft/internal/unionreader/union_reader.go index 86a493ad0..5f2a2938f 100644 --- a/syft/internal/unionreader/union_reader.go +++ b/syft/internal/unionreader/union_reader.go @@ -7,6 +7,7 @@ import ( macho "github.com/anchore/go-macholibre" "github.com/anchore/syft/internal/log" + "github.com/anchore/syft/syft/file" ) // UnionReader is a single interface with all reading functions needed by multi-arch binary catalogers @@ -44,6 +45,17 @@ func GetUnionReader(readerCloser io.ReadCloser) (UnionReader, error) { return reader, nil } + // file.LocationReadCloser embeds a ReadCloser, which is likely + // to implement UnionReader. Check whether the embedded read closer + // implements UnionReader, and just return that if so. + r, ok := readerCloser.(file.LocationReadCloser) + if ok { + ur, ok := r.ReadCloser.(UnionReader) + if ok { + return ur, nil + } + } + b, err := io.ReadAll(readerCloser) if err != nil { return nil, fmt.Errorf("unable to read contents from binary: %w", err) diff --git a/syft/internal/unionreader/union_reader_test.go b/syft/internal/unionreader/union_reader_test.go index 15c67459a..f4353f1a9 100644 --- a/syft/internal/unionreader/union_reader_test.go +++ b/syft/internal/unionreader/union_reader_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/anchore/syft/syft/file" ) func Test_getUnionReader_notUnionReader(t *testing.T) { @@ -28,3 +30,34 @@ func Test_getUnionReader_notUnionReader(t *testing.T) { assert.Equal(t, expectedContents, string(b)) } + +type panickingUnionReader struct{} + +func (p2 *panickingUnionReader) ReadAt(p []byte, off int64) (n int, err error) { + panic("don't call this in your unit test!") +} + +func (p2 *panickingUnionReader) Seek(offset int64, whence int) (int64, error) { + panic("don't call this in your unit test!") +} + +func (p2 *panickingUnionReader) Read(p []byte) (n int, err error) { + panic("don't call this in your unit test!") +} + +func (p2 *panickingUnionReader) Close() error { + panic("don't call this in your unit test!") +} + +var _ UnionReader = (*panickingUnionReader)(nil) + +func Test_getUnionReader_fileLocationReadCloser(t *testing.T) { + // panickingUnionReader is a UnionReader + p := &panickingUnionReader{} + embedsUnionReader := file.NewLocationReadCloser(file.Location{}, p) + + // embedded union reader is returned without "ReadAll" invocation + ur, err := GetUnionReader(embedsUnionReader) + require.NoError(t, err) + require.Equal(t, p, ur) +}