From da352a2a1b1f1aa99ec1cb46ad82012deeb0c5e9 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 2 Sep 2026 15:41:51 +0100 Subject: [PATCH] archive: fix zip file entries named for a directory causing confusion A file entry in a zip whose name refers to a directory, such as ".", "/", "" or "sub/.", was only skipped when it named the root of an archive which was itself the root of the remote. When the archive was found by listing its parent directory the entry appeared as a file with the same name as the archive alongside the directory for it, and copying the archive tried to write both. When the entry named a subdirectory it appeared as a file alongside that directory, and with that subdirectory mounted as the archive root the entry was taken to be the single file the root points at, hiding every real entry. Skip any file entry whose last path component is "", "." or "..", checked on the raw name before it is cleaned or joined on the prefix. --- backend/archive/zip/zip.go | 18 ++++++------- backend/archive/zip/zip_internal_test.go | 33 +++++++++++++++++------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/backend/archive/zip/zip.go b/backend/archive/zip/zip.go index 6349807b1..129ce010e 100644 --- a/backend/archive/zip/zip.go +++ b/backend/archive/zip/zip.go @@ -145,6 +145,13 @@ func (f *Fs) readZip() (singleObject bool, err error) { skipped++ continue } + isDir := strings.HasSuffix(file.Name, "/") + // A file entry whose last component is "", "." or ".." names + // a directory rather than a file, so skip + if !isDir && sanitize.Leaf(path.Base(file.Name)) != nil { + skipped++ + continue + } remote = path.Join(f.prefix, remote) if f.root != "" { // Ignore all files outside the root, requiring a path @@ -159,19 +166,12 @@ func (f *Fs) readZip() (singleObject bool, err error) { remote = strings.TrimPrefix(remote, f.root+"/") } } - if strings.HasSuffix(file.Name, "/") { + if isDir { dir := fs.NewDir(remote, file.Modified) dt.AddDir(dir) } else { if remote == "" { - // A file at the root itself can only be the - // archive member f.root points at - with no root - // it is a crafted name for the archive's own - // directory, which can't be a file - if f.root == "" { - skipped++ - continue - } + // A file at the root is the archive member f.root points at remote = path.Base(f.root) singleObject = true dt = dirtree.New() diff --git a/backend/archive/zip/zip_internal_test.go b/backend/archive/zip/zip_internal_test.go index d13bb9140..6aff03cd5 100644 --- a/backend/archive/zip/zip_internal_test.go +++ b/backend/archive/zip/zip_internal_test.go @@ -4,6 +4,7 @@ import ( "archive/zip" "bytes" "context" + "fmt" "os" "path/filepath" "sort" @@ -105,10 +106,12 @@ func TestReadZipRootBoundary(t *testing.T) { assert.Equal(t, []string{"a.txt"}, remotes) } -// A file entry whose name refers to the archive's own root (".", "/", -// "./" or "") must be skipped, not turn the whole archive into a single -// file which hides every other entry. -func TestReadZipRootNamedEntry(t *testing.T) { +// A file entry whose name refers to a directory (".", "/", "./", "" or +// "sub/.") must be skipped, not turn the archive or the directory +// mounted as its root into a single file which hides every other +// entry, or become an object named for the archive itself when the +// archive is found by listing its directory. +func TestReadZipDirNamedFileEntry(t *testing.T) { ctx := context.Background() dir := t.TempDir() name := writeZip(t, dir, "dot.zip", @@ -117,16 +120,28 @@ func TestReadZipRootNamedEntry(t *testing.T) { "./", "", "good.txt", + "sub/", + "sub/.", + "sub/good.txt", ) localFs, err := cache.Get(ctx, dir) require.NoError(t, err) - f, err := New(ctx, localFs, name, "", "") - require.NoError(t, err) - - remotes := allRemotes(t, f) - assert.Equal(t, []string{"good.txt"}, remotes) + for _, test := range []struct { + prefix, root string + want []string + }{ + {"", "", []string{"good.txt", "sub/good.txt"}}, + {"sub/dot.zip", "", []string{"sub/dot.zip/good.txt", "sub/dot.zip/sub/good.txt"}}, + {"", "sub", []string{"good.txt"}}, + } { + t.Run(fmt.Sprintf("prefix=%q,root=%q", test.prefix, test.root), func(t *testing.T) { + f, err := New(ctx, localFs, name, test.prefix, test.root) + require.NoError(t, err) + assert.Equal(t, test.want, allRemotes(t, f)) + }) + } } // Listings are served from a cache which must survive callers