diff options
author | Jaime Martinez <jmartinez@gitlab.com> | 2021-12-16 04:04:02 +0300 |
---|---|---|
committer | Jaime Martinez <jmartinez@gitlab.com> | 2021-12-16 04:04:02 +0300 |
commit | 8acd848251bf939316df0bad1ac4bc9bb17ef19d (patch) | |
tree | bd9c9853faeb030c9f22782d57667d6c6e4f5d92 | |
parent | 0b86a39d2c27bc548cc492bf6468cb80bde8f5ed (diff) | |
parent | b4a8532a926d6f3b5cfdcc829276c7554d6c9a86 (diff) |
Merge branch 'feat/etag' into 'master'
feat: implement ETag support for zip serving
Closes #82
See merge request gitlab-org/gitlab-pages!588
-rw-r--r-- | internal/serving/disk/reader.go | 14 | ||||
-rw-r--r-- | internal/serving/disk/zip/serving_test.go | 86 | ||||
-rw-r--r-- | test/acceptance/auth_test.go | 5 | ||||
-rw-r--r-- | test/acceptance/testdata/api_responses.go | 5 | ||||
-rw-r--r-- | test/acceptance/zip_test.go | 125 |
5 files changed, 210 insertions, 25 deletions
diff --git a/internal/serving/disk/reader.go b/internal/serving/disk/reader.go index ec97b128..b83425eb 100644 --- a/internal/serving/disk/reader.go +++ b/internal/serving/disk/reader.go @@ -109,7 +109,7 @@ func (reader *Reader) tryFile(h serving.Handler) bool { return true } - return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.HasAccessControl) + return reader.serveFile(ctx, h.Writer, h.Request, root, fullPath, h.LookupPath.SHA256, h.LookupPath.HasAccessControl) } func redirectPath(request *http.Request) string { @@ -194,7 +194,7 @@ 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) bool { +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) @@ -211,6 +211,9 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h return true } + ce := w.Header().Get("Content-Encoding") + w.Header().Set("ETag", fmt.Sprintf("%q", etag(ce, sha))) + if !accessControl { // Set caching headers w.Header().Set("Cache-Control", "max-age=600") @@ -242,6 +245,13 @@ func (reader *Reader) serveFile(ctx context.Context, w http.ResponseWriter, r *h return true } +func etag(contentEncoding, sha string) string { + if contentEncoding == "" { + return sha + } + return fmt.Sprintf("%s-%s", sha, contentEncoding) +} + func (reader *Reader) serveCustomFile(ctx context.Context, w http.ResponseWriter, r *http.Request, code int, root vfs.Root, origPath string) error { fullPath := reader.handleContentEncoding(ctx, w, r, root, origPath) diff --git a/internal/serving/disk/zip/serving_test.go b/internal/serving/disk/zip/serving_test.go index fefc8cbe..2b603b5d 100644 --- a/internal/serving/disk/zip/serving_test.go +++ b/internal/serving/disk/zip/serving_test.go @@ -2,6 +2,8 @@ package zip import ( "crypto/sha256" + "encoding/hex" + "fmt" "io" "net/http" "net/http/httptest" @@ -59,6 +61,15 @@ func TestZip_ServeFileHTTP(t *testing.T) { "If-Modified-Since": {time.Now().Format(http.TimeFormat)}, }, }, + "accessing / If-Modified-Since fails": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + extraHeaders: http.Header{ + "If-Modified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, + }, + }, "accessing / If-Unmodified-Since": { vfsPath: httpURL, path: "/", @@ -67,6 +78,15 @@ func TestZip_ServeFileHTTP(t *testing.T) { "If-Unmodified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, }, }, + "accessing / If-Unmodified-Since fails": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + extraHeaders: http.Header{ + "If-Unmodified-Since": {time.Now().Format(http.TimeFormat)}, + }, + }, "accessing / from disk": { vfsPath: fileURL, path: "/", @@ -77,13 +97,13 @@ func TestZip_ServeFileHTTP(t *testing.T) { vfsPath: httpURL, path: "", expectedStatus: http.StatusFound, - expectedBody: `<a href="//zip.gitlab.io/zip/">Found</a>.`, + expectedBody: "<a href=\"//zip.gitlab.io/zip/\">Found</a>.\n\n", }, "accessing without / from disk": { vfsPath: fileURL, path: "", expectedStatus: http.StatusFound, - expectedBody: `<a href="//zip.gitlab.io/zip/">Found</a>.`, + expectedBody: "<a href=\"//zip.gitlab.io/zip/\">Found</a>.\n\n", }, "accessing archive that is 404": { vfsPath: testServerURL + "/invalid.zip", @@ -101,6 +121,48 @@ func TestZip_ServeFileHTTP(t *testing.T) { path: "/index.html", expectedStatus: http.StatusInternalServerError, }, + "accessing / If-None-Match": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusNotModified, + extraHeaders: http.Header{ + "If-None-Match": {fmt.Sprintf("%q", sha(httpURL))}, + }, + }, + "accessing / If-None-Match fails": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + extraHeaders: http.Header{ + "If-None-Match": {fmt.Sprintf("%q", "badetag")}, + }, + }, + "accessing / If-Match": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusOK, + expectedBody: "zip.gitlab.io/project/index.html\n", + extraHeaders: http.Header{ + "If-Match": {fmt.Sprintf("%q", sha(httpURL))}, + }, + }, + "accessing / If-Match fails": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusPreconditionFailed, + extraHeaders: http.Header{ + "If-Match": {fmt.Sprintf("%q", "wrongetag")}, + }, + }, + "accessing / If-Match fails2": { + vfsPath: httpURL, + path: "/", + expectedStatus: http.StatusPreconditionFailed, + extraHeaders: http.Header{ + "If-Match": {","}, + }, + }, } cfg := &config.Config{ @@ -123,10 +185,9 @@ func TestZip_ServeFileHTTP(t *testing.T) { w.Code = 0 // ensure that code is not set, and it is being set by handler 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[:]) + if test.extraHeaders != nil { + r.Header = test.extraHeaders + } handler := serving.Handler{ Writer: w, @@ -134,7 +195,7 @@ func TestZip_ServeFileHTTP(t *testing.T) { LookupPath: &serving.LookupPath{ Prefix: "/zip/", Path: test.vfsPath, - SHA256: etag, + SHA256: sha(test.vfsPath), }, SubPath: test.path, } @@ -156,13 +217,22 @@ func TestZip_ServeFileHTTP(t *testing.T) { if test.expectedStatus == http.StatusOK { require.NotEmpty(t, resp.Header.Get("Last-Modified")) + require.NotEmpty(t, resp.Header.Get("ETag")) } - require.Contains(t, string(body), test.expectedBody) + if test.expectedStatus != http.StatusInternalServerError { + require.Equal(t, test.expectedBody, string(body)) + } }) } } +func sha(path string) string { + sha := sha256.Sum256([]byte(path)) + s := hex.EncodeToString(sha[:]) + return s +} + var chdirSet = false func newZipFileServerURL(t *testing.T, zipFilePath string) (string, func()) { diff --git a/test/acceptance/auth_test.go b/test/acceptance/auth_test.go index ad5c343a..d115dba5 100644 --- a/test/acceptance/auth_test.go +++ b/test/acceptance/auth_test.go @@ -539,7 +539,10 @@ func testAccessControl(t *testing.T, runPages runPagesFunc) { defer rsp3.Body.Close() require.Equal(t, tt.status, rsp3.StatusCode) - require.Equal(t, "", rsp3.Header.Get("Cache-Control")) + + // Make sure there are no cache headers + require.Empty(t, rsp3.Header.Values("Cache-Control")) + require.Empty(t, rsp3.Header.Values("Expires")) if tt.redirectBack { loc3, err := url.Parse(rsp3.Header.Get("Location")) diff --git a/test/acceptance/testdata/api_responses.go b/test/acceptance/testdata/api_responses.go index 5061dcaf..baa70f8f 100644 --- a/test/acceptance/testdata/api_responses.go +++ b/test/acceptance/testdata/api_responses.go @@ -2,6 +2,7 @@ package testdata import ( "crypto/sha256" + "encoding/hex" "fmt" "os" "path/filepath" @@ -165,7 +166,7 @@ func generateVirtualDomainFromDir(dir, rootDomain string, perPrefixConfig map[st sourcePath := fmt.Sprintf("file://%s", wd+"/"+dir+project) sum := sha256.Sum256([]byte(sourcePath)) - sha := string(sum[:]) + sha := hex.EncodeToString(sum[:]) lookupPath := api.LookupPath{ ProjectID: cfg.projectID, @@ -204,7 +205,7 @@ func customDomain(config projectConfig) responseFn { sourcePath := fmt.Sprintf("file://%s/%s/public.zip", wd, config.pathOnDisk) sum := sha256.Sum256([]byte(sourcePath)) - sha := string(sum[:]) + sha := hex.EncodeToString(sum[:]) return api.VirtualDomain{ Certificate: "", diff --git a/test/acceptance/zip_test.go b/test/acceptance/zip_test.go index fcaecf25..42734d14 100644 --- a/test/acceptance/zip_test.go +++ b/test/acceptance/zip_test.go @@ -1,6 +1,7 @@ package acceptance_test import ( + "fmt" "io" "net" "net/http" @@ -9,9 +10,14 @@ import ( "time" "github.com/stretchr/testify/require" + + "gitlab.com/gitlab-org/gitlab-pages/internal/feature" + "gitlab.com/gitlab-org/gitlab-pages/internal/testhelpers" ) func TestZipServing(t *testing.T) { + testhelpers.StubFeatureFlagValue(t, feature.HandleCacheHeaders.EnvVariable, true) + runObjectStorage(t, "../../shared/pages/group/zip.gitlab.io/public.zip") RunPagesProcess(t, @@ -88,6 +94,11 @@ func TestZipServing(t *testing.T) { require.Equal(t, tt.expectedStatusCode, response.StatusCode) + if tt.expectedStatusCode == http.StatusOK { + require.NotEmpty(t, response.Header.Get("ETag")) + require.NotEmpty(t, response.Header.Get("Last-Modified")) + } + body, err := io.ReadAll(response.Body) require.NoError(t, err) @@ -108,47 +119,132 @@ func TestZipServingCache(t *testing.T) { urlSuffix string expectedStatusCode int expectedContent string - extraHeaders http.Header + extraHeaders func(string) http.Header }{ + "base_domain_if_none_match": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusNotModified, + extraHeaders: func(etag string) http.Header { + return http.Header{ + "If-None-Match": {etag}, + } + }, + }, + "base_domain_if_none_match_fail": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusOK, + expectedContent: "zip.gitlab.io/project/index.html\n", + extraHeaders: func(etag string) http.Header { + return http.Header{ + "If-None-Match": {fmt.Sprintf("%q", "badetag")}, + } + }, + }, + "base_domain_if_match": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusOK, + expectedContent: "zip.gitlab.io/project/index.html\n", + extraHeaders: func(etag string) http.Header { + return http.Header{ + "If-Match": {etag}, + } + }, + }, + "base_domain_if_match_fail": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusPreconditionFailed, + extraHeaders: func(etag string) http.Header { + return http.Header{ + "If-Match": {fmt.Sprintf("%q", "wrongetag")}, + } + }, + }, + "base_domain_if_match_fail2": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusPreconditionFailed, + extraHeaders: func(etag string) http.Header { + return http.Header{ + "If-Match": {","}, + } + }, + }, "base_domain_if_modified": { host: "zip.gitlab.io", urlSuffix: "/", expectedStatusCode: http.StatusNotModified, - extraHeaders: http.Header{ - "If-Modified-Since": {time.Now().Format(http.TimeFormat)}, + extraHeaders: func(string) http.Header { + return http.Header{ + "If-Modified-Since": {time.Now().Format(http.TimeFormat)}, + } + }, + }, + "base_domain_if_modified_fails": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusOK, + expectedContent: "zip.gitlab.io/project/index.html\n", + extraHeaders: func(string) http.Header { + return http.Header{ + "If-Modified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, + } }, }, "base_domain_if_unmodified": { host: "zip.gitlab.io", urlSuffix: "/", expectedStatusCode: http.StatusPreconditionFailed, - extraHeaders: http.Header{ - "If-Unmodified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, + extraHeaders: func(string) http.Header { + return http.Header{ + "If-Unmodified-Since": {time.Now().AddDate(-10, 0, 0).Format(http.TimeFormat)}, + } + }, + }, + "base_domain_if_unmodified_fails": { + host: "zip.gitlab.io", + urlSuffix: "/", + expectedStatusCode: http.StatusOK, + expectedContent: "zip.gitlab.io/project/index.html\n", + extraHeaders: func(string) http.Header { + return http.Header{ + "If-Unmodified-Since": {time.Now().Format(http.TimeFormat)}, + } }, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - response, err := GetPageFromListenerWithHeaders(t, httpListener, tt.host, tt.urlSuffix, tt.extraHeaders) + // send a request to get the ETag + response, err := GetPageFromListener(t, httpListener, tt.host, tt.urlSuffix) require.NoError(t, err) defer response.Body.Close() + require.Equal(t, http.StatusOK, response.StatusCode) - require.Equal(t, tt.expectedStatusCode, response.StatusCode) + etag := response.Header.Get("ETag") + require.NotEmpty(t, etag) - if tt.expectedStatusCode == http.StatusOK { - require.NotEmpty(t, response.Header.Get("Last-Modified")) - } + // actual test + rsp, err := GetPageFromListenerWithHeaders(t, httpListener, tt.host, tt.urlSuffix, tt.extraHeaders(etag)) + require.NoError(t, err) + require.Equal(t, tt.expectedStatusCode, rsp.StatusCode) - body, err := io.ReadAll(response.Body) + body, err := io.ReadAll(rsp.Body) require.NoError(t, err) - require.Contains(t, string(body), tt.expectedContent, "content mismatch") + defer rsp.Body.Close() + require.Equal(t, tt.expectedContent, string(body), "content mismatch") }) } } func TestZipServingFromDisk(t *testing.T) { + testhelpers.StubFeatureFlagValue(t, feature.HandleCacheHeaders.EnvVariable, true) + RunPagesProcess(t, withListeners([]ListenSpec{httpListener}), ) @@ -223,6 +319,11 @@ func TestZipServingFromDisk(t *testing.T) { require.Equal(t, tt.expectedStatusCode, response.StatusCode) + if tt.expectedStatusCode == http.StatusOK { + require.NotEmpty(t, response.Header.Get("ETag")) + require.NotEmpty(t, response.Header.Get("Last-Modified")) + } + body, err := io.ReadAll(response.Body) require.NoError(t, err) |