diff options
author | Vladimir Shushlin <vshushlin@gitlab.com> | 2020-10-15 11:42:58 +0300 |
---|---|---|
committer | Vladimir Shushlin <vshushlin@gitlab.com> | 2020-10-15 11:42:58 +0300 |
commit | e728fe3617fb977739a9a7fbf0b165cbd0a09f07 (patch) | |
tree | ecd9ab4c1582761f732d7f0c0a3415e780815909 | |
parent | db87fd9fdf0b7b661fd09c1b4aea71211142c3c3 (diff) | |
parent | 715421d626df612c78264a6377edcca1fbb1e36a (diff) |
Merge branch 'fix-zip-directories' into 'master'
Fix support for archives without directory structure
Closes #482
See merge request gitlab-org/gitlab-pages!373
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 64 | ||||
-rw-r--r-- | internal/vfs/zip/archive.go | 57 | ||||
-rw-r--r-- | internal/vfs/zip/archive_test.go | 44 | ||||
-rw-r--r-- | shared/pages/group/zip.gitlab.io/public-without-dirs.zip | bin | 0 -> 2117 bytes |
4 files changed, 126 insertions, 39 deletions
diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go index 14fdabdf..e95432ae 100644 --- a/internal/serving/disk/zip/serving_test.go +++ b/internal/serving/disk/zip/serving_test.go @@ -13,32 +13,60 @@ import ( ) func TestZip_ServeFileHTTP(t *testing.T) { - testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip") + testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public-without-dirs.zip") defer cleanup() - s := Instance() - w := httptest.NewRecorder() - r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip/index.html", nil) - handler := serving.Handler{ - Writer: w, - Request: r, - LookupPath: &serving.LookupPath{ - Prefix: "", - Path: testServerURL + "/public.zip", + tests := map[string]struct { + path string + expectedStatus int + expectedBody string + }{ + "accessing /index.html": { + path: "/index.html", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + }, + "accessing /": { + path: "/", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + }, + "accessing without /": { + path: "", + expectedStatus: http.StatusFound, + expectedBody: `<a href="//zip.gitlab.io/zip/">Found</a>.`, }, - SubPath: "/index.html", } - require.True(t, s.ServeFileHTTP(handler)) + s := Instance() + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + w := httptest.NewRecorder() + r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip"+test.path, nil) + + handler := serving.Handler{ + Writer: w, + Request: r, + LookupPath: &serving.LookupPath{ + Prefix: "/zip/", + Path: testServerURL + "/public.zip", + }, + SubPath: test.path, + } - resp := w.Result() - defer resp.Body.Close() + require.True(t, s.ServeFileHTTP(handler)) - require.Equal(t, http.StatusOK, resp.StatusCode) - body, err := ioutil.ReadAll(resp.Body) - require.NoError(t, err) + resp := w.Result() + defer resp.Body.Close() - require.Contains(t, string(body), "zip.gitlab.io/project/index.html\n") + require.Equal(t, test.expectedStatus, resp.StatusCode) + body, err := ioutil.ReadAll(resp.Body) + require.NoError(t, err) + + require.Contains(t, string(body), test.expectedBody) + }) + } } var chdirSet = false 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) diff --git a/shared/pages/group/zip.gitlab.io/public-without-dirs.zip b/shared/pages/group/zip.gitlab.io/public-without-dirs.zip Binary files differnew file mode 100644 index 00000000..a6cfdfcf --- /dev/null +++ b/shared/pages/group/zip.gitlab.io/public-without-dirs.zip |