diff --git a/modules/httpcache/httpcache.go b/modules/httpcache/httpcache.go index 785363c29ca..3f23938be2f 100644 --- a/modules/httpcache/httpcache.go +++ b/modules/httpcache/httpcache.go @@ -55,13 +55,20 @@ func CacheControlForPrivateStatic() *CacheControlOptions { // checkIfNoneMatchIsValid tests if the header If-None-Match matches the ETag func checkIfNoneMatchIsValid(req *http.Request, etag string) bool { - ifNoneMatch := req.Header.Get("If-None-Match") - if len(ifNoneMatch) > 0 { - for item := range strings.SplitSeq(ifNoneMatch, ",") { - item = strings.TrimPrefix(strings.TrimSpace(item), "W/") // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag#directives - if item == etag { - return true - } + ifNoneMatch := strings.TrimSpace(req.Header.Get("If-None-Match")) + if ifNoneMatch == "" { + return false + } + // https://www.rfc-editor.org/rfc/rfc9110#section-13.1.2 + if ifNoneMatch == "*" { + return true + } + // https://www.rfc-editor.org/rfc/rfc9110#section-8.8.3.2 + etag = strings.TrimPrefix(etag, "W/") + for item := range strings.SplitSeq(ifNoneMatch, ",") { + item = strings.TrimPrefix(strings.TrimSpace(item), "W/") + if item == etag { + return true } } return false diff --git a/modules/httpcache/httpcache_test.go b/modules/httpcache/httpcache_test.go index d715cac4a24..353d9ee62da 100644 --- a/modules/httpcache/httpcache_test.go +++ b/modules/httpcache/httpcache_test.go @@ -16,11 +16,13 @@ import ( func TestHandleGenericETagCache(t *testing.T) { matchedEtag := `"matched-etag"` + weakEtag := `W/"matched-etag"` lastModifiedTime := new(time.Date(2021, time.January, 2, 15, 4, 5, 0, time.FixedZone("test-zone", 8*3600))) lastModified := lastModifiedTime.UTC().Format(http.TimeFormat) cacheControl := "max-age=0, private, must-revalidate" type testCase struct { name string + respEtag string reqHeaders map[string]string wantHandled bool wantHeaders map[string]string @@ -51,6 +53,29 @@ func TestHandleGenericETagCache(t *testing.T) { wantHandled: false, wantHeaders: map[string]string{"Last-Modified": lastModified, "Cache-Control": cacheControl, "Etag": matchedEtag}, }, + { + name: "Wildcard If-None-Match matches any current representation", + reqHeaders: map[string]string{"If-None-Match": "*"}, + wantHandled: true, + wantHeaders: map[string]string{"Last-Modified": lastModified, "Cache-Control": "", "Etag": matchedEtag}, + wantStatus: http.StatusNotModified, + }, + { + name: "Weak response tag matched by a strong request tag", + respEtag: weakEtag, + reqHeaders: map[string]string{"If-None-Match": matchedEtag}, + wantHandled: true, + wantHeaders: map[string]string{"Last-Modified": lastModified, "Cache-Control": "", "Etag": weakEtag}, + wantStatus: http.StatusNotModified, + }, + { + name: "Weak response tag matched by a weak request tag", + respEtag: weakEtag, + reqHeaders: map[string]string{"If-None-Match": weakEtag}, + wantHandled: true, + wantHeaders: map[string]string{"Last-Modified": lastModified, "Cache-Control": "", "Etag": weakEtag}, + wantStatus: http.StatusNotModified, + }, { name: "Multiple Matched If-None-Match", reqHeaders: map[string]string{"If-None-Match": `"mismatched-etag", ` + matchedEtag}, @@ -67,7 +92,7 @@ func TestHandleGenericETagCache(t *testing.T) { req.Header.Set(k, v) } w := httptest.NewRecorder() - assert.Equal(t, tc.wantHandled, HandleGenericETagPrivateCache(req, w, matchedEtag, lastModifiedTime)) + assert.Equal(t, tc.wantHandled, HandleGenericETagPrivateCache(req, w, util.IfZero(tc.respEtag, matchedEtag), lastModifiedTime)) resp := w.Result() for k, v := range tc.wantHeaders { assert.Equal(t, v, resp.Header.Get(k)) diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index 2a0a4b3d70a..e2b6eeb8d8d 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -17,6 +17,7 @@ import ( "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/graceful" + "gitea.dev/modules/httpcache" "gitea.dev/modules/httplib" "gitea.dev/modules/log" "gitea.dev/modules/process" @@ -363,6 +364,11 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error downloadName := archiveReq.Repo.Name + "-" + archiveReq.GetArchiveName() if setting.Repository.StreamArchives || len(archiveReq.Paths) > 0 { + // Use weak ETag because the bytes also depend on the git version, compression level and repo config + etag := fmt.Sprintf(`W/"%s-%s-%t"`, archiveReq.CommitID, archiveReq.Type.String(), setting.Repository.PrefixArchiveFiles) + if len(archiveReq.Paths) == 0 && httpcache.HandleGenericETagPrivateCache(ctx.Req, ctx.Resp, etag, nil) { + return nil + } // the header must be set before starting streaming even an error would occur, // because errors may happen in git command and such cases aren't in our control. httplib.ServeSetHeaders(ctx.Resp, httplib.ServeHeaderOptions{Filename: downloadName}) diff --git a/tests/integration/repo_archive_test.go b/tests/integration/repo_archive_test.go index fd8403496c5..c667ffa669d 100644 --- a/tests/integration/repo_archive_test.go +++ b/tests/integration/repo_archive_test.go @@ -6,6 +6,7 @@ package integration import ( "io" "net/http" + "strings" "testing" "gitea.dev/modules/setting" @@ -34,9 +35,26 @@ func TestRepoDownloadArchive(t *testing.T) { assert.Len(t, bs, 320) }) + t.Run("Conditional", func(t *testing.T) { + defer test.MockVariableValue(&setting.Repository.StreamArchives, true)() + archiveURL := "/user2/glob/archive/master.tar.gz" + etag := MakeRequest(t, NewRequest(t, "GET", archiveURL), http.StatusOK).Header().Get("ETag") + assert.True(t, strings.HasPrefix(etag, `W/"`)) + MakeRequest(t, NewRequest(t, "GET", archiveURL).SetHeader("If-None-Match", etag), http.StatusNotModified) + MakeRequest(t, NewRequest(t, "GET", archiveURL+"?path=aaa.doc").SetHeader("If-None-Match", etag), http.StatusOK) + assert.NotEqual(t, etag, MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/archive/master.tar.gz"), http.StatusOK).Header().Get("ETag")) + + restoreStreaming := test.MockVariableValue(&setting.Repository.StreamArchives, false) + MakeRequest(t, NewRequest(t, "GET", archiveURL).SetHeader("If-None-Match", etag), http.StatusOK) + restoreStreaming() + + defer test.MockVariableValue(&setting.Repository.PrefixArchiveFiles, !setting.Repository.PrefixArchiveFiles)() + MakeRequest(t, NewRequest(t, "GET", archiveURL).SetHeader("If-None-Match", etag), http.StatusOK) + }) + t.Run("SubPath", func(t *testing.T) { // When using "archiving and caching" approach, archiving with paths will always use streaming and never be cached - defer test.MockVariableValue(&setting.Repository.StreamArchives, false) // this can be removed if there is always streaming mode + defer test.MockVariableValue(&setting.Repository.StreamArchives, false)() // this can be removed if there is always streaming mode req := NewRequest(t, "GET", "/user2/glob/archive/master.tar.gz?path=aaa.doc&path=x/y") resp := MakeRequest(t, req, http.StatusOK) content, err := test.ReadAllTarGzContent(resp.Body)