feat: implement ETag support for zip serving

Changelog: added

2 files changed, 90 insertions, 10 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()) {