diff options
Diffstat (limited to 'internal/serving')
-rw-r--r-- | internal/serving/disk/lazy.go | 80 | ||||
-rw-r--r-- | internal/serving/disk/reader.go | 26 | ||||
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 33 |
3 files changed, 14 insertions, 125 deletions
diff --git a/internal/serving/disk/lazy.go b/internal/serving/disk/lazy.go deleted file mode 100644 index 9b58457c..00000000 --- a/internal/serving/disk/lazy.go +++ /dev/null @@ -1,80 +0,0 @@ -package disk - -import ( - "context" - "errors" - "fmt" - "io" - - "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" -) - -var ( - // Make sure lazyFile is a vfs.File to support vfs.ServeCompressedFile - // if the file is compressed. - // This should always be satisfied because root.Open always returns - // a vfs.File. - _ vfs.File = &lazyFile{} - - // Make sure lazyFile is a ReadSeeker to support http.ServeContent - // if the file is not compressed. - // Note: lazyFile.Seek only works if the underlying root.Open returns - // a vfs.SeekableFile which is the case if the file is not compressed. - _ io.ReadSeeker = &lazyFile{} - - // ErrInvalidSeeker is returned if lazyFile.Seek is called and the - // underlying file is not seekable - ErrInvalidSeeker = errors.New("file is not seekable") -) - -type lazyFile struct { - f vfs.File - err error - load func() (vfs.File, error) -} - -func lazyOpen(ctx context.Context, root vfs.Root, fullPath string) lazyFile { - lf := lazyFile{ - load: func() (vfs.File, error) { - return root.Open(ctx, fullPath) - }, - } - - return lf -} - -func (lf lazyFile) Read(p []byte) (int, error) { - if lf.f == nil && lf.err == nil { - lf.f, lf.err = lf.load() - } - - if lf.err != nil { - return 0, lf.err - } - - return lf.f.Read(p) -} - -func (lf lazyFile) Close() error { - if lf.f != nil { - return lf.f.Close() - } - - return nil -} - -func (lf lazyFile) Seek(offset int64, whence int) (int64, error) { - if lf.f == nil && lf.err == nil { - lf.f, lf.err = lf.load() - } - - if lf.err != nil { - return 0, lf.err - } - - if sf, ok := lf.f.(io.ReadSeeker); ok { - return sf.Seek(offset, whence) - } - - return 0, fmt.Errorf("unable to seek from %T: %w", lf.f, ErrInvalidSeeker) -} diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index f804c92c..f250a0d6 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -199,6 +199,14 @@ func (reader *Reader) resolvePath(ctx context.Context, root vfs.Root, subPath .. func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *http.Request, root vfs.Root, origPath, sha string, accessControl bool) bool { fullPath := reader.handleContentEncoding(ctx, w, r, root, origPath) + file, err := root.Open(ctx, fullPath) + if err != nil { + httperrors.Serve500WithRequest(w, r, "root.Open", err) + return true + } + + defer file.Close() + fi, err := root.Lstat(ctx, fullPath) if err != nil { httperrors.Serve500WithRequest(w, r, "root.Lstat", err) @@ -224,20 +232,12 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h reader.fileSizeMetric.WithLabelValues(reader.vfs.Name()).Observe(float64(fi.Size())) - compressed, err := root.IsCompressed(fullPath) - if err != nil { - httperrors.Serve500WithRequest(w, r, "root.IsCompressed", err) - return true - } - - lf := lazyOpen(ctx, root, fullPath) - defer lf.Close() - - if compressed { - w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10)) - vfsServing.ServeCompressedFile(w, r, fi.ModTime(), lf) + // Support vfs.SeekableFile if available (uncompressed files) + if rs, ok := file.(vfs.SeekableFile); ok { + http.ServeContent(w, r, origPath, fi.ModTime(), rs) } else { - http.ServeContent(w, r, origPath, fi.ModTime(), lf) + w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10)) + vfsServing.ServeCompressedFile(w, r, fi.ModTime(), file) } return true diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go index 88db51aa..c3b18f2b 100644 --- a/internal/serving/disk/zip/serving_test.go +++ b/internal/serving/disk/zip/serving_test.go @@ -11,13 +11,11 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitlab-pages/internal/config" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" - "gitlab.com/gitlab-org/gitlab-pages/metrics" ) func TestZip_ServeFileHTTP(t *testing.T) { @@ -35,32 +33,25 @@ func TestZip_ServeFileHTTP(t *testing.T) { path string expectedStatus int expectedBody string - // Used to assert there are no zip requests if we don't - // need to open or read from the file, making sure the - // lazyFile is working as intended. - expectedOpen bool - extraHeaders http.Header + extraHeaders http.Header }{ "accessing /index.html": { vfsPath: httpURL, path: "/index.html", expectedStatus: http.StatusOK, expectedBody: "zip.gitlab.io/project/index.html\n", - expectedOpen: true, }, "accessing /index.html from disk": { vfsPath: fileURL, path: "/index.html", expectedStatus: http.StatusOK, expectedBody: "zip.gitlab.io/project/index.html\n", - expectedOpen: true, }, "accessing /": { vfsPath: httpURL, path: "/", expectedStatus: http.StatusOK, expectedBody: "zip.gitlab.io/project/index.html\n", - expectedOpen: true, }, "accessing / If-Modified-Since": { vfsPath: httpURL, @@ -75,7 +66,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { path: "/", expectedStatus: http.StatusOK, expectedBody: "zip.gitlab.io/project/index.html\n", - expectedOpen: true, extraHeaders: http.Header{ "If-Modified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, }, @@ -92,7 +82,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { vfsPath: httpURL, path: "/", expectedStatus: http.StatusOK, - expectedOpen: true, expectedBody: "zip.gitlab.io/project/index.html\n", extraHeaders: http.Header{ "If-Unmodified-Since": {time.Now().Format(http.TimeFormat)}, @@ -102,7 +91,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { vfsPath: fileURL, path: "/", expectedStatus: http.StatusOK, - expectedOpen: true, expectedBody: "zip.gitlab.io/project/index.html\n", }, "accessing without /": { @@ -145,7 +133,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { vfsPath: httpURL, path: "/", expectedStatus: http.StatusOK, - expectedOpen: true, expectedBody: "zip.gitlab.io/project/index.html\n", extraHeaders: http.Header{ "If-None-Match": {fmt.Sprintf("%q", "badetag")}, @@ -155,7 +142,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { vfsPath: httpURL, path: "/", expectedStatus: http.StatusOK, - expectedOpen: true, expectedBody: "zip.gitlab.io/project/index.html\n", extraHeaders: http.Header{ "If-Match": {fmt.Sprintf("%q", sha(httpURL))}, @@ -203,8 +189,6 @@ func TestZip_ServeFileHTTP(t *testing.T) { r.Header = test.extraHeaders } - zipRequestsCountBefore := zipCacheRequests() - handler := serving.Handler{ Writer: w, Request: r, @@ -239,25 +223,10 @@ func TestZip_ServeFileHTTP(t *testing.T) { if test.expectedStatus != http.StatusInternalServerError { require.Equal(t, test.expectedBody, string(body)) } - - zipRequestsCountAfter := zipCacheRequests() - - if !test.expectedOpen { - require.Equal(t, zipRequestsCountBefore, zipRequestsCountAfter) - } else { - require.NotEqual(t, zipRequestsCountBefore, zipRequestsCountAfter) - } }) } } -func zipCacheRequests() float64 { - zipMetricHit := metrics.ZipCacheRequests.WithLabelValues("data-offset", "hit") - zipMetricMiss := metrics.ZipCacheRequests.WithLabelValues("data-offset", "miss") - - return testutil.ToFloat64(zipMetricHit) + testutil.ToFloat64(zipMetricMiss) -} - func sha(path string) string { sha := sha256.Sum256([]byte(path)) s := hex.EncodeToString(sha[:]) |