diff options
Diffstat (limited to 'internal/serving')
-rw-r--r-- | internal/serving/disk/reader.go | 67 | ||||
-rw-r--r-- | internal/serving/disk/symlink/path_test.go | 10 | ||||
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 30 | ||||
-rw-r--r-- | internal/serving/lookup_path.go | 3 |
4 files changed, 81 insertions, 29 deletions
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index a39e38b9..2aeeeb27 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -2,8 +2,10 @@ package disk import ( "context" + "errors" "fmt" "io" + "io/fs" "net/http" "strconv" "strings" @@ -13,10 +15,12 @@ import ( "gitlab.com/gitlab-org/labkit/errortracking" "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors" + "gitlab.com/gitlab-org/gitlab-pages/internal/logging" "gitlab.com/gitlab-org/gitlab-pages/internal/redirects" "gitlab.com/gitlab-org/gitlab-pages/internal/serving" "gitlab.com/gitlab-org/gitlab-pages/internal/serving/disk/symlink" "gitlab.com/gitlab-org/gitlab-pages/internal/vfs" + vfsServing "gitlab.com/gitlab-org/gitlab-pages/internal/vfs/serving" ) // Reader is a disk access driver @@ -36,12 +40,10 @@ func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirec // tryRedirects returns true if it successfully handled request func (reader *Reader) tryRedirects(h serving.Handler) bool { ctx := h.Request.Context() - root, err := reader.vfs.Root(ctx, h.LookupPath.Path) - if vfs.IsNotExist(err) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) - return true + + root, served := reader.root(h) + if root == nil { + return served } r := redirects.ParseRedirects(ctx, root) @@ -51,7 +53,7 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { if err != redirects.ErrNoRedirect { // We assume that rewrite failure is not fatal // and we only capture the error - errortracking.Capture(err, errortracking.WithRequest(h.Request)) + errortracking.Capture(err, errortracking.WithRequest(h.Request), errortracking.WithStackTrace()) } return false } @@ -69,13 +71,9 @@ func (reader *Reader) tryRedirects(h serving.Handler) bool { func (reader *Reader) tryFile(h serving.Handler) bool { ctx := h.Request.Context() - root, err := reader.vfs.Root(ctx, h.LookupPath.Path) - if vfs.IsNotExist(err) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, - "vfs.Root", err) - return true + root, served := reader.root(h) + if root == nil { + return served } fullPath, err := reader.resolvePath(ctx, root, h.SubPath) @@ -127,12 +125,9 @@ func redirectPath(request *http.Request) string { func (reader *Reader) tryNotFound(h serving.Handler) bool { ctx := h.Request.Context() - root, err := reader.vfs.Root(ctx, h.LookupPath.Path) - if vfs.IsNotExist(err) { - return false - } else if err != nil { - httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) - return true + root, served := reader.root(h) + if root == nil { + return served } page404, err := reader.resolvePath(ctx, root, "404.html") @@ -144,6 +139,12 @@ func (reader *Reader) tryNotFound(h serving.Handler) bool { err = reader.serveCustomFile(ctx, h.Writer, h.Request, http.StatusNotFound, root, page404) if err != nil { + // Handle context.Canceled error as not exist https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 + if errors.Is(err, context.Canceled) { + logging.LogRequest(h.Request).WithError(err).Warn("user cancelled request") + return false + } + httperrors.Serve500WithRequest(h.Writer, h.Request, "serveCustomFile", err) return true } @@ -229,10 +230,8 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h if rs, ok := file.(vfs.SeekableFile); ok { http.ServeContent(w, r, origPath, fi.ModTime(), rs) } else { - // compressed files will be served by io.Copy - // TODO: Add extra headers https://gitlab.com/gitlab-org/gitlab-pages/-/issues/466 w.Header().Set("Content-Length", strconv.FormatInt(fi.Size(), 10)) - io.Copy(w, file) + vfsServing.ServeCompressedFile(w, r, fi.ModTime(), file) } return true @@ -271,3 +270,25 @@ func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter return nil } + +// root tries to resolve the vfs.Root and handles errors for it. +// It returns whether we served the response or not. +func (reader *Reader) root(h serving.Handler) (vfs.Root, bool) { + root, err := reader.vfs.Root(h.Request.Context(), h.LookupPath.Path, h.LookupPath.SHA256) + if err == nil { + return root, false + } + + if errors.Is(err, fs.ErrNotExist) { + return nil, false + } + + if errors.Is(err, context.Canceled) { + // Handle context.Canceled error as not found exist https://gitlab.com/gitlab-org/gitlab-pages/-/issues/669 + httperrors.Serve404(h.Writer) + return nil, true + } + + httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err) + return nil, true +} diff --git a/internal/serving/disk/symlink/path_test.go b/internal/serving/disk/symlink/path_test.go index 400432c2..59cb685f 100644 --- a/internal/serving/disk/symlink/path_test.go +++ b/internal/serving/disk/symlink/path_test.go @@ -6,6 +6,8 @@ package symlink_test import ( "context" + "errors" + "io/fs" "os" "path/filepath" "runtime" @@ -20,7 +22,7 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/vfs/local" ) -var fs = vfs.Instrumented(&local.VFS{}) +var localFs = vfs.Instrumented(&local.VFS{}) type EvalSymlinksTest struct { // If dest is empty, the path is created; otherwise the dest is symlinked to the path. @@ -70,7 +72,7 @@ func simpleJoin(path ...string) string { } func testEvalSymlinks(t *testing.T, wd, path, want string) { - root, err := fs.Root(context.Background(), wd) + root, err := localFs.Root(context.Background(), wd, "") require.NoError(t, err) have, err := symlink.EvalSymlinks(context.Background(), root, path) @@ -125,7 +127,7 @@ func TestEvalSymlinksIsNotExist(t *testing.T) { root, _ := testhelpers.TmpDir(t, "symlink_tests") _, err := symlink.EvalSymlinks(context.Background(), root, "notexist") - if !os.IsNotExist(err) { + if !errors.Is(err, fs.ErrNotExist) { t.Errorf("expected the file is not found, got %v\n", err) } @@ -136,7 +138,7 @@ func TestEvalSymlinksIsNotExist(t *testing.T) { defer os.Remove("link") _, err = symlink.EvalSymlinks(context.Background(), root, "link") - if !os.IsNotExist(err) { + if !errors.Is(err, fs.ErrNotExist) { t.Errorf("expected the file is not found, got %v\n", err) } } diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go index 167c47c3..fefc8cbe 100644 --- a/internal/serving/disk/zip/serving_test.go +++ b/internal/serving/disk/zip/serving_test.go @@ -1,6 +1,7 @@ package zip import ( + "crypto/sha256" "io" "net/http" "net/http/httptest" @@ -30,6 +31,7 @@ func TestZip_ServeFileHTTP(t *testing.T) { path string expectedStatus int expectedBody string + extraHeaders http.Header }{ "accessing /index.html": { vfsPath: httpURL, @@ -49,6 +51,22 @@ func TestZip_ServeFileHTTP(t *testing.T) { expectedStatus: http.StatusOK, expectedBody: "zip.gitlab.io/project/index.html\n", }, + "accessing / If-Modified-Since": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusNotModified, + extraHeaders: http.Header{ + "If-Modified-Since": {time.Now().Format(http.TimeFormat)}, + }, + }, + "accessing / If-Unmodified-Since": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusPreconditionFailed, + extraHeaders: http.Header{ + "If-Unmodified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, + }, + }, "accessing / from disk": { vfsPath: fileURL, path: "/", @@ -103,7 +121,12 @@ func TestZip_ServeFileHTTP(t *testing.T) { t.Run(name, func(t *testing.T) { w := httptest.NewRecorder() w.Code = 0 // ensure that code is not set, and it is being set by handler - r := httptest.NewRequest("GET", "http://zip.gitlab.io/zip"+test.path, nil) + r := httptest.NewRequest(http.MethodGet, "http://zip.gitlab.io/zip"+test.path, nil) + + r.Header = test.extraHeaders + + sha := sha256.Sum256([]byte(test.vfsPath)) + etag := string(sha[:]) handler := serving.Handler{ Writer: w, @@ -111,6 +134,7 @@ func TestZip_ServeFileHTTP(t *testing.T) { LookupPath: &serving.LookupPath{ Prefix: "/zip/", Path: test.vfsPath, + SHA256: etag, }, SubPath: test.path, } @@ -130,6 +154,10 @@ func TestZip_ServeFileHTTP(t *testing.T) { body, err := io.ReadAll(resp.Body) require.NoError(t, err) + if test.expectedStatus == http.StatusOK { + require.NotEmpty(t, resp.Header.Get("Last-Modified")) + } + require.Contains(t, string(body), test.expectedBody) }) } diff --git a/internal/serving/lookup_path.go b/internal/serving/lookup_path.go index 1aefe1b8..421e2a51 100644 --- a/internal/serving/lookup_path.go +++ b/internal/serving/lookup_path.go @@ -5,7 +5,8 @@ type LookupPath struct { ServingType string // Serving type being used, like `zip` Prefix string // Project prefix, for example, /my/project in group.gitlab.io/my/project/index.html Path string // Path is an internal and serving-specific location of a document - IsNamespaceProject bool // IsNamespaceProject is DEPRECATED, see https://gitlab.com/gitlab-org/gitlab-pages/issues/272 + SHA256 string + IsNamespaceProject bool // IsNamespaceProject is DEPRECATED, see https://gitlab.com/gitlab-org/gitlab-pages/issues/272 IsHTTPSOnly bool HasAccessControl bool ProjectID uint64 |