From a3cc34472b4fe730aa8766d874ebf00c75cef2c8 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:07:59 -0700 Subject: [PATCH] Pass ServeHeaderOptions by value instead of pointer, fine tune httplib tests (#36982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass `ServeHeaderOptions` by value instead of pointer across all call sites — no nil-check semantics are needed and the struct is small enough that copying is fine. ## Changes - **`services/context/base.go`**: `SetServeHeaders` and `ServeContent` accept `ServeHeaderOptions` (value, not pointer); internal unsafe pointer cast replaced with a clean type conversion - **`routers/api/packages/helper/helper.go`**: `ServePackageFile` variadic changed from `...*context.ServeHeaderOptions` to `...context.ServeHeaderOptions`; internal variable is now a value type - **All call sites** (13 files): `&context.ServeHeaderOptions{...}` → `context.ServeHeaderOptions{...}` Before/after at the definition level: ```go // Before func (b *Base) SetServeHeaders(opt *ServeHeaderOptions) { ... } func (b *Base) ServeContent(r io.ReadSeeker, opts *ServeHeaderOptions) { ... } func ServePackageFile(..., forceOpts ...*context.ServeHeaderOptions) { ... } // After func (b *Base) SetServeHeaders(opts ServeHeaderOptions) { ... } func (b *Base) ServeContent(r io.ReadSeeker, opts ServeHeaderOptions) { ... } func ServePackageFile(..., forceOpts ...context.ServeHeaderOptions) { ... } ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com> Co-authored-by: wxiaoguang --- modules/httplib/serve.go | 2 +- routers/api/actions/artifacts.go | 2 +- routers/api/packages/alpine/alpine.go | 2 +- routers/api/packages/arch/arch.go | 4 +- routers/api/packages/debian/debian.go | 4 +- routers/api/packages/helper/helper.go | 6 +- routers/api/packages/maven/maven.go | 2 +- routers/api/packages/rpm/rpm.go | 4 +- routers/api/packages/rubygems/rubygems.go | 4 +- routers/api/packages/swift/swift.go | 4 +- routers/common/actions.go | 2 +- routers/web/user/setting/packages.go | 2 +- services/context/base.go | 10 +- services/repository/archiver/archiver.go | 2 +- tests/integration/download_test.go | 124 +++++++++------------- 15 files changed, 77 insertions(+), 97 deletions(-) diff --git a/modules/httplib/serve.go b/modules/httplib/serve.go index e8299d1c80..8abf6f1887 100644 --- a/modules/httplib/serve.go +++ b/modules/httplib/serve.go @@ -87,7 +87,7 @@ func serveSetHeadersByUserContent(w http.ResponseWriter, contentPrefetchBuf []by if setting.MimeTypeMap.Enabled { fileExtension := strings.ToLower(path.Ext(opts.Filename)) opts.ContentType = setting.MimeTypeMap.Map[fileExtension] - detectCharset = !strings.Contains(opts.ContentType, "charset=") + detectCharset = strings.HasPrefix(opts.ContentType, "text/") && !strings.Contains(opts.ContentType, "charset=") } if opts.ContentType == "" { diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index a6722616cf..13cbecb5cd 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -496,7 +496,7 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) { ctx.Resp.Header().Set("Content-Encoding", "gzip") } log.Debug("[artifact] downloadArtifact, name: %s, path: %s, storage: %s, size: %d", artifact.ArtifactName, artifact.ArtifactPath, artifact.StoragePath, artifact.FileSize) - ctx.ServeContent(fd, &context.ServeHeaderOptions{ + ctx.ServeContent(fd, context.ServeHeaderOptions{ Filename: artifact.ArtifactName, LastModified: artifact.CreatedUnix.AsLocalTime(), }) diff --git a/routers/api/packages/alpine/alpine.go b/routers/api/packages/alpine/alpine.go index f250a1a549..52fc287a0e 100644 --- a/routers/api/packages/alpine/alpine.go +++ b/routers/api/packages/alpine/alpine.go @@ -54,7 +54,7 @@ func GetRepositoryKey(ctx *context.Context) { return } - ctx.ServeContent(strings.NewReader(pub), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(pub), context.ServeHeaderOptions{ ContentType: "application/x-pem-file", Filename: fmt.Sprintf("%s@%s.rsa.pub", ctx.Package.Owner.LowerName, hex.EncodeToString(fingerprint)), }) diff --git a/routers/api/packages/arch/arch.go b/routers/api/packages/arch/arch.go index 5a124f6918..f3b70f39b6 100644 --- a/routers/api/packages/arch/arch.go +++ b/routers/api/packages/arch/arch.go @@ -35,7 +35,7 @@ func GetRepositoryKey(ctx *context.Context) { return } - ctx.ServeContent(strings.NewReader(pub), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(pub), context.ServeHeaderOptions{ ContentType: "application/pgp-keys", }) } @@ -232,7 +232,7 @@ func GetPackageOrRepositoryFile(ctx *context.Context) { return } - ctx.ServeContent(bytes.NewReader(data), &context.ServeHeaderOptions{ + ctx.ServeContent(bytes.NewReader(data), context.ServeHeaderOptions{ Filename: filenameOrig, }) return diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go index 82c7952bdb..785efb6dda 100644 --- a/routers/api/packages/debian/debian.go +++ b/routers/api/packages/debian/debian.go @@ -35,7 +35,7 @@ func GetRepositoryKey(ctx *context.Context) { return } - ctx.ServeContent(strings.NewReader(pub), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(pub), context.ServeHeaderOptions{ ContentType: "application/pgp-keys", Filename: "repository.key", }) @@ -233,7 +233,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - helper.ServePackageFile(ctx, s, u, pf, &context.ServeHeaderOptions{ + helper.ServePackageFile(ctx, s, u, pf, context.ServeHeaderOptions{ ContentType: "application/vnd.debian.binary-package", Filename: pf.Name, LastModified: pf.CreatedUnix.AsLocalTime(), diff --git a/routers/api/packages/helper/helper.go b/routers/api/packages/helper/helper.go index 27d4e6ffdc..01ae5d2b7e 100644 --- a/routers/api/packages/helper/helper.go +++ b/routers/api/packages/helper/helper.go @@ -39,7 +39,7 @@ func ProcessErrorForUser(ctx *context.Context, status int, errObj any) string { // ServePackageFile the content of the package file // If the url is set it will redirect the request, otherwise the content is copied to the response. -func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf *packages_model.PackageFile, forceOpts ...*context.ServeHeaderOptions) { +func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf *packages_model.PackageFile, forceOpts ...context.ServeHeaderOptions) { if u != nil { ctx.Redirect(u.String()) return @@ -47,11 +47,11 @@ func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf defer s.Close() - var opts *context.ServeHeaderOptions + var opts context.ServeHeaderOptions if len(forceOpts) > 0 { opts = forceOpts[0] } else { - opts = &context.ServeHeaderOptions{ + opts = context.ServeHeaderOptions{ Filename: pf.Name, LastModified: pf.CreatedUnix.AsLocalTime(), } diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go index 6c2916908b..446398caf7 100644 --- a/routers/api/packages/maven/maven.go +++ b/routers/api/packages/maven/maven.go @@ -200,7 +200,7 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool return } - opts := &context.ServeHeaderOptions{ + opts := context.ServeHeaderOptions{ ContentLength: &pb.Size, LastModified: pf.CreatedUnix.AsLocalTime(), } diff --git a/routers/api/packages/rpm/rpm.go b/routers/api/packages/rpm/rpm.go index 5abbb0c8ae..4447a0c3cf 100644 --- a/routers/api/packages/rpm/rpm.go +++ b/routers/api/packages/rpm/rpm.go @@ -57,7 +57,7 @@ func GetRepositoryKey(ctx *context.Context) { return } - ctx.ServeContent(strings.NewReader(pub), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(pub), context.ServeHeaderOptions{ ContentType: "application/pgp-keys", Filename: "repository.key", }) @@ -80,7 +80,7 @@ func CheckRepositoryFileExistence(ctx *context.Context) { return } - ctx.SetServeHeaders(&context.ServeHeaderOptions{ + ctx.SetServeHeaders(context.ServeHeaderOptions{ Filename: pf.Name, LastModified: pf.CreatedUnix.AsLocalTime(), }) diff --git a/routers/api/packages/rubygems/rubygems.go b/routers/api/packages/rubygems/rubygems.go index 69764c1df3..fe2e7af6d9 100644 --- a/routers/api/packages/rubygems/rubygems.go +++ b/routers/api/packages/rubygems/rubygems.go @@ -79,7 +79,7 @@ func enumeratePackages(ctx *context.Context, filename string, pvs []*packages_mo }) } - ctx.SetServeHeaders(&context.ServeHeaderOptions{ + ctx.SetServeHeaders(context.ServeHeaderOptions{ Filename: filename + ".gz", }) @@ -119,7 +119,7 @@ func ServePackageSpecification(ctx *context.Context) { return } - ctx.SetServeHeaders(&context.ServeHeaderOptions{ + ctx.SetServeHeaders(context.ServeHeaderOptions{ Filename: filename, }) diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index 66c28c9772..948ece7a27 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -281,7 +281,7 @@ func DownloadManifest(ctx *context.Context) { filename = fmt.Sprintf("Package@swift-%s.swift", swiftVersion) } - ctx.ServeContent(strings.NewReader(m.Content), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(m.Content), context.ServeHeaderOptions{ ContentType: "text/x-swift", Filename: filename, LastModified: pv.CreatedUnix.AsLocalTime(), @@ -437,7 +437,7 @@ func DownloadPackageFile(ctx *context.Context) { Digest: pd.Files[0].Blob.HashSHA256, }) - helper.ServePackageFile(ctx, s, u, pf, &context.ServeHeaderOptions{ + helper.ServePackageFile(ctx, s, u, pf, context.ServeHeaderOptions{ Filename: pf.Name, ContentType: "application/zip", LastModified: pf.CreatedUnix.AsLocalTime(), diff --git a/routers/common/actions.go b/routers/common/actions.go index f698ba9436..4eb7078db6 100644 --- a/routers/common/actions.go +++ b/routers/common/actions.go @@ -58,7 +58,7 @@ func DownloadActionsRunJobLogs(ctx *context.Base, ctxRepo *repo_model.Repository if p := strings.Index(workflowName, "."); p > 0 { workflowName = workflowName[0:p] } - ctx.ServeContent(reader, &context.ServeHeaderOptions{ + ctx.ServeContent(reader, context.ServeHeaderOptions{ Filename: fmt.Sprintf("%v-%v-%v.log", workflowName, curJob.Name, task.ID), ContentLength: &task.LogSize, ContentType: "text/plain; charset=utf-8", diff --git a/routers/web/user/setting/packages.go b/routers/web/user/setting/packages.go index 62b0240642..66aa241377 100644 --- a/routers/web/user/setting/packages.go +++ b/routers/web/user/setting/packages.go @@ -112,7 +112,7 @@ func RegenerateChefKeyPair(ctx *context.Context) { return } - ctx.ServeContent(strings.NewReader(priv), &context.ServeHeaderOptions{ + ctx.ServeContent(strings.NewReader(priv), context.ServeHeaderOptions{ ContentType: "application/x-pem-file", Filename: ctx.Doer.Name + ".priv", }) diff --git a/services/context/base.go b/services/context/base.go index 06ccefa3aa..8d44de5bc7 100644 --- a/services/context/base.go +++ b/services/context/base.go @@ -170,15 +170,15 @@ func (b *Base) Redirect(location string, status ...int) { http.Redirect(b.Resp, b.Req, location, code) } -type ServeHeaderOptions httplib.ServeHeaderOptions +type ServeHeaderOptions = httplib.ServeHeaderOptions -func (b *Base) SetServeHeaders(opt *ServeHeaderOptions) { - httplib.ServeSetHeaders(b.Resp, *(*httplib.ServeHeaderOptions)(opt)) +func (b *Base) SetServeHeaders(opts ServeHeaderOptions) { + httplib.ServeSetHeaders(b.Resp, opts) } // ServeContent serves content to http request -func (b *Base) ServeContent(r io.ReadSeeker, opts *ServeHeaderOptions) { - httplib.ServeSetHeaders(b.Resp, *(*httplib.ServeHeaderOptions)(opts)) +func (b *Base) ServeContent(r io.ReadSeeker, opts ServeHeaderOptions) { + httplib.ServeSetHeaders(b.Resp, opts) http.ServeContent(b.Resp, b.Req, opts.Filename, opts.LastModified, r) } diff --git a/services/repository/archiver/archiver.go b/services/repository/archiver/archiver.go index 2431ae4b93..f7069f226b 100644 --- a/services/repository/archiver/archiver.go +++ b/services/repository/archiver/archiver.go @@ -359,7 +359,7 @@ func ServeRepoArchive(ctx *gitea_context.Base, archiveReq *ArchiveRequest) error } defer fr.Close() - ctx.ServeContent(fr, &gitea_context.ServeHeaderOptions{ + ctx.ServeContent(fr, gitea_context.ServeHeaderOptions{ Filename: downloadName, LastModified: archiver.CreatedUnix.AsLocalTime(), }) diff --git a/tests/integration/download_test.go b/tests/integration/download_test.go index efe5ac791c..3e7be98b09 100644 --- a/tests/integration/download_test.go +++ b/tests/integration/download_test.go @@ -8,86 +8,66 @@ import ( "testing" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) -func TestDownloadByID(t *testing.T) { +func TestDownloadRepoContent(t *testing.T) { defer tests.PrepareTestEnv(t)() session := loginUser(t, "user2") - // Request raw blob - req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f") - resp := session.MakeRequest(t, req, http.StatusOK) + t.Run("RawBlob", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo1/raw/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + }) - assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) -} - -func TestDownloadByIDForSVGUsesSecureHeaders(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - session := loginUser(t, "user2") - - // Request raw blob - req := NewRequest(t, "GET", "/user2/repo2/raw/blob/6395b68e1feebb1e4c657b4f9f6ba2676a283c0b") - resp := session.MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, "default-src 'none'; style-src 'unsafe-inline'; sandbox", resp.Header().Get("Content-Security-Policy")) - assert.Equal(t, "image/svg+xml", resp.Header().Get("Content-Type")) - assert.Equal(t, "nosniff", resp.Header().Get("X-Content-Type-Options")) -} - -func TestDownloadByIDMedia(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - session := loginUser(t, "user2") - - // Request raw blob - req := NewRequest(t, "GET", "/user2/repo1/media/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f") - resp := session.MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) -} - -func TestDownloadByIDMediaForSVGUsesSecureHeaders(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - session := loginUser(t, "user2") - - // Request raw blob - req := NewRequest(t, "GET", "/user2/repo2/media/blob/6395b68e1feebb1e4c657b4f9f6ba2676a283c0b") - resp := session.MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, "default-src 'none'; style-src 'unsafe-inline'; sandbox", resp.Header().Get("Content-Security-Policy")) - assert.Equal(t, "image/svg+xml", resp.Header().Get("Content-Type")) - assert.Equal(t, "nosniff", resp.Header().Get("X-Content-Type-Options")) -} - -func TestDownloadRawTextFileWithoutMimeTypeMapping(t *testing.T) { - defer tests.PrepareTestEnv(t)() - - session := loginUser(t, "user2") - - req := NewRequest(t, "GET", "/user2/repo2/raw/branch/master/test.xml") - resp := session.MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) -} - -func TestDownloadRawTextFileWithMimeTypeMapping(t *testing.T) { - defer tests.PrepareTestEnv(t)() - setting.MimeTypeMap.Map[".xml"] = "text/xml" - setting.MimeTypeMap.Enabled = true - - session := loginUser(t, "user2") - - req := NewRequest(t, "GET", "/user2/repo2/raw/branch/master/test.xml") - resp := session.MakeRequest(t, req, http.StatusOK) - - assert.Equal(t, "text/xml; charset=utf-8", resp.Header().Get("Content-Type")) - - delete(setting.MimeTypeMap.Map, ".xml") - setting.MimeTypeMap.Enabled = false + t.Run("SVGUsesSecureHeaders", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo2/raw/blob/6395b68e1feebb1e4c657b4f9f6ba2676a283c0b") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "default-src 'none'; style-src 'unsafe-inline'; sandbox", resp.Header().Get("Content-Security-Policy")) + assert.Equal(t, "image/svg+xml", resp.Header().Get("Content-Type")) + assert.Equal(t, "nosniff", resp.Header().Get("X-Content-Type-Options")) + }) + + t.Run("MediaBlob", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo1/media/blob/4b4851ad51df6a7d9f25c979345979eaeb5b349f") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "# repo1\n\nDescription for repo1", resp.Body.String()) + }) + + t.Run("MediaSVGUsesSecureHeaders", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo2/media/blob/6395b68e1feebb1e4c657b4f9f6ba2676a283c0b") + resp := session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "default-src 'none'; style-src 'unsafe-inline'; sandbox", resp.Header().Get("Content-Security-Policy")) + assert.Equal(t, "image/svg+xml", resp.Header().Get("Content-Type")) + assert.Equal(t, "nosniff", resp.Header().Get("X-Content-Type-Options")) + }) + + t.Run("MimeTypeMap", func(t *testing.T) { + req := NewRequest(t, "GET", "/user2/repo2/raw/branch/master/test.xml") + resp := session.MakeRequest(t, req, http.StatusOK) + // although the file is a valid XML file, it is served as "text/plain" to avoid site content spamming (the same to "text/html" files) + assert.Equal(t, "text/plain; charset=utf-8", resp.Header().Get("Content-Type")) + + defer tests.PrepareTestEnv(t)() + defer test.MockVariableValue(&setting.MimeTypeMap)() + setting.MimeTypeMap.Enabled = true + + setting.MimeTypeMap.Map[".xml"] = "text/xml" + req = NewRequest(t, "GET", "/user2/repo2/raw/branch/master/test.xml") + resp = session.MakeRequest(t, req, http.StatusOK) + // respect the mime mapping, and "text/plain" protection isn't used anymore + assert.Equal(t, "text/xml; charset=utf-8", resp.Header().Get("Content-Type")) + assert.Equal(t, "inline; filename=test.xml", resp.Header().Get("Content-Disposition")) + + setting.MimeTypeMap.Map[".xml"] = "application/xml" + req = NewRequest(t, "GET", "/user2/repo2/raw/branch/master/test.xml") + resp = session.MakeRequest(t, req, http.StatusOK) + // non-text file don't have "charset" + assert.Equal(t, "application/xml", resp.Header().Get("Content-Type")) + }) }