Merge branch 'fix-zip-directories' into 'master'

Fix support for archives without directory structure

Closes #482

See merge request gitlab-org/gitlab-pages!373

2 files changed, 80 insertions, 21 deletions

diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go
index 548ba651..ba15af20 100644
--- a/internal/vfs/zip/archive.go
+++ b/internal/vfs/zip/archive.go
@@ -53,7 +53,8 @@ type zipArchive struct {
 	archive  *zip.Reader
 	err      error
 
-	files map[string]*zip.File
+	files       map[string]*zip.File
+	directories map[string]*zip.FileHeader
 }
 
 func newArchive(fs *zipVFS, path string, openTimeout time.Duration) *zipArchive {
@@ -62,6 +63,7 @@ func newArchive(fs *zipVFS, path string, openTimeout time.Duration) *zipArchive
 		path:           path,
 		done:           make(chan struct{}),
 		files:          make(map[string]*zip.File),
+		directories:    make(map[string]*zip.FileHeader),
 		openTimeout:    openTimeout,
 		cacheNamespace: strconv.FormatInt(atomic.AddInt64(&fs.archiveCount, 1), 10) + ":",
 	}
@@ -133,7 +135,14 @@ func (a *zipArchive) readArchive() {
 		if !strings.HasPrefix(file.Name, dirPrefix) {
 			continue
 		}
-		a.files[file.Name] = file
+
+		if file.Mode().IsDir() {
+			a.directories[file.Name] = &file.FileHeader
+		} else {
+			a.files[file.Name] = file
+		}
+
+		a.addPathDirectory(file.Name)
 	}
 
 	// recycle memory
@@ -145,6 +154,23 @@ func (a *zipArchive) readArchive() {
 	metrics.ZipArchiveEntriesCached.Add(fileCount)
 }
 
+// addPathDirectory adds a directory for a given path
+func (a *zipArchive) addPathDirectory(path string) {
+	// Split dir and file from `path`
+	path, _ = filepath.Split(path)
+	if path == "" {
+		return
+	}
+
+	if a.directories[path] != nil {
+		return
+	}
+
+	a.directories[path] = &zip.FileHeader{
+		Name: path,
+	}
+}
+
 func (a *zipArchive) findFile(name string) *zip.File {
 	name = filepath.Join(dirPrefix, name)
 
@@ -152,17 +178,22 @@ func (a *zipArchive) findFile(name string) *zip.File {
 		return file
 	}
 
-	if dir := a.files[name+"/"]; dir != nil {
-		return dir
-	}
-
 	return nil
 }
 
+func (a *zipArchive) findDirectory(name string) *zip.FileHeader {
+	name = filepath.Join(dirPrefix, name)
+
+	return a.directories[name+"/"]
+}
+
 // Open finds the file by name inside the zipArchive and returns a reader that can be served by the VFS
 func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) {
 	file := a.findFile(name)
 	if file == nil {
+		if a.findDirectory(name) != nil {
+			return nil, errNotFile
+		}
 		return nil, os.ErrNotExist
 	}
 
@@ -193,17 +224,25 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) {
 // Lstat finds the file by name inside the zipArchive and returns its FileInfo
 func (a *zipArchive) Lstat(ctx context.Context, name string) (os.FileInfo, error) {
 	file := a.findFile(name)
-	if file == nil {
-		return nil, os.ErrNotExist
+	if file != nil {
+		return file.FileInfo(), nil
 	}
 
-	return file.FileInfo(), nil
+	directory := a.findDirectory(name)
+	if directory != nil {
+		return directory.FileInfo(), nil
+	}
+
+	return nil, os.ErrNotExist
 }
 
 // ReadLink finds the file by name inside the zipArchive and returns the contents of the symlink
 func (a *zipArchive) Readlink(ctx context.Context, name string) (string, error) {
 	file := a.findFile(name)
 	if file == nil {
+		if a.findDirectory(name) != nil {
+			return "", errNotSymlink
+		}
 		return "", os.ErrNotExist
 	}
 
diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go
index bd7627b1..ef6785b5 100644
--- a/internal/vfs/zip/archive_test.go
+++ b/internal/vfs/zip/archive_test.go
@@ -102,24 +102,44 @@ func TestLstat(t *testing.T) {
 	defer cleanup()
 
 	tests := map[string]struct {
-		file        string
-		isDir       bool
-		isSymlink   bool
-		expectedErr error
+		file         string
+		isDir        bool
+		isSymlink    bool
+		expectedName string
+		expectedErr  error
 	}{
 		"file_exists": {
-			file: "index.html",
+			file:         "index.html",
+			expectedName: "index.html",
 		},
 		"file_exists_in_subdir": {
-			file: "subdir/hello.html",
+			file:         "subdir/hello.html",
+			expectedName: "hello.html",
 		},
 		"file_exists_symlink": {
-			file:      "symlink.html",
-			isSymlink: true,
+			file:         "symlink.html",
+			isSymlink:    true,
+			expectedName: "symlink.html",
+		},
+		"has_root": {
+			file:         "",
+			isDir:        true,
+			expectedName: "public",
+		},
+		"has_root_dot": {
+			file:         ".",
+			isDir:        true,
+			expectedName: "public",
+		},
+		"has_root_slash": {
+			file:         "/",
+			isDir:        true,
+			expectedName: "public",
 		},
 		"is_dir": {
-			file:  "subdir",
-			isDir: true,
+			file:         "subdir",
+			isDir:        true,
+			expectedName: "subdir",
 		},
 		"file_does_not_exist": {
 			file:        "unknown.html",
@@ -136,7 +156,7 @@ func TestLstat(t *testing.T) {
 			}
 
 			require.NoError(t, err)
-			require.Contains(t, tt.file, fi.Name())
+			require.Equal(t, tt.expectedName, fi.Name())
 			require.Equal(t, tt.isDir, fi.IsDir())
 			require.NotEmpty(t, fi.ModTime())
 
@@ -265,7 +285,7 @@ func openZipArchive(t *testing.T, requests *int64) (*zipArchive, func()) {
 		requests = new(int64)
 	}
 
-	testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip", requests)
+	testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public-without-dirs.zip", requests)
 
 	fs := New().(*zipVFS)
 	zip := newArchive(fs, testServerURL+"/public.zip", time.Second)