From 0ef6b5982e26f5957fbe5eaf0332f73625a3d24f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Tue, 13 Oct 2020 12:45:20 +0200 Subject: [PATCH 1/2] 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 ``` --- internal/serving/disk/zip/serving_test.go | 64 +++++++++++++----- internal/vfs/zip/archive.go | 55 +++++++++++++-- internal/vfs/zip/archive_test.go | 44 ++++++++---- .../zip.gitlab.io/public-without-dirs.zip | Bin 0 -> 2117 bytes 4 files changed, 127 insertions(+), 36 deletions(-) create mode 100644 shared/pages/group/zip.gitlab.io/public-without-dirs.zip diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go index 14fdabdf6..e95432ae3 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: `Found.`, }, - 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 548ba6518..5a49a2a40 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 bd7627b10..ef6785b5e 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 new file mode 100644 index 0000000000000000000000000000000000000000..a6cfdfcfc595b3fafeb77abe6c8d1c28a88bfa04 GIT binary patch literal 2117 zcmWIWW@h1H0D*U+`hnj1q8%Cx3=AMF!63s>P@0sJnXF%2nv{}Rq@R(RlasHPQIeYz z8p6rIEYF`1AKua$Us}P-*(j;NIIybN(%CuOJlQ?3A|pK`C9y2ZD9Otuq^hjcD7Prf z%-}J*T+L-_SiL$+^=I4&4UFX6 z&CA5dzyQMH2)mLJQ{sy&b3tKDCnWdH^7^bNsbwjK2VD=2~f$#z{tSBu%r>h#9g4VLJBmr!VKAT%wh`J^eP4h zwBm9E$9ZT7D+4pA(83y=xC$xc;5^NU9-K&V4mKWC{a`g7krq(PBxK{Qnc&95${At} zMJ<4k4PA^d6j7uQV=O3Rpq4cdV?inQ|3ZFdxLYwx9##fsP|1TQAV7wr<~n4 Date: Wed, 14 Oct 2020 08:26:51 +0000 Subject: [PATCH 2/2] Apply 2 suggestion(s) to 1 file(s) --- internal/vfs/zip/archive.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go index 5a49a2a40..ba15af200 100644 --- a/internal/vfs/zip/archive.go +++ b/internal/vfs/zip/archive.go @@ -156,7 +156,7 @@ func (a *zipArchive) readArchive() { // addPathDirectory adds a directory for a given path func (a *zipArchive) addPathDirectory(path string) { - // Split makes `path` with `/` + // Split dir and file from `path` path, _ = filepath.Split(path) if path == "" { return @@ -184,11 +184,7 @@ func (a *zipArchive) findFile(name string) *zip.File { func (a *zipArchive) findDirectory(name string) *zip.FileHeader { name = filepath.Join(dirPrefix, name) - if dir := a.directories[name+"/"]; dir != nil { - return dir - } - - return nil + return a.directories[name+"/"] } // Open finds the file by name inside the zipArchive and returns a reader that can be served by the VFS -- GitLab