Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-pages.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--acceptance_test.go31
-rw-r--r--helpers_test.go3
-rw-r--r--internal/httperrors/httperrors.go14
-rw-r--r--internal/httprange/http_reader.go5
-rw-r--r--internal/httprange/http_reader_test.go8
-rw-r--r--internal/httprange/resource.go3
-rw-r--r--internal/httprange/resource_test.go3
-rw-r--r--internal/serving/disk/local/serving_test.go84
-rw-r--r--internal/serving/disk/reader.go81
-rw-r--r--internal/serving/disk/serving.go6
-rw-r--r--internal/serving/disk/zip/serving_test.go32
-rw-r--r--internal/vfs/errors.go18
-rw-r--r--internal/vfs/local/vfs.go8
-rw-r--r--internal/vfs/local/vfs_test.go4
-rw-r--r--internal/vfs/zip/archive_test.go3
-rw-r--r--internal/vfs/zip/vfs.go6
-rw-r--r--internal/vfs/zip/vfs_test.go4
-rw-r--r--shared/lookups/zip-malformed.gitlab.io.json16
-rw-r--r--shared/lookups/zip-not-found.gitlab.io.json16
19 files changed, 281 insertions, 64 deletions
diff --git a/acceptance_test.go b/acceptance_test.go
index 28dc7bd4..a6ac31d0 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -2023,60 +2023,79 @@ func TestZipServing(t *testing.T) {
defer cleanup()
tests := map[string]struct {
+ host string
urlSuffix string
expectedStatusCode int
expectedContent string
}{
"base_domain_no_suffix": {
+ host: "zip.gitlab.io",
urlSuffix: "/",
expectedStatusCode: http.StatusOK,
expectedContent: "zip.gitlab.io/project/index.html\n",
},
"file_exists": {
+ host: "zip.gitlab.io",
urlSuffix: "/index.html",
expectedStatusCode: http.StatusOK,
expectedContent: "zip.gitlab.io/project/index.html\n",
},
"file_exists_in_subdir": {
+ host: "zip.gitlab.io",
urlSuffix: "/subdir/hello.html",
expectedStatusCode: http.StatusOK,
expectedContent: "zip.gitlab.io/project/subdir/hello.html\n",
},
"file_exists_symlink": {
+ host: "zip.gitlab.io",
urlSuffix: "/symlink.html",
expectedStatusCode: http.StatusOK,
expectedContent: "symlink.html->subdir/linked.html\n",
},
"dir": {
+ host: "zip.gitlab.io",
urlSuffix: "/subdir/",
expectedStatusCode: http.StatusNotFound,
expectedContent: "zip.gitlab.io/project/404.html\n",
},
"file_does_not_exist": {
+ host: "zip.gitlab.io",
urlSuffix: "/unknown.html",
expectedStatusCode: http.StatusNotFound,
expectedContent: "zip.gitlab.io/project/404.html\n",
},
"bad_symlink": {
+ host: "zip.gitlab.io",
urlSuffix: "/bad-symlink.html",
expectedStatusCode: http.StatusNotFound,
expectedContent: "zip.gitlab.io/project/404.html\n",
},
+ "with_not_found_zip": {
+ host: "zip-not-found.gitlab.io",
+ urlSuffix: "/",
+ expectedStatusCode: http.StatusNotFound,
+ expectedContent: "The page you're looking for could not be found",
+ },
+ "with_malformed_zip": {
+ host: "zip-malformed.gitlab.io",
+ urlSuffix: "/",
+ expectedStatusCode: http.StatusInternalServerError,
+ expectedContent: "Something went wrong (500)",
+ },
}
for name, tt := range tests {
t.Run(name, func(t *testing.T) {
- response, err := GetPageFromListener(t, httpListener, "zip.gitlab.io", tt.urlSuffix)
+ response, err := GetPageFromListener(t, httpListener, tt.host, tt.urlSuffix)
require.NoError(t, err)
defer response.Body.Close()
require.Equal(t, tt.expectedStatusCode, response.StatusCode)
- if tt.expectedStatusCode == http.StatusOK || tt.expectedStatusCode == http.StatusNotFound {
- body, err := ioutil.ReadAll(response.Body)
- require.NoError(t, err)
- require.Equal(t, tt.expectedContent, string(body), "content mismatch")
- }
+ body, err := ioutil.ReadAll(response.Body)
+ require.NoError(t, err)
+
+ require.Contains(t, string(body), tt.expectedContent, "content mismatch")
})
}
}
diff --git a/helpers_test.go b/helpers_test.go
index eec3c94b..2e6fec58 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -542,6 +542,9 @@ func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) {
m.HandleFunc("/public.zip", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.ServeFile(w, r, zipFilePath)
}))
+ m.HandleFunc("/malformed.zip", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusInternalServerError)
+ }))
// create a listener with the desired port.
l, err := net.Listen("tcp", objectStorageMockServer)
diff --git a/internal/httperrors/httperrors.go b/internal/httperrors/httperrors.go
index 1ae5224b..476d270c 100644
--- a/internal/httperrors/httperrors.go
+++ b/internal/httperrors/httperrors.go
@@ -3,6 +3,10 @@ package httperrors
import (
"fmt"
"net/http"
+
+ log "github.com/sirupsen/logrus"
+
+ "gitlab.com/gitlab-org/labkit/errortracking"
)
type content struct {
@@ -177,6 +181,16 @@ func Serve500(w http.ResponseWriter) {
serveErrorPage(w, content500)
}
+// Serve500WithRequest returns a 500 error response / HTML page to the http.ResponseWriter
+func Serve500WithRequest(w http.ResponseWriter, r *http.Request, reason string, err error) {
+ log.WithFields(log.Fields{
+ "host": r.Host,
+ "path": r.URL.Path,
+ }).WithError(err).Error(reason)
+ errortracking.Capture(err, errortracking.WithRequest(r))
+ serveErrorPage(w, content500)
+}
+
// Serve502 returns a 502 error response / HTML page to the http.ResponseWriter
func Serve502(w http.ResponseWriter) {
serveErrorPage(w, content502)
diff --git a/internal/httprange/http_reader.go b/internal/httprange/http_reader.go
index 44694e85..5dc0f693 100644
--- a/internal/httprange/http_reader.go
+++ b/internal/httprange/http_reader.go
@@ -14,6 +14,9 @@ import (
)
var (
+ // ErrNotFound is returned when servers responds with 404
+ ErrNotFound = errors.New("resource not found")
+
// ErrRangeRequestsNotSupported is returned by Seek and Read
// when the remote server does not allow range requests (Accept-Ranges was not set)
ErrRangeRequestsNotSupported = errors.New("range requests are not supported by the remote server")
@@ -131,6 +134,8 @@ func (r *Reader) setResponse(res *http.Response) error {
if r.offset > 0 || r.Resource.ETag != "" && r.Resource.ETag != res.Header.Get("ETag") {
return ErrContentHasChanged
}
+ case http.StatusNotFound:
+ return ErrNotFound
case http.StatusPartialContent:
// Requested `Range` request succeeded https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/206
break
diff --git a/internal/httprange/http_reader_test.go b/internal/httprange/http_reader_test.go
index 5e971575..9c8f11d0 100644
--- a/internal/httprange/http_reader_test.go
+++ b/internal/httprange/http_reader_test.go
@@ -226,9 +226,13 @@ func TestReaderSetResponse(t *testing.T) {
status: http.StatusRequestedRangeNotSatisfiable,
expectedErrMsg: ErrRangeRequestsNotSupported.Error(),
},
- "unhandled_status_code": {
+ "not_found": {
status: http.StatusNotFound,
- expectedErrMsg: "httprange: read response 404:",
+ expectedErrMsg: ErrNotFound.Error(),
+ },
+ "unhandled_status_code": {
+ status: http.StatusInternalServerError,
+ expectedErrMsg: "httprange: read response 500:",
},
}
for name, tt := range tests {
diff --git a/internal/httprange/resource.go b/internal/httprange/resource.go
index 7e21ef29..d2dbd340 100644
--- a/internal/httprange/resource.go
+++ b/internal/httprange/resource.go
@@ -71,6 +71,9 @@ func NewResource(ctx context.Context, url string) (*Resource, error) {
case http.StatusRequestedRangeNotSatisfiable:
return nil, ErrRangeRequestsNotSupported
+ case http.StatusNotFound:
+ return nil, ErrNotFound
+
default:
return nil, fmt.Errorf("httprange: new resource %d: %q", res.StatusCode, res.Status)
}
diff --git a/internal/httprange/resource_test.go b/internal/httprange/resource_test.go
index 89d15a21..ace8f92c 100644
--- a/internal/httprange/resource_test.go
+++ b/internal/httprange/resource_test.go
@@ -2,7 +2,6 @@ package httprange
import (
"context"
- "fmt"
"net/http"
"net/http/httptest"
"testing"
@@ -60,7 +59,7 @@ func TestNewResource(t *testing.T) {
"not_found": {
url: "/some/resource",
status: http.StatusNotFound,
- expectedErrMsg: fmt.Sprintf("httprange: new resource %d: %q", http.StatusNotFound, "404 Not Found"),
+ expectedErrMsg: ErrNotFound.Error(),
},
"invalid_url": {
url: "/%",
diff --git a/internal/serving/disk/local/serving_test.go b/internal/serving/disk/local/serving_test.go
index 60f01acd..2602451f 100644
--- a/internal/serving/disk/local/serving_test.go
+++ b/internal/serving/disk/local/serving_test.go
@@ -15,29 +15,79 @@ import (
func TestDisk_ServeFileHTTP(t *testing.T) {
defer setUpTests(t)()
- s := Instance()
- w := httptest.NewRecorder()
- r := httptest.NewRequest("GET", "http://group.gitlab-example.com/serving/index.html", nil)
- handler := serving.Handler{
- Writer: w,
- Request: r,
- LookupPath: &serving.LookupPath{
- Prefix: "/serving",
- Path: "group/serving/public",
+ tests := map[string]struct {
+ vfsPath string
+ path string
+ expectedStatus int
+ expectedBody string
+ }{
+ "accessing /index.html": {
+ vfsPath: "group/serving/public",
+ path: "/index.html",
+ expectedStatus: http.StatusOK,
+ expectedBody: "HTML Document",
+ },
+ "accessing /": {
+ vfsPath: "group/serving/public",
+ path: "/",
+ expectedStatus: http.StatusOK,
+ expectedBody: "HTML Document",
+ },
+ "accessing without /": {
+ vfsPath: "group/serving/public",
+ path: "",
+ expectedStatus: http.StatusFound,
+ expectedBody: `<a href="//group.gitlab-example.com/serving/">Found</a>.`,
+ },
+ "accessing vfs path that is missing": {
+ vfsPath: "group/serving/public-missing",
+ path: "/index.html",
+ // we expect the status to not be set
+ expectedStatus: 0,
+ },
+ "accessing vfs path that is forbidden (like file)": {
+ vfsPath: "group/serving/public/index.html",
+ path: "/index.html",
+ expectedStatus: http.StatusInternalServerError,
},
- SubPath: "/index.html",
}
- require.True(t, s.ServeFileHTTP(handler))
+ s := Instance()
- resp := w.Result()
- defer resp.Body.Close()
+ for name, test := range tests {
+ 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://group.gitlab-example.com/serving"+test.path, nil)
- require.Equal(t, http.StatusOK, resp.StatusCode)
- body, err := ioutil.ReadAll(resp.Body)
- require.NoError(t, err)
+ handler := serving.Handler{
+ Writer: w,
+ Request: r,
+ LookupPath: &serving.LookupPath{
+ Prefix: "/serving/",
+ Path: test.vfsPath,
+ },
+ SubPath: test.path,
+ }
- require.Contains(t, string(body), "HTML Document")
+ if test.expectedStatus == 0 {
+ require.False(t, s.ServeFileHTTP(handler))
+ require.Zero(t, w.Code, "we expect status to not be set")
+ return
+ }
+
+ require.True(t, s.ServeFileHTTP(handler))
+
+ resp := w.Result()
+ defer resp.Body.Close()
+
+ 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/serving/disk/reader.go b/internal/serving/disk/reader.go
index e7f15a1e..5b0c56f0 100644
--- a/internal/serving/disk/reader.go
+++ b/internal/serving/disk/reader.go
@@ -11,7 +11,9 @@ import (
"time"
"github.com/prometheus/client_golang/prometheus"
+ "gitlab.com/gitlab-org/labkit/errortracking"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
"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"
@@ -25,39 +27,51 @@ type Reader struct {
}
// Show the user some validation messages for their _redirects file
-func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirects.Redirects) error {
+func (reader *Reader) serveRedirectsStatus(h serving.Handler, redirects *redirects.Redirects) {
h.Writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
h.Writer.Header().Set("X-Content-Type-Options", "nosniff")
h.Writer.WriteHeader(http.StatusOK)
- _, err := fmt.Fprintln(h.Writer, redirects.Status())
- return err
+ fmt.Fprintln(h.Writer, redirects.Status())
}
-func (reader *Reader) tryRedirects(h serving.Handler) error {
+// 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 err != nil {
- return err
+ if vfs.IsNotExist(err) {
+ return false
+ } else if err != nil {
+ httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err)
+ return true
}
r := redirects.ParseRedirects(ctx, root)
rewrittenURL, status, err := r.Rewrite(h.Request.URL)
if err != nil {
- return err
+ 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))
+ }
+ return false
}
http.Redirect(h.Writer, h.Request, rewrittenURL.Path, status)
-
- return nil
+ return true
}
-func (reader *Reader) tryFile(h serving.Handler) error {
+// tryFile returns true if it successfully handled request
+func (reader *Reader) tryFile(h serving.Handler) bool {
ctx := h.Request.Context()
root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
- if err != nil {
- return err
+ if vfs.IsNotExist(err) {
+ return false
+ } else if err != nil {
+ httperrors.Serve500WithRequest(h.Writer, h.Request,
+ "vfs.Root", err)
+ return true
}
fullPath, err := reader.resolvePath(ctx, root, h.SubPath)
@@ -80,7 +94,7 @@ func (reader *Reader) tryFile(h serving.Handler) error {
// Ensure that there's always "/" at end
redirectPath = strings.TrimSuffix(redirectPath, "/") + "/"
http.Redirect(h.Writer, h.Request, redirectPath, 302)
- return nil
+ return true
}
}
@@ -89,7 +103,9 @@ func (reader *Reader) tryFile(h serving.Handler) error {
}
if err != nil {
- return err
+ // We assume that this is mostly missing file type of the error
+ // and additional handlers should try to process the request
+ return false
}
// Serve status of `_redirects` under `_redirects`
@@ -97,34 +113,42 @@ func (reader *Reader) tryFile(h serving.Handler) error {
if fullPath == redirects.ConfigFile {
if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
r := redirects.ParseRedirects(ctx, root)
- return reader.serveRedirectsStatus(h, r)
+ reader.serveRedirectsStatus(h, r)
+ return true
}
h.Writer.WriteHeader(http.StatusForbidden)
- return nil
+ return true
}
return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.HasAccessControl)
}
-func (reader *Reader) tryNotFound(h serving.Handler) error {
+func (reader *Reader) tryNotFound(h serving.Handler) bool {
ctx := h.Request.Context()
root, err := reader.vfs.Root(ctx, h.LookupPath.Path)
- if err != nil {
- return err
+ if vfs.IsNotExist(err) {
+ return false
+ } else if err != nil {
+ httperrors.Serve500WithRequest(h.Writer, h.Request, "vfs.Root", err)
+ return true
}
page404, err := reader.resolvePath(ctx, root, "404.html")
if err != nil {
- return err
+ // We assume that this is mostly missing file type of the error
+ // and additional handlers should try to process the request
+ return false
}
err = reader.serveCustomFile(ctx, h.Writer, h.Request, http.StatusNotFound, root, page404)
if err != nil {
- return err
+ httperrors.Serve500WithRequest(h.Writer, h.Request, "serveCustomFile", err)
+ return true
}
- return nil
+
+ return true
}
// Resolve the HTTP request to a path on disk, converting requests for
@@ -168,19 +192,21 @@ func (reader *Reader) resolvePath(ctx context.Context, root vfs.Root, subPath ..
return fullPath, nil
}
-func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *http.Request, root vfs.Root, origPath string, accessControl bool) error {
+func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *http.Request, root vfs.Root, origPath string, accessControl bool) bool {
fullPath := reader.handleContentEncoding(ctx, w, r, root, origPath)
file, err := root.Open(ctx, fullPath)
if err != nil {
- return err
+ httperrors.Serve500WithRequest(w, r, "root.Open", err)
+ return true
}
defer file.Close()
fi, err := root.Lstat(ctx, fullPath)
if err != nil {
- return err
+ httperrors.Serve500WithRequest(w, r, "root.Lstat", err)
+ return true
}
if !accessControl {
@@ -191,7 +217,8 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h
contentType, err := reader.detectContentType(ctx, root, origPath)
if err != nil {
- return err
+ httperrors.Serve500WithRequest(w, r, "detectContentType", err)
+ return true
}
w.Header().Set("Content-Type", contentType)
@@ -208,7 +235,7 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h
io.Copy(w, file)
}
- return nil
+ return true
}
func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter, r *http.Request, code int, root vfs.Root, origPath string) error {
diff --git a/internal/serving/disk/serving.go b/internal/serving/disk/serving.go
index 11b1689e..30c821ea 100644
--- a/internal/serving/disk/serving.go
+++ b/internal/serving/disk/serving.go
@@ -17,12 +17,12 @@ type Disk struct {
// ServeFileHTTP serves a file from disk and returns true. It returns false
// when a file could not been found.
func (s *Disk) ServeFileHTTP(h serving.Handler) bool {
- if s.reader.tryFile(h) == nil {
+ if s.reader.tryFile(h) {
return true
}
if os.Getenv("FF_ENABLE_REDIRECTS") != "false" {
- if s.reader.tryRedirects(h) == nil {
+ if s.reader.tryRedirects(h) {
return true
}
}
@@ -32,7 +32,7 @@ func (s *Disk) ServeFileHTTP(h serving.Handler) bool {
// ServeNotFoundHTTP tries to read a custom 404 page
func (s *Disk) ServeNotFoundHTTP(h serving.Handler) {
- if s.reader.tryNotFound(h) == nil {
+ if s.reader.tryNotFound(h) {
return
}
diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go
index e95432ae..bca2ae51 100644
--- a/internal/serving/disk/zip/serving_test.go
+++ b/internal/serving/disk/zip/serving_test.go
@@ -17,25 +17,40 @@ func TestZip_ServeFileHTTP(t *testing.T) {
defer cleanup()
tests := map[string]struct {
+ vfsPath string
path string
expectedStatus int
expectedBody string
}{
"accessing /index.html": {
+ vfsPath: testServerURL + "/public.zip",
path: "/index.html",
expectedStatus: http.StatusOK,
expectedBody: "zip.gitlab.io/project/index.html\n",
},
"accessing /": {
+ vfsPath: testServerURL + "/public.zip",
path: "/",
expectedStatus: http.StatusOK,
expectedBody: "zip.gitlab.io/project/index.html\n",
},
"accessing without /": {
+ vfsPath: testServerURL + "/public.zip",
path: "",
expectedStatus: http.StatusFound,
expectedBody: `<a href="//zip.gitlab.io/zip/">Found</a>.`,
},
+ "accessing archive that is 404": {
+ vfsPath: testServerURL + "/invalid.zip",
+ path: "/index.html",
+ // we expect the status to not be set
+ expectedStatus: 0,
+ },
+ "accessing archive that is 500": {
+ vfsPath: testServerURL + "/500",
+ path: "/index.html",
+ expectedStatus: http.StatusInternalServerError,
+ },
}
s := Instance()
@@ -43,6 +58,7 @@ func TestZip_ServeFileHTTP(t *testing.T) {
for name, test := range tests {
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)
handler := serving.Handler{
@@ -50,11 +66,17 @@ func TestZip_ServeFileHTTP(t *testing.T) {
Request: r,
LookupPath: &serving.LookupPath{
Prefix: "/zip/",
- Path: testServerURL + "/public.zip",
+ Path: test.vfsPath,
},
SubPath: test.path,
}
+ if test.expectedStatus == 0 {
+ require.False(t, s.ServeFileHTTP(handler))
+ require.Zero(t, w.Code, "we expect status to not be set")
+ return
+ }
+
require.True(t, s.ServeFileHTTP(handler))
resp := w.Result()
@@ -76,9 +98,15 @@ func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) {
chdir := testhelpers.ChdirInPath(t, "../../../../shared/pages", &chdirSet)
- testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ m := http.NewServeMux()
+ m.HandleFunc("/public.zip", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.ServeFile(w, r, zipFilePath)
}))
+ m.HandleFunc("/500", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusInternalServerError)
+ }))
+
+ testServer := httptest.NewServer(m)
return testServer.URL, func() {
chdir()
diff --git a/internal/vfs/errors.go b/internal/vfs/errors.go
new file mode 100644
index 00000000..32b86192
--- /dev/null
+++ b/internal/vfs/errors.go
@@ -0,0 +1,18 @@
+package vfs
+
+import (
+ "fmt"
+)
+
+type ErrNotExist struct {
+ Inner error
+}
+
+func (e ErrNotExist) Error() string {
+ return fmt.Sprintf("not exist: %q", e.Inner)
+}
+
+func IsNotExist(err error) bool {
+ _, ok := err.(*ErrNotExist)
+ return ok
+}
diff --git a/internal/vfs/local/vfs.go b/internal/vfs/local/vfs.go
index ca74dfbe..bdcd5160 100644
--- a/internal/vfs/local/vfs.go
+++ b/internal/vfs/local/vfs.go
@@ -20,12 +20,16 @@ func (fs VFS) Root(ctx context.Context, path string) (vfs.Root, error) {
}
rootPath, err = filepath.EvalSymlinks(rootPath)
- if err != nil {
+ if os.IsNotExist(err) {
+ return nil, &vfs.ErrNotExist{Inner: err}
+ } else if err != nil {
return nil, err
}
fi, err := os.Lstat(rootPath)
- if err != nil {
+ if os.IsNotExist(err) {
+ return nil, &vfs.ErrNotExist{Inner: err}
+ } else if err != nil {
return nil, err
}
diff --git a/internal/vfs/local/vfs_test.go b/internal/vfs/local/vfs_test.go
index ec67d595..b678cfa7 100644
--- a/internal/vfs/local/vfs_test.go
+++ b/internal/vfs/local/vfs_test.go
@@ -9,6 +9,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
+
+ "gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
)
var localVFS = &VFS{}
@@ -98,7 +100,7 @@ func TestVFSRoot(t *testing.T) {
rootVFS, err := localVFS.Root(context.Background(), filepath.Join(tmpDir, test.path))
if test.expectedIsNotExist {
- require.Equal(t, test.expectedIsNotExist, os.IsNotExist(err))
+ require.Equal(t, test.expectedIsNotExist, vfs.IsNotExist(err))
return
}
diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go
index ef6785b5..2474d419 100644
--- a/internal/vfs/zip/archive_test.go
+++ b/internal/vfs/zip/archive_test.go
@@ -12,6 +12,7 @@ import (
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/httprange"
"gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers"
)
@@ -272,7 +273,7 @@ func TestReadArchiveFails(t *testing.T) {
err := zip.openArchive(context.Background())
require.Error(t, err)
- require.Contains(t, err.Error(), "Not Found")
+ require.Contains(t, err.Error(), httprange.ErrNotFound.Error())
_, err = zip.Open(context.Background(), "index.html")
require.EqualError(t, err, os.ErrNotExist.Error())
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index 78a77e1c..d3c5cf9b 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -9,6 +9,7 @@ import (
"github.com/patrickmn/go-cache"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/httprange"
"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
"gitlab.com/gitlab-org/gitlab-pages/metrics"
)
@@ -82,6 +83,11 @@ func (fs *zipVFS) Root(ctx context.Context, path string) (vfs.Root, error) {
continue
}
+ // If archive is not found, return a known `vfs` error
+ if err == httprange.ErrNotFound {
+ err = &vfs.ErrNotExist{Inner: err}
+ }
+
return root, err
}
}
diff --git a/internal/vfs/zip/vfs_test.go b/internal/vfs/zip/vfs_test.go
index c12e49cd..d4730524 100644
--- a/internal/vfs/zip/vfs_test.go
+++ b/internal/vfs/zip/vfs_test.go
@@ -9,6 +9,8 @@ import (
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/httprange"
+ "gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
"gitlab.com/gitlab-org/gitlab-pages/metrics"
)
@@ -25,7 +27,7 @@ func TestVFSRoot(t *testing.T) {
},
"zip_file_does_not_exist": {
path: "/unknown",
- expectedErrMsg: "404 Not Found",
+ expectedErrMsg: vfs.ErrNotExist{Inner: httprange.ErrNotFound}.Error(),
},
"invalid_url": {
path: "/%",
diff --git a/shared/lookups/zip-malformed.gitlab.io.json b/shared/lookups/zip-malformed.gitlab.io.json
new file mode 100644
index 00000000..37ad1ddd
--- /dev/null
+++ b/shared/lookups/zip-malformed.gitlab.io.json
@@ -0,0 +1,16 @@
+{
+ "certificate": "",
+ "key": "",
+ "lookup_paths": [
+ {
+ "access_control": false,
+ "https_only": false,
+ "prefix": "/",
+ "project_id": 123,
+ "source": {
+ "path": "http://127.0.0.1:37003/malformed.zip",
+ "type": "zip"
+ }
+ }
+ ]
+}
diff --git a/shared/lookups/zip-not-found.gitlab.io.json b/shared/lookups/zip-not-found.gitlab.io.json
new file mode 100644
index 00000000..94de4a90
--- /dev/null
+++ b/shared/lookups/zip-not-found.gitlab.io.json
@@ -0,0 +1,16 @@
+{
+ "certificate": "",
+ "key": "",
+ "lookup_paths": [
+ {
+ "access_control": false,
+ "https_only": false,
+ "prefix": "/",
+ "project_id": 123,
+ "source": {
+ "path": "http://127.0.0.1:37003/not-found.zip",
+ "type": "zip"
+ }
+ }
+ ]
+}
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-16 14:30:00 +0300