From c890454769562e0ec2978e123aaf3d9a43e5ef4f Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Mon, 3 Jul 2023 15:33:28 +0200 Subject: [PATCH] Add direct serving of package content (#25543) Fixes #24723 Direct serving of content aka HTTP redirect is not mentioned in any of the package registry specs but lots of official registries do that so it should be supported by the usual clients. --- modules/packages/content_store.go | 10 +++ routers/api/packages/alpine/alpine.go | 16 +--- routers/api/packages/cargo/cargo.go | 8 +- routers/api/packages/chef/chef.go | 8 +- routers/api/packages/composer/composer.go | 8 +- routers/api/packages/conan/conan.go | 8 +- routers/api/packages/conda/conda.go | 8 +- routers/api/packages/container/container.go | 64 ++++++++-------- routers/api/packages/cran/cran.go | 8 +- routers/api/packages/debian/debian.go | 21 ++---- routers/api/packages/generic/generic.go | 8 +- routers/api/packages/goproxy/goproxy.go | 8 +- routers/api/packages/helm/helm.go | 8 +- routers/api/packages/helper/helper.go | 26 +++++++ routers/api/packages/maven/maven.go | 12 +-- routers/api/packages/npm/npm.go | 16 +--- routers/api/packages/nuget/nuget.go | 16 +--- routers/api/packages/pub/pub.go | 8 +- routers/api/packages/pypi/pypi.go | 8 +- routers/api/packages/rpm/rpm.go | 17 +---- routers/api/packages/rubygems/rubygems.go | 8 +- routers/api/packages/swift/swift.go | 5 +- routers/api/packages/vagrant/vagrant.go | 8 +- routers/web/user/package.go | 12 +-- services/packages/packages.go | 74 +++++++++---------- .../integration/api_packages_generic_test.go | 37 ++++++++++ 26 files changed, 195 insertions(+), 235 deletions(-) diff --git a/modules/packages/content_store.go b/modules/packages/content_store.go index 1181fa4d5..da93e6cf6 100644 --- a/modules/packages/content_store.go +++ b/modules/packages/content_store.go @@ -5,9 +5,11 @@ package packages import ( "io" + "net/url" "path" "strings" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" ) @@ -31,6 +33,14 @@ func (s *ContentStore) Get(key BlobHash256Key) (storage.Object, error) { return s.store.Open(KeyToRelativePath(key)) } +func (s *ContentStore) ShouldServeDirect() bool { + return setting.Packages.Storage.MinioConfig.ServeDirect +} + +func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string) (*url.URL, error) { + return s.store.URL(KeyToRelativePath(key), filename) +} + // FIXME: Workaround to be removed in v1.20 // https://github.com/go-gitea/gitea/issues/19586 func (s *ContentStore) Has(key BlobHash256Key) error { diff --git a/routers/api/packages/alpine/alpine.go b/routers/api/packages/alpine/alpine.go index 9a551a219..e357e9cb9 100644 --- a/routers/api/packages/alpine/alpine.go +++ b/routers/api/packages/alpine/alpine.go @@ -68,7 +68,7 @@ func GetRepositoryFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pv, &packages_service.PackageFileInfo{ @@ -84,12 +84,8 @@ func GetRepositoryFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } func UploadPackageFile(ctx *context.Context) { @@ -200,7 +196,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) + s, u, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) @@ -209,12 +205,8 @@ func DownloadPackageFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } func DeletePackageFile(ctx *context.Context) { diff --git a/routers/api/packages/cargo/cargo.go b/routers/api/packages/cargo/cargo.go index b666bdde6..a0a0cea92 100644 --- a/routers/api/packages/cargo/cargo.go +++ b/routers/api/packages/cargo/cargo.go @@ -165,7 +165,7 @@ func ListOwners(ctx *context.Context) { // DownloadPackageFile serves the content of a package func DownloadPackageFile(ctx *context.Context) { - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -185,12 +185,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // https://doc.rust-lang.org/cargo/reference/registries.html#publish diff --git a/routers/api/packages/chef/chef.go b/routers/api/packages/chef/chef.go index b48b1778c..355f01a8f 100644 --- a/routers/api/packages/chef/chef.go +++ b/routers/api/packages/chef/chef.go @@ -341,17 +341,13 @@ func DownloadPackage(ctx *context.Context) { pf := pd.Files[0].File - s, _, err := packages_service.GetPackageFileStream(ctx, pf) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // https://github.com/chef/chef/blob/main/knife/lib/chef/knife/supermarket_unshare.rb diff --git a/routers/api/packages/composer/composer.go b/routers/api/packages/composer/composer.go index d93b11efd..06b4f4652 100644 --- a/routers/api/packages/composer/composer.go +++ b/routers/api/packages/composer/composer.go @@ -162,7 +162,7 @@ func PackageMetadata(ctx *context.Context) { // DownloadPackageFile serves the content of a package func DownloadPackageFile(ctx *context.Context) { - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -182,12 +182,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage creates a new package diff --git a/routers/api/packages/conan/conan.go b/routers/api/packages/conan/conan.go index caeb8c11b..616e57b36 100644 --- a/routers/api/packages/conan/conan.go +++ b/routers/api/packages/conan/conan.go @@ -453,7 +453,7 @@ func downloadFile(ctx *context.Context, fileFilter container.Set[string], fileKe return } - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -474,12 +474,8 @@ func downloadFile(ctx *context.Context, fileFilter container.Set[string], fileKe apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // DeleteRecipeV1 deletes the requested recipe(s) diff --git a/routers/api/packages/conda/conda.go b/routers/api/packages/conda/conda.go index f77869063..9c5edd548 100644 --- a/routers/api/packages/conda/conda.go +++ b/routers/api/packages/conda/conda.go @@ -292,15 +292,11 @@ func DownloadPackageFile(ctx *context.Context) { pf := pfs[0] - s, _, err := packages_service.GetPackageFileStream(ctx, pf) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 63c49809a..07cf387dd 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -482,22 +482,7 @@ func GetBlob(ctx *context.Context) { return } - s, _, err := packages_service.GetPackageFileStream(ctx, blob.File) - if err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - defer s.Close() - - setResponseHeaders(ctx.Resp, &containerHeaders{ - ContentDigest: blob.Properties.GetByName(container_module.PropertyDigest), - ContentType: blob.Properties.GetByName(container_module.PropertyMediaType), - ContentLength: blob.Blob.Size, - Status: http.StatusOK, - }) - if _, err := io.Copy(ctx.Resp, s); err != nil { - log.Error("Error whilst copying content to response: %v", err) - } + serveBlob(ctx, blob) } // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#deleting-blobs @@ -636,22 +621,7 @@ func GetManifest(ctx *context.Context) { return } - s, _, err := packages_service.GetPackageFileStream(ctx, manifest.File) - if err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - defer s.Close() - - setResponseHeaders(ctx.Resp, &containerHeaders{ - ContentDigest: manifest.Properties.GetByName(container_module.PropertyDigest), - ContentType: manifest.Properties.GetByName(container_module.PropertyMediaType), - ContentLength: manifest.Blob.Size, - Status: http.StatusOK, - }) - if _, err := io.Copy(ctx.Resp, s); err != nil { - log.Error("Error whilst copying content to response: %v", err) - } + serveBlob(ctx, manifest) } // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#deleting-tags @@ -686,6 +656,36 @@ func DeleteManifest(ctx *context.Context) { }) } +func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) { + s, u, _, err := packages_service.GetPackageBlobStream(ctx, pfd.File, pfd.Blob) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + return + } + + headers := &containerHeaders{ + ContentDigest: pfd.Properties.GetByName(container_module.PropertyDigest), + ContentType: pfd.Properties.GetByName(container_module.PropertyMediaType), + ContentLength: pfd.Blob.Size, + Status: http.StatusOK, + } + + if u != nil { + headers.Status = http.StatusTemporaryRedirect + headers.Location = u.String() + + setResponseHeaders(ctx.Resp, headers) + return + } + + defer s.Close() + + setResponseHeaders(ctx.Resp, headers) + if _, err := io.Copy(ctx.Resp, s); err != nil { + log.Error("Error whilst copying content to response: %v", err) + } +} + // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#content-discovery func GetTagList(ctx *context.Context) { image := ctx.Params("image") diff --git a/routers/api/packages/cran/cran.go b/routers/api/packages/cran/cran.go index eb3f9a452..76de3b787 100644 --- a/routers/api/packages/cran/cran.go +++ b/routers/api/packages/cran/cran.go @@ -249,7 +249,7 @@ func downloadPackageFile(ctx *context.Context, opts *cran_model.SearchOptions) { return } - s, _, err := packages_service.GetPackageFileStream(ctx, pf) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) @@ -258,10 +258,6 @@ func downloadPackageFile(ctx *context.Context, opts *cran_model.SearchOptions) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go index cfc03ae52..676816cf7 100644 --- a/routers/api/packages/debian/debian.go +++ b/routers/api/packages/debian/debian.go @@ -59,7 +59,7 @@ func GetRepositoryFile(ctx *context.Context) { key += "|" + component + "|" + architecture } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pv, &packages_service.PackageFileInfo{ @@ -75,12 +75,8 @@ func GetRepositoryFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // https://wiki.debian.org/DebianRepository/Format#indices_acquisition_via_hashsums_.28by-hash.29 @@ -110,7 +106,7 @@ func GetRepositoryFileByHash(ctx *context.Context) { return } - s, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) + s, u, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) @@ -119,12 +115,8 @@ func GetRepositoryFileByHash(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } func UploadPackageFile(ctx *context.Context) { @@ -217,7 +209,7 @@ func DownloadPackageFile(ctx *context.Context) { name := ctx.Params("name") version := ctx.Params("version") - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -238,9 +230,8 @@ func DownloadPackageFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &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/generic/generic.go b/routers/api/packages/generic/generic.go index 0c873119e..7cd1d1a5b 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -30,7 +30,7 @@ func apiError(ctx *context.Context, status int, obj interface{}) { // DownloadPackageFile serves the specific generic package. func DownloadPackageFile(ctx *context.Context) { - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -50,12 +50,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage uploads the specific generic package. diff --git a/routers/api/packages/goproxy/goproxy.go b/routers/api/packages/goproxy/goproxy.go index d0bc9c1e9..350d2a389 100644 --- a/routers/api/packages/goproxy/goproxy.go +++ b/routers/api/packages/goproxy/goproxy.go @@ -105,7 +105,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, _, err := packages_service.GetPackageFileStream(ctx, pfs[0]) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pfs[0]) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) @@ -114,12 +114,8 @@ func DownloadPackageFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pfs[0].Name, - LastModified: pfs[0].CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pfs[0]) } func resolvePackage(ctx *context.Context, ownerID int64, name, version string) (*packages_model.PackageVersion, error) { diff --git a/routers/api/packages/helm/helm.go b/routers/api/packages/helm/helm.go index b7edc8b7f..b50059951 100644 --- a/routers/api/packages/helm/helm.go +++ b/routers/api/packages/helm/helm.go @@ -121,7 +121,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pvs[0], &packages_service.PackageFileInfo{ @@ -136,12 +136,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage creates a new package diff --git a/routers/api/packages/helper/helper.go b/routers/api/packages/helper/helper.go index 660aaec1a..3dec07f48 100644 --- a/routers/api/packages/helper/helper.go +++ b/routers/api/packages/helper/helper.go @@ -5,8 +5,11 @@ package helper import ( "fmt" + "io" "net/http" + "net/url" + packages_model "code.gitea.io/gitea/models/packages" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -35,3 +38,26 @@ func LogAndProcessError(ctx *context.Context, status int, obj interface{}, cb fu cb(message) } } + +// Serves 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) { + if u != nil { + ctx.Redirect(u.String()) + return + } + + defer s.Close() + + var opts *context.ServeHeaderOptions + if len(forceOpts) > 0 { + opts = forceOpts[0] + } else { + opts = &context.ServeHeaderOptions{ + Filename: pf.Name, + LastModified: pf.CreatedUnix.AsLocalTime(), + } + } + + ctx.ServeContent(s, opts) +} diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go index dd270ff0e..215cfa7e1 100644 --- a/routers/api/packages/maven/maven.go +++ b/routers/api/packages/maven/maven.go @@ -210,21 +210,15 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool return } - s, err := packages_module.NewContentStore().Get(packages_module.BlobHash256Key(pb.HashSHA256)) + s, u, _, err := packages_service.GetPackageBlobStream(ctx, pf, pb) if err != nil { apiError(ctx, http.StatusInternalServerError, err) - } - defer s.Close() - - if pf.IsLead { - if err := packages_model.IncrementDownloadCounter(ctx, pv.ID); err != nil { - log.Error("Error incrementing download counter: %v", err) - } + return } opts.Filename = pf.Name - ctx.ServeContent(s, opts) + helper.ServePackageFile(ctx, s, u, pf, opts) } // UploadPackageFile adds a file to the package. If the package does not exist, it gets created. diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go index 89476a776..77a820d27 100644 --- a/routers/api/packages/npm/npm.go +++ b/routers/api/packages/npm/npm.go @@ -83,7 +83,7 @@ func DownloadPackageFile(ctx *context.Context) { packageVersion := ctx.Params("version") filename := ctx.Params("filename") - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -103,12 +103,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // DownloadPackageFileByName finds the version and serves the contents of a package @@ -134,7 +130,7 @@ func DownloadPackageFileByName(ctx *context.Context) { return } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pvs[0], &packages_service.PackageFileInfo{ @@ -149,12 +145,8 @@ func DownloadPackageFileByName(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage creates a new package diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 716d8a969..167776a38 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -362,7 +362,7 @@ func DownloadPackageFile(ctx *context.Context) { packageVersion := ctx.Params("version") filename := ctx.Params("filename") - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -382,12 +382,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackage creates a new package with the metadata contained in the uploaded nupgk file @@ -600,7 +596,7 @@ func DownloadSymbolFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) + s, u, pf, err := packages_service.GetPackageFileStream(ctx, pfs[0]) if err != nil { if err == packages_model.ErrPackageNotExist || err == packages_model.ErrPackageFileNotExist { apiError(ctx, http.StatusNotFound, err) @@ -609,12 +605,8 @@ func DownloadSymbolFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // DeletePackage hard deletes the package diff --git a/routers/api/packages/pub/pub.go b/routers/api/packages/pub/pub.go index ae0c6e785..26fcd53c4 100644 --- a/routers/api/packages/pub/pub.go +++ b/routers/api/packages/pub/pub.go @@ -273,15 +273,11 @@ func DownloadPackageFile(ctx *context.Context) { pf := pd.Files[0].File - s, _, err := packages_service.GetPackageFileStream(ctx, pf) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go index 90a37ec2a..3ae5470ce 100644 --- a/routers/api/packages/pypi/pypi.go +++ b/routers/api/packages/pypi/pypi.go @@ -80,7 +80,7 @@ func DownloadPackageFile(ctx *context.Context) { packageVersion := ctx.Params("version") filename := ctx.Params("filename") - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -100,12 +100,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackageFile adds a file to the package. If the package does not exist, it gets created. diff --git a/routers/api/packages/rpm/rpm.go b/routers/api/packages/rpm/rpm.go index 73e457237..b4c62e225 100644 --- a/routers/api/packages/rpm/rpm.go +++ b/routers/api/packages/rpm/rpm.go @@ -65,7 +65,7 @@ func GetRepositoryFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pv, &packages_service.PackageFileInfo{ @@ -80,12 +80,8 @@ func GetRepositoryFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } func UploadPackageFile(ctx *context.Context) { @@ -173,7 +169,7 @@ func DownloadPackageFile(ctx *context.Context) { name := ctx.Params("name") version := ctx.Params("version") - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -193,13 +189,8 @@ func DownloadPackageFile(ctx *context.Context) { } return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - ContentType: "application/x-rpm", - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } func DeletePackageFile(webctx *context.Context) { diff --git a/routers/api/packages/rubygems/rubygems.go b/routers/api/packages/rubygems/rubygems.go index 740efa9ba..fd5be9730 100644 --- a/routers/api/packages/rubygems/rubygems.go +++ b/routers/api/packages/rubygems/rubygems.go @@ -175,7 +175,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, pf, err := packages_service.GetFileStreamByPackageVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageVersion( ctx, pvs[0], &packages_service.PackageFileInfo{ @@ -190,12 +190,8 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } // UploadPackageFile adds a file to the package. If the package does not exist, it gets created. diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index 06f592dd6..263235a0c 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -397,18 +397,17 @@ func DownloadPackageFile(ctx *context.Context) { pf := pd.Files[0].File - s, _, err := packages_service.GetPackageFileStream(ctx, pf) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() setResponseHeaders(ctx.Resp, &headers{ Digest: pd.Files[0].Blob.HashSHA256, }) - ctx.ServeContent(s, &context.ServeHeaderOptions{ + helper.ServePackageFile(ctx, s, u, pf, &context.ServeHeaderOptions{ Filename: pf.Name, ContentType: "application/zip", LastModified: pf.CreatedUnix.AsLocalTime(), diff --git a/routers/api/packages/vagrant/vagrant.go b/routers/api/packages/vagrant/vagrant.go index cefdc45b1..0decb2c02 100644 --- a/routers/api/packages/vagrant/vagrant.go +++ b/routers/api/packages/vagrant/vagrant.go @@ -216,7 +216,7 @@ func UploadPackageFile(ctx *context.Context) { } func DownloadPackageFile(ctx *context.Context) { - s, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( + s, u, pf, err := packages_service.GetFileStreamByPackageNameAndVersion( ctx, &packages_service.PackageInfo{ Owner: ctx.Package.Owner, @@ -236,10 +236,6 @@ func DownloadPackageFile(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + helper.ServePackageFile(ctx, s, u, pf) } diff --git a/routers/web/user/package.go b/routers/web/user/package.go index 20141914b..551e7f54c 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" + packages_helper "code.gitea.io/gitea/routers/api/packages/helper" shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/forms" packages_service "code.gitea.io/gitea/services/packages" @@ -443,18 +444,11 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, _, err := packages_service.GetPackageFileStream( - ctx, - pf, - ) + s, u, _, err := packages_service.GetPackageFileStream(ctx, pf) if err != nil { ctx.ServerError("GetPackageFileStream", err) return } - defer s.Close() - ctx.ServeContent(s, &context.ServeHeaderOptions{ - Filename: pf.Name, - LastModified: pf.CreatedUnix.AsLocalTime(), - }) + packages_helper.ServePackageFile(ctx, s, u, pf) } diff --git a/services/packages/packages.go b/services/packages/packages.go index 23aa8a5c3..e6d3b0fe5 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "net/url" "strings" "code.gitea.io/gitea/models/db" @@ -20,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/notification" packages_module "code.gitea.io/gitea/modules/packages" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" ) @@ -562,70 +564,62 @@ func DeletePackageFile(ctx context.Context, pf *packages_model.PackageFile) erro } // GetFileStreamByPackageNameAndVersion returns the content of the specific package file -func GetFileStreamByPackageNameAndVersion(ctx context.Context, pvi *PackageInfo, pfi *PackageFileInfo) (io.ReadSeekCloser, *packages_model.PackageFile, error) { +func GetFileStreamByPackageNameAndVersion(ctx context.Context, pvi *PackageInfo, pfi *PackageFileInfo) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { log.Trace("Getting package file stream: %v, %v, %s, %s, %s, %s", pvi.Owner.ID, pvi.PackageType, pvi.Name, pvi.Version, pfi.Filename, pfi.CompositeKey) pv, err := packages_model.GetVersionByNameAndVersion(ctx, pvi.Owner.ID, pvi.PackageType, pvi.Name, pvi.Version) if err != nil { if err == packages_model.ErrPackageNotExist { - return nil, nil, err + return nil, nil, nil, err } log.Error("Error getting package: %v", err) - return nil, nil, err + return nil, nil, nil, err } return GetFileStreamByPackageVersion(ctx, pv, pfi) } -// GetFileStreamByPackageVersionAndFileID returns the content of the specific package file -func GetFileStreamByPackageVersionAndFileID(ctx context.Context, owner *user_model.User, versionID, fileID int64) (io.ReadSeekCloser, *packages_model.PackageFile, error) { - log.Trace("Getting package file stream: %v, %v, %v", owner.ID, versionID, fileID) - - pv, err := packages_model.GetVersionByID(ctx, versionID) - if err != nil { - if err != packages_model.ErrPackageNotExist { - log.Error("Error getting package version: %v", err) - } - return nil, nil, err - } - - p, err := packages_model.GetPackageByID(ctx, pv.PackageID) - if err != nil { - log.Error("Error getting package: %v", err) - return nil, nil, err - } - - if p.OwnerID != owner.ID { - return nil, nil, packages_model.ErrPackageNotExist - } - - pf, err := packages_model.GetFileForVersionByID(ctx, versionID, fileID) - if err != nil { - log.Error("Error getting file: %v", err) - return nil, nil, err - } - - return GetPackageFileStream(ctx, pf) -} - // GetFileStreamByPackageVersion returns the content of the specific package file -func GetFileStreamByPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pfi *PackageFileInfo) (io.ReadSeekCloser, *packages_model.PackageFile, error) { +func GetFileStreamByPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pfi *PackageFileInfo) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { pf, err := packages_model.GetFileForVersionByName(ctx, pv.ID, pfi.Filename, pfi.CompositeKey) if err != nil { - return nil, nil, err + return nil, nil, nil, err } return GetPackageFileStream(ctx, pf) } // GetPackageFileStream returns the content of the specific package file -func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) (io.ReadSeekCloser, *packages_model.PackageFile, error) { +func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { pb, err := packages_model.GetBlobByID(ctx, pf.BlobID) if err != nil { - return nil, nil, err + return nil, nil, nil, err + } + + return GetPackageBlobStream(ctx, pf, pb) +} + +// GetPackageBlobStream returns the content of the specific package blob +// If the storage supports direct serving and it's enabled, only the direct serving url is returned. +func GetPackageBlobStream(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { + key := packages_module.BlobHash256Key(pb.HashSHA256) + + cs := packages_module.NewContentStore() + + var s io.ReadSeekCloser + var u *url.URL + var err error + + if cs.ShouldServeDirect() { + u, err = cs.GetServeDirectURL(key, pf.Name) + if err != nil && !errors.Is(err, storage.ErrURLNotSupported) { + log.Error("Error getting serve direct url: %v", err) + } + } + if u == nil { + s, err = cs.Get(key) } - s, err := packages_module.NewContentStore().Get(packages_module.BlobHash256Key(pb.HashSHA256)) if err == nil { if pf.IsLead { if err := packages_model.IncrementDownloadCounter(ctx, pf.VersionID); err != nil { @@ -633,7 +627,7 @@ func GetPackageFileStream(ctx context.Context, pf *packages_model.PackageFile) ( } } } - return s, pf, err + return s, u, pf, err } // RemoveAllPackages for User diff --git a/tests/integration/api_packages_generic_test.go b/tests/integration/api_packages_generic_test.go index 765d11fd8..f5d8def0f 100644 --- a/tests/integration/api_packages_generic_test.go +++ b/tests/integration/api_packages_generic_test.go @@ -6,6 +6,7 @@ package integration import ( "bytes" "fmt" + "io" "net/http" "testing" @@ -139,6 +140,42 @@ func TestPackageGeneric(t *testing.T) { req = NewRequest(t, "GET", url+"/dummy.bin") MakeRequest(t, req, http.StatusUnauthorized) }) + + t.Run("ServeDirect", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + if setting.Packages.Storage.Type != setting.MinioStorageType { + t.Skip("Test skipped for non-Minio-storage.") + return + } + + if !setting.Packages.Storage.MinioConfig.ServeDirect { + old := setting.Packages.Storage.MinioConfig.ServeDirect + defer func() { + setting.Packages.Storage.MinioConfig.ServeDirect = old + }() + + setting.Packages.Storage.MinioConfig.ServeDirect = true + } + + req := NewRequest(t, "GET", url+"/"+filename) + resp := MakeRequest(t, req, http.StatusSeeOther) + + checkDownloadCount(3) + + location := resp.Header().Get("Location") + assert.NotEmpty(t, location) + + resp2, err := (&http.Client{}).Get(location) + assert.NoError(t, err) + assert.Equal(t, http.StatusOK, resp2.StatusCode) + + body, err := io.ReadAll(resp2.Body) + assert.NoError(t, err) + assert.Equal(t, content, body) + + checkDownloadCount(3) + }) }) t.Run("Delete", func(t *testing.T) {