diff --git a/internal/file/test-fixtures/generate-zip-fixture.sh b/internal/file/test-fixtures/generate-zip-fixture-from-source-dir.sh similarity index 100% rename from internal/file/test-fixtures/generate-zip-fixture.sh rename to internal/file/test-fixtures/generate-zip-fixture-from-source-dir.sh diff --git a/internal/file/zip_file_helpers_test.go b/internal/file/zip_file_helpers_test.go index 78855a212..7bf89a08c 100644 --- a/internal/file/zip_file_helpers_test.go +++ b/internal/file/zip_file_helpers_test.go @@ -1,7 +1,6 @@ package file import ( - "fmt" "io/ioutil" "os" "os/exec" @@ -9,6 +8,8 @@ import ( "path/filepath" "syscall" "testing" + + "github.com/stretchr/testify/assert" ) var expectedZipArchiveEntries = []string{ @@ -18,35 +19,21 @@ var expectedZipArchiveEntries = []string{ "nested.zip", } -// fatalIfError calls the supplied function. If the function returns a non-nil error, t.Fatal(err) is called. -func fatalIfError(t *testing.T, fn func() error) { - t.Helper() - - if fn == nil { - return - } - - err := fn() - if err != nil { - t.Fatal(err) - } -} - // createZipArchive creates a new ZIP archive file at destinationArchivePath based on the directory found at // sourceDirPath. -func createZipArchive(t *testing.T, sourceDirPath, destinationArchivePath string) error { +func createZipArchive(t testing.TB, sourceDirPath, destinationArchivePath string) { t.Helper() cwd, err := os.Getwd() if err != nil { - return fmt.Errorf("unable to get cwd: %+v", err) + t.Fatalf("unable to get cwd: %+v", err) } - cmd := exec.Command("./generate-zip-fixture.sh", destinationArchivePath, path.Base(sourceDirPath)) + cmd := exec.Command("./generate-zip-fixture-from-source-dir.sh", destinationArchivePath, path.Base(sourceDirPath)) cmd.Dir = filepath.Join(cwd, "test-fixtures") if err := cmd.Start(); err != nil { - return fmt.Errorf("unable to start generate zip fixture script: %+v", err) + t.Fatalf("unable to start generate zip fixture script: %+v", err) } if err := cmd.Wait(); err != nil { @@ -59,61 +46,62 @@ func createZipArchive(t *testing.T, sourceDirPath, destinationArchivePath string // an ExitStatus() method with the same signature. if status, ok := exiterr.Sys().(syscall.WaitStatus); ok { if status.ExitStatus() != 0 { - return fmt.Errorf("failed to generate fixture: rc=%d", status.ExitStatus()) + t.Fatalf("failed to generate fixture: rc=%d", status.ExitStatus()) } } } else { - return fmt.Errorf("unable to get generate fixture script result: %+v", err) + t.Fatalf("unable to get generate fixture script result: %+v", err) } } - return nil +} + +func assertNoError(t testing.TB, fn func() error) func() { + return func() { + assert.NoError(t, fn()) + } } // setupZipFileTest encapsulates common test setup work for zip file tests. It returns a cleanup function, // which should be called (typically deferred) by the caller, the path of the created zip archive, and an error, // which should trigger a fatal test failure in the consuming test. The returned cleanup function will never be nil // (even if there's an error), and it should always be called. -func setupZipFileTest(t *testing.T, sourceDirPath string) (func() error, string, error) { +func setupZipFileTest(t testing.TB, sourceDirPath string) string { t.Helper() - // Keep track of any needed cleanup work as we go - var cleanupFns []func() error - cleanup := func(fns []func() error) func() error { - return func() error { - for _, fn := range fns { - err := fn() - if err != nil { - return err - } - } - - return nil - } - } - archivePrefix, err := ioutil.TempFile("", "syft-ziputil-archive-TEST-") if err != nil { - return cleanup(cleanupFns), "", fmt.Errorf("unable to create tempfile: %+v", err) + t.Fatalf("unable to create tempfile: %+v", err) } - cleanupFns = append(cleanupFns, func() error { return os.Remove(archivePrefix.Name()) }) + + t.Cleanup( + assertNoError(t, + func() error { + return os.Remove(archivePrefix.Name()) + }, + ), + ) destinationArchiveFilePath := archivePrefix.Name() + ".zip" t.Logf("archive path: %s", destinationArchiveFilePath) - err = createZipArchive(t, sourceDirPath, destinationArchiveFilePath) - cleanupFns = append(cleanupFns, func() error { return os.Remove(destinationArchiveFilePath) }) - if err != nil { - return cleanup(cleanupFns), "", err - } + createZipArchive(t, sourceDirPath, destinationArchiveFilePath) + + t.Cleanup( + assertNoError(t, + func() error { + return os.Remove(destinationArchiveFilePath) + }, + ), + ) cwd, err := os.Getwd() if err != nil { - return cleanup(cleanupFns), "", fmt.Errorf("unable to get cwd: %+v", err) + t.Fatalf("unable to get cwd: %+v", err) } t.Logf("running from: %s", cwd) - return cleanup(cleanupFns), destinationArchiveFilePath, nil + return destinationArchiveFilePath } // TODO: Consider moving any non-git asset generation to a task (e.g. make) that's run ahead of running go tests. @@ -121,11 +109,7 @@ func ensureNestedZipExists(t *testing.T, sourceDirPath string) error { t.Helper() nestedArchiveFilePath := path.Join(sourceDirPath, "nested.zip") - err := createZipArchive(t, sourceDirPath, nestedArchiveFilePath) - - if err != nil { - return fmt.Errorf("unable to create nested archive for test fixture: %+v", err) - } + createZipArchive(t, sourceDirPath, nestedArchiveFilePath) return nil } diff --git a/internal/file/zip_file_manifest.go b/internal/file/zip_file_manifest.go index d7b7b80be..afe0435a8 100644 --- a/internal/file/zip_file_manifest.go +++ b/internal/file/zip_file_manifest.go @@ -1,23 +1,36 @@ package file import ( - "archive/zip" "fmt" "os" "sort" "strings" "github.com/anchore/syft/internal" - "github.com/anchore/syft/internal/log" ) // ZipFileManifest is a collection of paths and their file metadata. type ZipFileManifest map[string]os.FileInfo -// newZipManifest creates an empty ZipFileManifest. -func newZipManifest() ZipFileManifest { - return make(ZipFileManifest) +// NewZipFileManifest creates and returns a new ZipFileManifest populated with path and metadata from the given zip archive path. +func NewZipFileManifest(archivePath string) (ZipFileManifest, error) { + zipReader, err := OpenZip(archivePath) + manifest := make(ZipFileManifest) + if err != nil { + return manifest, fmt.Errorf("unable to open zip archive (%s): %w", archivePath, err) + } + defer func() { + err = zipReader.Close() + if err != nil { + log.Errorf("unable to close zip archive (%s): %+v", archivePath, err) + } + }() + + for _, file := range zipReader.Reader.File { + manifest.Add(file.Name, file.FileInfo()) + } + return manifest, nil } // Add a new path and it's file metadata to the collection. @@ -47,26 +60,6 @@ func (z ZipFileManifest) GlobMatch(patterns ...string) []string { return results } -// NewZipFileManifest creates and returns a new ZipFileManifest populated with path and metadata from the given zip archive path. -func NewZipFileManifest(archivePath string) (ZipFileManifest, error) { - zipReader, err := zip.OpenReader(archivePath) - manifest := newZipManifest() - if err != nil { - return manifest, fmt.Errorf("unable to open zip archive (%s): %w", archivePath, err) - } - defer func() { - err = zipReader.Close() - if err != nil { - log.Errorf("unable to close zip archive (%s): %+v", archivePath, err) - } - }() - - for _, file := range zipReader.Reader.File { - manifest.Add(file.Name, file.FileInfo()) - } - return manifest, nil -} - // normalizeZipEntryName takes the given path entry and ensures it is prefixed with "/". func normalizeZipEntryName(entry string) string { if !strings.HasPrefix(entry, "/") { diff --git a/internal/file/zip_file_manifest_test.go b/internal/file/zip_file_manifest_test.go index 5596a5dbc..203605e51 100644 --- a/internal/file/zip_file_manifest_test.go +++ b/internal/file/zip_file_manifest_test.go @@ -19,11 +19,7 @@ func TestNewZipFileManifest(t *testing.T) { t.Fatal(err) } - cleanup, archiveFilePath, err := setupZipFileTest(t, sourceDirPath) - defer fatalIfError(t, cleanup) - if err != nil { - t.Fatal(err) - } + archiveFilePath := setupZipFileTest(t, sourceDirPath) actual, err := NewZipFileManifest(archiveFilePath) if err != nil { @@ -63,12 +59,7 @@ func TestZipFileManifest_GlobMatch(t *testing.T) { t.Fatal(err) } - cleanup, archiveFilePath, err := setupZipFileTest(t, sourceDirPath) - //goland:noinspection GoNilness - defer fatalIfError(t, cleanup) - if err != nil { - t.Fatal(err) - } + archiveFilePath := setupZipFileTest(t, sourceDirPath) z, err := NewZipFileManifest(archiveFilePath) if err != nil { diff --git a/internal/file/zip_file_traversal.go b/internal/file/zip_file_traversal.go index 9069035f4..f9bdae7e3 100644 --- a/internal/file/zip_file_traversal.go +++ b/internal/file/zip_file_traversal.go @@ -47,7 +47,7 @@ func newZipTraverseRequest(paths ...string) zipTraversalRequest { func TraverseFilesInZip(archivePath string, visitor func(*zip.File) error, paths ...string) error { request := newZipTraverseRequest(paths...) - zipReader, err := zip.OpenReader(archivePath) + zipReader, err := OpenZip(archivePath) if err != nil { return fmt.Errorf("unable to open zip archive (%s): %w", archivePath, err) } diff --git a/internal/file/zip_file_traversal_test.go b/internal/file/zip_file_traversal_test.go index f65e1230c..98d08a977 100644 --- a/internal/file/zip_file_traversal_test.go +++ b/internal/file/zip_file_traversal_test.go @@ -44,14 +44,12 @@ func TestUnzipToDir(t *testing.T) { goldenRootDir := filepath.Join(cwd, "test-fixtures") sourceDirPath := path.Join(goldenRootDir, "zip-source") - cleanup, archiveFilePath, err := setupZipFileTest(t, sourceDirPath) - defer fatalIfError(t, cleanup) - if err != nil { - t.Fatal(err) - } + archiveFilePath := setupZipFileTest(t, sourceDirPath) unzipDestinationDir, err := ioutil.TempDir("", "syft-ziputil-contents-TEST-") - defer os.RemoveAll(unzipDestinationDir) + t.Cleanup(assertNoError(t, func() error { + return os.RemoveAll(unzipDestinationDir) + })) if err != nil { t.Fatalf("unable to create tempdir: %+v", err) } @@ -127,41 +125,119 @@ func TestUnzipToDir(t *testing.T) { } func TestContentsFromZip(t *testing.T) { + tests := []struct { + name string + archivePrep func(tb testing.TB) string + }{ + { + name: "standard, non-nested zip", + archivePrep: prepZipSourceFixture, + }, + { + name: "zip with prepended bytes", + archivePrep: prepZipSourceFixtureWithPrependedBytes(t, "junk at the beginning of the file..."), + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + archivePath := test.archivePrep(t) + expected := zipSourceFixtureExpectedContents() + + var paths []string + for p := range expected { + paths = append(paths, p) + } + + actual, err := ContentsFromZip(archivePath, paths...) + if err != nil { + t.Fatalf("unable to extract from unzip archive: %+v", err) + } + + assertZipSourceFixtureContents(t, actual, expected) + }) + } +} + +func prepZipSourceFixtureWithPrependedBytes(tb testing.TB, value string) func(tb testing.TB) string { + if len(value) == 0 { + tb.Fatalf("no bytes given to prefix") + } + return func(t testing.TB) string { + archivePath := prepZipSourceFixture(t) + + // create a temp file + tmpFile, err := ioutil.TempFile("", "syft-ziputil-archive-TEST-") + if err != nil { + t.Fatalf("unable to create tempfile: %+v", err) + } + defer tmpFile.Close() + + // write junk to the temp file + if _, err := tmpFile.WriteString(value); err != nil { + t.Fatalf("unable to write to tempfile: %+v", err) + } + + // open the original archive + sourceFile, err := os.Open(archivePath) + if err != nil { + t.Fatalf("unable to read source file: %+v", err) + } + + // copy all contents from the archive to the temp file + if _, err := io.Copy(tmpFile, sourceFile); err != nil { + t.Fatalf("unable to copy source to dest: %+v", err) + } + + sourceFile.Close() + + // remove the original archive and replace it with the temp file + if err := os.Remove(archivePath); err != nil { + t.Fatalf("unable to remove original source archive") + } + + if err := os.Rename(tmpFile.Name(), archivePath); err != nil { + t.Fatalf("unable to move new archive to old path") + } + + return archivePath + } +} + +func prepZipSourceFixture(t testing.TB) string { + t.Helper() archivePrefix, err := ioutil.TempFile("", "syft-ziputil-archive-TEST-") if err != nil { t.Fatalf("unable to create tempfile: %+v", err) } - defer os.Remove(archivePrefix.Name()) + + t.Cleanup(func() { + assert.NoError(t, os.Remove(archivePrefix.Name())) + }) + // the zip utility will add ".zip" to the end of the given name archivePath := archivePrefix.Name() + ".zip" - defer os.Remove(archivePath) + + t.Cleanup(func() { + assert.NoError(t, os.Remove(archivePath)) + }) + t.Logf("archive path: %s", archivePath) - err = createZipArchive(t, "zip-source", archivePrefix.Name()) - if err != nil { - t.Fatal(err) - } - - cwd, err := os.Getwd() - if err != nil { - t.Errorf("unable to get cwd: %+v", err) - } - - t.Logf("running from: %s", cwd) - - aFilePath := filepath.Join("some-dir", "a-file.txt") - bFilePath := filepath.Join("b-file.txt") - - expected := map[string]string{ - aFilePath: "A file! nice!", - bFilePath: "B file...", - } - - actual, err := ContentsFromZip(archivePath, aFilePath, bFilePath) - if err != nil { - t.Fatalf("unable to extract from unzip archive: %+v", err) + createZipArchive(t, "zip-source", archivePrefix.Name()) + + return archivePath +} + +func zipSourceFixtureExpectedContents() map[string]string { + return map[string]string{ + filepath.Join("some-dir", "a-file.txt"): "A file! nice!", + filepath.Join("b-file.txt"): "B file...", } +} +func assertZipSourceFixtureContents(t testing.TB, actual map[string]string, expected map[string]string) { + t.Helper() diffs := deep.Equal(actual, expected) if len(diffs) > 0 { for _, d := range diffs { diff --git a/internal/file/zip_read_closer_test.go b/internal/file/zip_read_closer_test.go new file mode 100644 index 000000000..5df0d6e26 --- /dev/null +++ b/internal/file/zip_read_closer_test.go @@ -0,0 +1,47 @@ +package file + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFindArchiveStartOffset(t *testing.T) { + tests := []struct { + name string + archivePrep func(tb testing.TB) string + expected uint64 + }{ + { + name: "standard, non-nested zip", + archivePrep: prepZipSourceFixture, + expected: 0, + }, + { + name: "zip with prepended bytes", + archivePrep: prepZipSourceFixtureWithPrependedBytes(t, "junk at the beginning of the file..."), + expected: 36, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + archivePath := test.archivePrep(t) + f, err := os.Open(archivePath) + if err != nil { + t.Fatalf("could not open archive %q: %+v", archivePath, err) + } + fi, err := os.Stat(f.Name()) + if err != nil { + t.Fatalf("unable to stat archive: %+v", err) + } + + actual, err := findArchiveStartOffset(f, fi.Size()) + if err != nil { + t.Fatalf("unable to find offset: %+v", err) + } + assert.Equal(t, test.expected, actual) + }) + } +}