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

@@ -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()

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