diff --git a/internal/archive/README.md b/internal/archive/README.md index ef7d9dbaf..c58df8d27 100644 --- a/internal/archive/README.md +++ b/internal/archive/README.md @@ -67,8 +67,8 @@ PATH_MAX are refused, and nesting stops at the task's Entries are indexed in a stereoscope `filetree.FileTree` + `filetree.Index` and searched with `filetree.SearchContext`, the same index the directory and image resolvers use. Paths are archive-relative with no leading slash; a leading slash on a query is tolerated. Directories implied -by entry paths exist for `HasPath` but are never files. The first entry at a path wins over later -duplicates. +by entry paths exist for `HasPath` but are never files. The last entry at a path wins over earlier +duplicates, as it does on extraction; a file and a directory at one path keep the first. Links resolve inside the archive only, as the file tree resolves them: a hard link names its target from the archive root. A lookup matching a link and its target returns one location, with diff --git a/internal/archive/resolver.go b/internal/archive/resolver.go index 5f0cd9286..fbd499937 100644 --- a/internal/archive/resolver.go +++ b/internal/archive/resolver.go @@ -99,13 +99,17 @@ func approxIndexBytes(hdr tar.Header) int64 { // add stores and indexes one entry, reading its content in full now since archive readers are // sequential. Reaching a limit stores nothing for this entry and returns ErrDiskLimitReached; entries -// added before it remain usable. The first entry at a path wins. +// added before it remain usable. The last entry at a path wins, as it does when the archive is +// extracted (tar appends updates, an OCI layer applies in order, and java's legacy zip reading +// overwrites too), so a stale earlier copy, such as an older pom.properties, can't stand in for what is +// really there. func (r *Resolver) add(hdr tar.Header, content io.Reader) error { entryPath := path.Clean("/" + hdr.Name) if entryPath == "/" { return nil } - if exists, res, _ := r.tree.File(stereoscopeFile.Path(entryPath)); exists && res.HasReference() { + replacing, ok := r.replace(entryPath, hdr) + if !ok { return nil } @@ -158,12 +162,37 @@ func (r *Resolver) add(hdr tar.Header, content io.Reader) error { log.WithFields("entry", hdr.Name, "error", err).Trace("skipping archive entry") return nil } - if hdr.Typeflag != tar.TypeDir { + if hdr.Typeflag != tar.TypeDir && !replacing { r.entries++ } return nil } +// replace clears the way for an entry at a path that is already held, reporting whether it did and +// false when the entry should be dropped. A later non-directory entry replaces an earlier one, refunding +// what it held. A file and a directory at one path keep the first, since swapping kinds would mean +// re-parenting what is already beneath it. +func (r *Resolver) replace(entryPath string, hdr tar.Header) (replacing, ok bool) { + exists, res, _ := r.tree.File(stereoscopeFile.Path(entryPath)) + if !exists || !res.HasReference() { + return false, true + } + earlier, err := r.index.Get(*res.Reference) + if err != nil || earlier.IsDir() || hdr.Typeflag == tar.TypeDir { + return false, false + } + // a removed node's index entry is left behind, but searches only answer with references in the tree + if err := r.tree.RemovePath(stereoscopeFile.Path(entryPath)); err != nil { + log.WithFields("entry", hdr.Name, "error", err).Trace("unable to replace archive entry") + return false, false + } + if b := r.content[res.ID()]; b != nil { + r.discard(b) + } + delete(r.content, res.ID()) + return true, true +} + func (r *Resolver) store(rooted string, hdr tar.Header, b *blob) error { ref, err := r.addToTree(rooted, hdr) if err != nil { diff --git a/internal/archive/resolver_test.go b/internal/archive/resolver_test.go index 33299df36..e39331f4f 100644 --- a/internal/archive/resolver_test.go +++ b/internal/archive/resolver_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + stereoscopeFile "github.com/anchore/stereoscope/pkg/file" "github.com/anchore/syft/syft/file" ) @@ -83,15 +84,40 @@ func TestResolver_add_indexMakesRoomBySpillingHeldContent(t *testing.T) { assert.Equal(t, strings.Repeat("a", 100), readStore(t, r)["a.txt"].body) } -func TestResolver_add_firstEntryAtAPathWins(t *testing.T) { +func TestResolver_add_lastEntryAtAPathWins(t *testing.T) { r, _ := resolverIn(t, memCharge(1<<20)) require.NoError(t, r.add(regularHeader("a.txt", 5), bytes.NewReader([]byte("first")))) require.NoError(t, r.add(regularHeader("a.txt", 6), bytes.NewReader([]byte("second")))) + require.NoError(t, r.add(regularHeader("b.txt", 1), bytes.NewReader([]byte("b")))) + require.NoError(t, r.add(tar.Header{Name: "b.txt", Typeflag: tar.TypeSymlink, Linkname: "a.txt"}, nil)) r.finish() - assert.Equal(t, "first", readStore(t, r)["a.txt"].body) - assert.Equal(t, 1, r.entries) + assert.Equal(t, "second", readStore(t, r)["a.txt"].body) + assert.Equal(t, "a.txt", readStore(t, r)["b.txt"].linkTarget, "a later entry may change kind") + assert.Equal(t, 2, r.entries) + assert.Equal(t, int64(len("second")), r.heldInMemory(), "the earlier copy is refunded") + + locations, err := r.FilesByGlob("**/a.txt") + require.NoError(t, err) + require.Len(t, locations, 1) + reader, err := r.FileContentsByLocation(locations[0]) + require.NoError(t, err) + assert.Equal(t, "second", readEntry(t, reader.(entryReader).ReaderAtSeeker)) +} + +func TestResolver_add_fileAndDirectoryAtOnePathKeepsTheFirst(t *testing.T) { + r, _ := resolverIn(t, memCharge(1<<20)) + + require.NoError(t, r.add(regularHeader("a/b", 1), bytes.NewReader([]byte("x")))) + require.NoError(t, r.add(regularHeader("a", 4), bytes.NewReader([]byte("file")))) + require.NoError(t, r.add(regularHeader("c", 4), bytes.NewReader([]byte("file")))) + require.NoError(t, r.add(tar.Header{Name: "c", Typeflag: tar.TypeDir, Mode: 0o755}, nil)) + r.finish() + + got := readStore(t, r) + assert.Equal(t, stereoscopeFile.TypeDirectory, got["a"].kind) + assert.Equal(t, "file", got["c"].body) } func TestResolver_pathsAndContents(t *testing.T) {