diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-13 13:45:20 +0300 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2020-10-13 14:03:02 +0300 |
commit | 0ef6b5982e26f5957fbe5eaf0332f73625a3d24f (patch) | |
tree | 76c9b0e5a16e9d19d0dd911029f77eec7512ddef | |
parent | 6755ba75aff4044f7dfa40578426195b9922907a (diff) |
Fix support for archives without directory structure
In case of archives that do not store directories
we would fail to automatically serve `index.html`
for a `/` type of request.
This makes us create directories when traversing
the file list.
```
Archive: public-without-dirs.zip
Length Date Time Name
--------- ---------- ----- ----
40 2020-09-15 02:47 public/subdir/hello.html
14 2020-09-15 03:35 public/subdir/2bp3Qzs9CCW7cGnxhghdavZ2bJDTzvu2mrj6O8Yqjm3YMRozRZULxBBKzJXCK16GlsvO1GlbCyONf2LTCndJU9cIr5T3PLDN7XnfG00lEmf9DWHPXiAbbi0v8ioSjnoTqdyjELVKuhsGRGxeV9RptLMyGnbpJx1w2uECiUQSHrRVQNuq2xoHLlk30UAmis1EhGXP5kKprzHxuavsKMdT4XRP0d79tie4tjqtfRsP4y60hmNS1vSujrxzhDa
33 2020-09-15 02:47 public/subdir/linked.html
31 2020-09-15 02:47 public/404.html
33 2020-09-15 02:47 public/index.html
258 2020-10-13 12:40 public/bad_symlink.html
18 2020-10-13 12:40 public/symlink.html
```
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 64 | ||||
-rw-r--r-- | internal/vfs/zip/archive.go | 55 | ||||
-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, 127 insertions, 36 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..5a49a2a4 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 makes `path` with `/` + 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,7 +178,13 @@ func (a *zipArchive) findFile(name string) *zip.File { return file } - if dir := a.files[name+"/"]; dir != nil { + return nil +} + +func (a *zipArchive) findDirectory(name string) *zip.FileHeader { + name = filepath.Join(dirPrefix, name) + + if dir := a.directories[name+"/"]; dir != nil { return dir } @@ -163,6 +195,9 @@ func (a *zipArchive) findFile(name string) *zip.File { 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 +228,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 |