From e1e4815a1c046851fd72d0ff5765116657fa9e86 Mon Sep 17 00:00:00 2001 From: Risu <79110363+risu729@users.noreply.github.com> Date: Wed, 16 Jul 2025 21:22:45 +1000 Subject: [PATCH] Redirect to a presigned URL of HEAD for HEAD requests (#35088) Resolves https://github.com/go-gitea/gitea/issues/35086. --------- Co-authored-by: wxiaoguang --- modules/actions/artifacts.go | 2 +- modules/packages/content_store.go | 4 +-- modules/storage/azureblob.go | 2 +- modules/storage/helper.go | 2 +- modules/storage/helper_test.go | 2 +- modules/storage/local.go | 2 +- modules/storage/minio.go | 9 +++-- modules/storage/storage.go | 2 +- routers/api/actions/artifacts.go | 2 +- routers/api/actions/artifactsv4.go | 2 +- routers/api/packages/alpine/alpine.go | 3 +- routers/api/packages/api.go | 2 +- routers/api/packages/arch/arch.go | 2 +- routers/api/packages/cargo/cargo.go | 1 + routers/api/packages/chef/chef.go | 2 +- routers/api/packages/composer/composer.go | 1 + routers/api/packages/conan/conan.go | 1 + routers/api/packages/conda/conda.go | 2 +- routers/api/packages/container/container.go | 2 +- routers/api/packages/cran/cran.go | 2 +- routers/api/packages/debian/debian.go | 4 ++- routers/api/packages/generic/generic.go | 1 + routers/api/packages/goproxy/goproxy.go | 2 +- routers/api/packages/helm/helm.go | 1 + routers/api/packages/maven/maven.go | 2 +- routers/api/packages/npm/npm.go | 2 ++ routers/api/packages/nuget/nuget.go | 3 +- routers/api/packages/pub/pub.go | 2 +- routers/api/packages/pypi/pypi.go | 1 + routers/api/packages/rpm/rpm.go | 2 ++ routers/api/packages/rubygems/rubygems.go | 1 + routers/api/packages/swift/swift.go | 2 +- routers/api/packages/vagrant/vagrant.go | 1 + routers/api/v1/repo/file.go | 4 +-- routers/web/base.go | 2 +- routers/web/repo/attachment.go | 2 +- routers/web/repo/download.go | 2 +- routers/web/repo/repo.go | 2 +- routers/web/user/package.go | 2 +- services/lfs/server.go | 4 ++- services/packages/packages.go | 32 +++++++++-------- .../integration/api_packages_generic_test.go | 36 +++++++------------ 42 files changed, 85 insertions(+), 72 deletions(-) diff --git a/modules/actions/artifacts.go b/modules/actions/artifacts.go index 4d074435ef..d28726e899 100644 --- a/modules/actions/artifacts.go +++ b/modules/actions/artifacts.go @@ -20,7 +20,7 @@ func IsArtifactV4(art *actions_model.ActionArtifact) bool { func DownloadArtifactV4ServeDirectOnly(ctx *context.Base, art *actions_model.ActionArtifact) (bool, error) { if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) + u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String(), http.StatusFound) return true, nil diff --git a/modules/packages/content_store.go b/modules/packages/content_store.go index dadb7eaefc..57974515e2 100644 --- a/modules/packages/content_store.go +++ b/modules/packages/content_store.go @@ -36,8 +36,8 @@ func (s *ContentStore) ShouldServeDirect() bool { return setting.Packages.Storage.ServeDirect() } -func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename string, reqParams url.Values) (*url.URL, error) { - return s.store.URL(KeyToRelativePath(key), filename, reqParams) +func (s *ContentStore) GetServeDirectURL(key BlobHash256Key, filename, method string, reqParams url.Values) (*url.URL, error) { + return s.store.URL(KeyToRelativePath(key), filename, method, reqParams) } // FIXME: Workaround to be removed in v1.20 diff --git a/modules/storage/azureblob.go b/modules/storage/azureblob.go index 837afd0ba6..6860d81131 100644 --- a/modules/storage/azureblob.go +++ b/modules/storage/azureblob.go @@ -247,7 +247,7 @@ func (a *AzureBlobStorage) Delete(path string) error { } // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes. -func (a *AzureBlobStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) { +func (a *AzureBlobStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL, error) { blobClient := a.getBlobClient(path) startTime := time.Now() diff --git a/modules/storage/helper.go b/modules/storage/helper.go index 9e6cceb537..f6c3d5eebb 100644 --- a/modules/storage/helper.go +++ b/modules/storage/helper.go @@ -30,7 +30,7 @@ func (s discardStorage) Delete(_ string) error { return fmt.Errorf("%s", s) } -func (s discardStorage) URL(_, _ string, _ url.Values) (*url.URL, error) { +func (s discardStorage) URL(_, _, _ string, _ url.Values) (*url.URL, error) { return nil, fmt.Errorf("%s", s) } diff --git a/modules/storage/helper_test.go b/modules/storage/helper_test.go index 62ebd8753c..3cba1e13c0 100644 --- a/modules/storage/helper_test.go +++ b/modules/storage/helper_test.go @@ -37,7 +37,7 @@ func Test_discardStorage(t *testing.T) { assert.Error(t, err, string(tt)) } { - got, err := tt.URL("path", "name", nil) + got, err := tt.URL("path", "name", "GET", nil) assert.Nil(t, got) assert.Errorf(t, err, string(tt)) } diff --git a/modules/storage/local.go b/modules/storage/local.go index 00c7f668aa..8a1776f606 100644 --- a/modules/storage/local.go +++ b/modules/storage/local.go @@ -114,7 +114,7 @@ func (l *LocalStorage) Delete(path string) error { } // URL gets the redirect URL to a file -func (l *LocalStorage) URL(path, name string, reqParams url.Values) (*url.URL, error) { +func (l *LocalStorage) URL(path, name, _ string, reqParams url.Values) (*url.URL, error) { return nil, ErrURLNotSupported } diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 1c5d25b2d4..01f2c16267 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -279,7 +279,7 @@ func (m *MinioStorage) Delete(path string) error { } // URL gets the redirect URL to a file. The presigned link is valid for 5 minutes. -func (m *MinioStorage) URL(path, name string, serveDirectReqParams url.Values) (*url.URL, error) { +func (m *MinioStorage) URL(path, name, method string, serveDirectReqParams url.Values) (*url.URL, error) { // copy serveDirectReqParams reqParams, err := url.ParseQuery(serveDirectReqParams.Encode()) if err != nil { @@ -287,7 +287,12 @@ func (m *MinioStorage) URL(path, name string, serveDirectReqParams url.Values) ( } // TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we? reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"") - u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams) + expires := 5 * time.Minute + if method == http.MethodHead { + u, err := m.client.PresignedHeadObject(m.ctx, m.bucket, m.buildMinioPath(path), expires, reqParams) + return u, convertMinioErr(err) + } + u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), expires, reqParams) return u, convertMinioErr(err) } diff --git a/modules/storage/storage.go b/modules/storage/storage.go index b0529941e7..2313951d8a 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -63,7 +63,7 @@ type ObjectStorage interface { Save(path string, r io.Reader, size int64) (int64, error) Stat(path string) (os.FileInfo, error) Delete(path string) error - URL(path, name string, reqParams url.Values) (*url.URL, error) + URL(path, name, method string, reqParams url.Values) (*url.URL, error) IterateObjects(path string, iterator func(path string, obj Object) error) error } diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 6473659e5c..d71a6f487c 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -428,7 +428,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) { for _, artifact := range artifacts { var downloadURL string if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName, nil) + u, err := ar.fs.URL(artifact.StoragePath, artifact.ArtifactName, ctx.Req.Method, nil) if err != nil && !errors.Is(err, storage.ErrURLNotSupported) { log.Error("Error getting serve direct url: %v", err) } diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index e9e9fc6393..6d27479628 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -517,7 +517,7 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { respData := GetSignedArtifactURLResponse{} if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, nil) + u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, ctx.Req.Method, nil) if u != nil && err == nil { respData.SignedUrl = u.String() } diff --git a/routers/api/packages/alpine/alpine.go b/routers/api/packages/alpine/alpine.go index ba4a4f23ce..9265adcdba 100644 --- a/routers/api/packages/alpine/alpine.go +++ b/routers/api/packages/alpine/alpine.go @@ -75,6 +75,7 @@ func GetRepositoryFile(ctx *context.Context) { Filename: alpine_service.IndexArchiveFilename, CompositeKey: fmt.Sprintf("%s|%s|%s", ctx.PathParam("branch"), ctx.PathParam("repository"), ctx.PathParam("architecture")), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, util.ErrNotExist) { @@ -216,7 +217,7 @@ func DownloadPackageFile(ctx *context.Context) { } } - s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0]) + s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0], ctx.Req.Method) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 878e0f9945..d914856706 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -339,7 +339,7 @@ func CommonRoutes() *web.Router { r.Group("/{packagename}/{packageversion}", func() { r.Delete("", reqPackageAccess(perm.AccessModeWrite), generic.DeletePackage) r.Group("/{filename}", func() { - r.Get("", generic.DownloadPackageFile) + r.Methods("HEAD,GET", "", generic.DownloadPackageFile) r.Group("", func() { r.Put("", generic.UploadPackage) r.Delete("", generic.DeletePackageFile) diff --git a/routers/api/packages/arch/arch.go b/routers/api/packages/arch/arch.go index bf9cc3f1b8..e5e25f443e 100644 --- a/routers/api/packages/arch/arch.go +++ b/routers/api/packages/arch/arch.go @@ -239,7 +239,7 @@ func GetPackageOrRepositoryFile(ctx *context.Context) { return } - s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0]) + s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0], ctx.Req.Method) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) diff --git a/routers/api/packages/cargo/cargo.go b/routers/api/packages/cargo/cargo.go index cfcf79244f..fc92089bb4 100644 --- a/routers/api/packages/cargo/cargo.go +++ b/routers/api/packages/cargo/cargo.go @@ -176,6 +176,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: strings.ToLower(fmt.Sprintf("%s-%s.crate", ctx.PathParam("package"), ctx.PathParam("version"))), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/chef/chef.go b/routers/api/packages/chef/chef.go index 1f11afe548..be86e5d792 100644 --- a/routers/api/packages/chef/chef.go +++ b/routers/api/packages/chef/chef.go @@ -343,7 +343,7 @@ func DownloadPackage(ctx *context.Context) { pf := pd.Files[0].File - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/composer/composer.go b/routers/api/packages/composer/composer.go index 9daf0ffeff..d15fb689b3 100644 --- a/routers/api/packages/composer/composer.go +++ b/routers/api/packages/composer/composer.go @@ -171,6 +171,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: ctx.PathParam("filename"), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/conan/conan.go b/routers/api/packages/conan/conan.go index fe70e02cd6..fb3aded9c4 100644 --- a/routers/api/packages/conan/conan.go +++ b/routers/api/packages/conan/conan.go @@ -492,6 +492,7 @@ func downloadFile(ctx *context.Context, fileFilter container.Set[string], fileKe Filename: filename, CompositeKey: fileKey, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/conda/conda.go b/routers/api/packages/conda/conda.go index e8c97503c8..3a523a01ab 100644 --- a/routers/api/packages/conda/conda.go +++ b/routers/api/packages/conda/conda.go @@ -317,7 +317,7 @@ func DownloadPackageFile(ctx *context.Context) { pf := pfs[0] - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index d532f698ad..b30522d68a 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -710,7 +710,7 @@ func DeleteManifest(ctx *context.Context) { func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor) { serveDirectReqParams := make(url.Values) serveDirectReqParams.Set("response-content-type", pfd.Properties.GetByName(container_module.PropertyMediaType)) - s, u, _, err := packages_service.OpenBlobForDownload(ctx, pfd.File, pfd.Blob, serveDirectReqParams) + s, u, _, err := packages_service.OpenBlobForDownload(ctx, pfd.File, pfd.Blob, ctx.Req.Method, serveDirectReqParams) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/cran/cran.go b/routers/api/packages/cran/cran.go index 732acd215f..46e1e0fcbb 100644 --- a/routers/api/packages/cran/cran.go +++ b/routers/api/packages/cran/cran.go @@ -250,7 +250,7 @@ func downloadPackageFile(ctx *context.Context, opts *cran_model.SearchOptions) { return } - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go index 346f71fa5d..9bbcae3503 100644 --- a/routers/api/packages/debian/debian.go +++ b/routers/api/packages/debian/debian.go @@ -66,6 +66,7 @@ func GetRepositoryFile(ctx *context.Context) { Filename: ctx.PathParam("filename"), CompositeKey: key, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { @@ -106,7 +107,7 @@ func GetRepositoryFileByHash(ctx *context.Context) { return } - s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0]) + s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0], ctx.Req.Method) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) @@ -222,6 +223,7 @@ func DownloadPackageFile(ctx *context.Context) { Filename: fmt.Sprintf("%s_%s_%s.deb", name, version, ctx.PathParam("architecture")), CompositeKey: fmt.Sprintf("%s|%s", ctx.PathParam("distribution"), ctx.PathParam("component")), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, util.ErrNotExist) { diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go index db7aeace50..389f8ac1ca 100644 --- a/routers/api/packages/generic/generic.go +++ b/routers/api/packages/generic/generic.go @@ -42,6 +42,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: ctx.PathParam("filename"), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/goproxy/goproxy.go b/routers/api/packages/goproxy/goproxy.go index 89ec86bce9..c2dd904254 100644 --- a/routers/api/packages/goproxy/goproxy.go +++ b/routers/api/packages/goproxy/goproxy.go @@ -106,7 +106,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, u, _, err := packages_service.OpenFileForDownload(ctx, pfs[0]) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pfs[0], ctx.Req.Method) if err != nil { if errors.Is(err, util.ErrNotExist) { apiError(ctx, http.StatusNotFound, err) diff --git a/routers/api/packages/helm/helm.go b/routers/api/packages/helm/helm.go index 39c34f4da4..b7f5d032fb 100644 --- a/routers/api/packages/helm/helm.go +++ b/routers/api/packages/helm/helm.go @@ -128,6 +128,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go index 40a8ff8242..b6cfa893a4 100644 --- a/routers/api/packages/maven/maven.go +++ b/routers/api/packages/maven/maven.go @@ -223,7 +223,7 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool return } - s, u, _, err := packages_service.OpenBlobForDownload(ctx, pf, pb, nil) + s, u, _, err := packages_service.OpenBlobForDownload(ctx, pf, pb, ctx.Req.Method, nil) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go index 1f09816d32..4f1099eb8b 100644 --- a/routers/api/packages/npm/npm.go +++ b/routers/api/packages/npm/npm.go @@ -96,6 +96,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { @@ -138,6 +139,7 @@ func DownloadPackageFileByName(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go index 92d62d90b1..023c1e36b3 100644 --- a/routers/api/packages/nuget/nuget.go +++ b/routers/api/packages/nuget/nuget.go @@ -416,6 +416,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { @@ -669,7 +670,7 @@ func DownloadSymbolFile(ctx *context.Context) { return } - s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0]) + s, u, pf, err := packages_service.OpenFileForDownload(ctx, pfs[0], ctx.Req.Method) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { apiError(ctx, http.StatusNotFound, err) diff --git a/routers/api/packages/pub/pub.go b/routers/api/packages/pub/pub.go index 4bd36e94b6..b246d1b34f 100644 --- a/routers/api/packages/pub/pub.go +++ b/routers/api/packages/pub/pub.go @@ -274,7 +274,7 @@ func DownloadPackageFile(ctx *context.Context) { pf := pd.Files[0].File - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go index 9b5ae6c89d..3a92387857 100644 --- a/routers/api/packages/pypi/pypi.go +++ b/routers/api/packages/pypi/pypi.go @@ -93,6 +93,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/rpm/rpm.go b/routers/api/packages/rpm/rpm.go index 938c35341d..e8e0f12fe3 100644 --- a/routers/api/packages/rpm/rpm.go +++ b/routers/api/packages/rpm/rpm.go @@ -103,6 +103,7 @@ func GetRepositoryFile(ctx *context.Context) { Filename: ctx.PathParam("filename"), CompositeKey: ctx.PathParam("group"), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, util.ErrNotExist) { @@ -232,6 +233,7 @@ func DownloadPackageFile(ctx *context.Context) { Filename: fmt.Sprintf("%s-%s.%s.rpm", name, version, ctx.PathParam("architecture")), CompositeKey: ctx.PathParam("group"), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, util.ErrNotExist) { diff --git a/routers/api/packages/rubygems/rubygems.go b/routers/api/packages/rubygems/rubygems.go index 774d5520fd..7dc4b0c24d 100644 --- a/routers/api/packages/rubygems/rubygems.go +++ b/routers/api/packages/rubygems/rubygems.go @@ -184,6 +184,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: filename, }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go index bf542f33a7..124aa98c3b 100644 --- a/routers/api/packages/swift/swift.go +++ b/routers/api/packages/swift/swift.go @@ -429,7 +429,7 @@ func DownloadPackageFile(ctx *context.Context) { pf := pd.Files[0].File - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { apiError(ctx, http.StatusInternalServerError, err) return diff --git a/routers/api/packages/vagrant/vagrant.go b/routers/api/packages/vagrant/vagrant.go index 9eb67e5397..841fa8f964 100644 --- a/routers/api/packages/vagrant/vagrant.go +++ b/routers/api/packages/vagrant/vagrant.go @@ -229,6 +229,7 @@ func DownloadPackageFile(ctx *context.Context) { &packages_service.PackageFileInfo{ Filename: ctx.PathParam("provider"), }, + ctx.Req.Method, ) if err != nil { if errors.Is(err, packages_model.ErrPackageNotExist) || errors.Is(err, packages_model.ErrPackageFileNotExist) { diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 69b5996222..a85dda79d0 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -210,7 +210,7 @@ func GetRawFileOrLFS(ctx *context.APIContext) { if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil) + u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return @@ -331,7 +331,7 @@ func download(ctx *context.APIContext, archiveName string, archiver *repo_model. rPath := archiver.RelativePath() if setting.RepoArchive.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.RepoArchives.URL(rPath, downloadName, nil) + u, err := storage.RepoArchives.URL(rPath, downloadName, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return diff --git a/routers/web/base.go b/routers/web/base.go index e43f36a97b..0f06cb5e4b 100644 --- a/routers/web/base.go +++ b/routers/web/base.go @@ -39,7 +39,7 @@ func avatarStorageHandler(storageSetting *setting.Storage, prefix string, objSto rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/") rPath = util.PathJoinRelX(rPath) - u, err := objStore.URL(rPath, path.Base(rPath), nil) + u, err := objStore.URL(rPath, path.Base(rPath), req.Method, nil) if err != nil { if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { log.Warn("Unable to find %s %s", prefix, rPath) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index 9eda926dad..f696669196 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -129,7 +129,7 @@ func ServeAttachment(ctx *context.Context, uuid string) { if setting.Attachment.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name, nil) + u, err := storage.Attachments.URL(attach.RelativePath(), attach.Name, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) diff --git a/routers/web/repo/download.go b/routers/web/repo/download.go index 020cebf196..6f394aae27 100644 --- a/routers/web/repo/download.go +++ b/routers/web/repo/download.go @@ -54,7 +54,7 @@ func ServeBlobOrLFS(ctx *context.Context, blob *git.Blob, lastModified *time.Tim if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage, blob storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), nil) + u, err := storage.LFS.URL(pointer.RelativePath(), blob.Name(), ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return nil diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 828ec08a8a..1b700aa6da 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -398,7 +398,7 @@ func download(ctx *context.Context, archiveName string, archiver *repo_model.Rep rPath := archiver.RelativePath() if setting.RepoArchive.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.RepoArchives.URL(rPath, downloadName, nil) + u, err := storage.RepoArchives.URL(rPath, downloadName, ctx.Req.Method, nil) if u != nil && err == nil { ctx.Redirect(u.String()) return diff --git a/routers/web/user/package.go b/routers/web/user/package.go index 216acdf927..d130d1dca1 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -513,7 +513,7 @@ func DownloadPackageFile(ctx *context.Context) { return } - s, u, _, err := packages_service.OpenFileForDownload(ctx, pf) + s, u, _, err := packages_service.OpenFileForDownload(ctx, pf, ctx.Req.Method) if err != nil { ctx.ServerError("OpenFileForDownload", err) return diff --git a/services/lfs/server.go b/services/lfs/server.go index c44cc35e53..c9d9f164bf 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -43,6 +43,7 @@ type requestContext struct { User string Repo string Authorization string + Method string } // Claims is a JWT Token Claims @@ -397,6 +398,7 @@ func getRequestContext(ctx *context.Context) *requestContext { User: ctx.PathParam("username"), Repo: strings.TrimSuffix(ctx.PathParam("reponame"), ".git"), Authorization: ctx.Req.Header.Get("Authorization"), + Method: ctx.Req.Method, } } @@ -465,7 +467,7 @@ func buildObjectResponse(rc *requestContext, pointer lfs_module.Pointer, downloa var link *lfs_module.Link if setting.LFS.Storage.ServeDirect() { // If we have a signed url (S3, object storage), redirect to this directly. - u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid, nil) + u, err := storage.LFS.URL(pointer.RelativePath(), pointer.Oid, rc.Method, nil) if u != nil && err == nil { // Presigned url does not need the Authorization header // https://github.com/go-gitea/gitea/issues/21525 diff --git a/services/packages/packages.go b/services/packages/packages.go index 517334cbc7..4b16ee7285 100644 --- a/services/packages/packages.go +++ b/services/packages/packages.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "net/http" "net/url" "strings" @@ -564,7 +565,7 @@ func DeletePackageFile(ctx context.Context, pf *packages_model.PackageFile) erro } // OpenFileForDownloadByPackageNameAndVersion returns the content of the specific package file and increases the download counter. -func OpenFileForDownloadByPackageNameAndVersion(ctx context.Context, pvi *PackageInfo, pfi *PackageFileInfo) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { +func OpenFileForDownloadByPackageNameAndVersion(ctx context.Context, pvi *PackageInfo, pfi *PackageFileInfo, method string) (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) @@ -576,27 +577,27 @@ func OpenFileForDownloadByPackageNameAndVersion(ctx context.Context, pvi *Packag return nil, nil, nil, err } - return OpenFileForDownloadByPackageVersion(ctx, pv, pfi) + return OpenFileForDownloadByPackageVersion(ctx, pv, pfi, method) } // OpenFileForDownloadByPackageVersion returns the content of the specific package file and increases the download counter. -func OpenFileForDownloadByPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pfi *PackageFileInfo) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { +func OpenFileForDownloadByPackageVersion(ctx context.Context, pv *packages_model.PackageVersion, pfi *PackageFileInfo, method string) (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, nil, err } - return OpenFileForDownload(ctx, pf) + return OpenFileForDownload(ctx, pf, method) } // OpenFileForDownload returns the content of the specific package file and increases the download counter. -func OpenFileForDownload(ctx context.Context, pf *packages_model.PackageFile) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { +func OpenFileForDownload(ctx context.Context, pf *packages_model.PackageFile, method string) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { pb, err := packages_model.GetBlobByID(ctx, pf.BlobID) if err != nil { return nil, nil, nil, err } - return OpenBlobForDownload(ctx, pf, pb, nil) + return OpenBlobForDownload(ctx, pf, pb, method, nil) } func OpenBlobStream(pb *packages_model.PackageBlob) (io.ReadSeekCloser, error) { @@ -607,7 +608,7 @@ func OpenBlobStream(pb *packages_model.PackageBlob) (io.ReadSeekCloser, error) { // OpenBlobForDownload returns the content of the specific package blob and increases the download counter. // If the storage supports direct serving and it's enabled, only the direct serving url is returned. -func OpenBlobForDownload(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, serveDirectReqParams url.Values) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { +func OpenBlobForDownload(ctx context.Context, pf *packages_model.PackageFile, pb *packages_model.PackageBlob, method string, serveDirectReqParams url.Values) (io.ReadSeekCloser, *url.URL, *packages_model.PackageFile, error) { key := packages_module.BlobHash256Key(pb.HashSHA256) cs := packages_module.NewContentStore() @@ -617,23 +618,24 @@ func OpenBlobForDownload(ctx context.Context, pf *packages_model.PackageFile, pb var err error if cs.ShouldServeDirect() { - u, err = cs.GetServeDirectURL(key, pf.Name, serveDirectReqParams) + u, err = cs.GetServeDirectURL(key, pf.Name, method, serveDirectReqParams) if err != nil && !errors.Is(err, storage.ErrURLNotSupported) { - log.Error("Error getting serve direct url: %v", err) + log.Error("Error getting serve direct url (fallback to local reader): %v", err) } } if u == nil { s, err = cs.OpenBlob(key) } + if err != nil { + return nil, nil, nil, err + } - if err == nil { - if pf.IsLead { - if err := packages_model.IncrementDownloadCounter(ctx, pf.VersionID); err != nil { - log.Error("Error incrementing download counter: %v", err) - } + if pf.IsLead && method == http.MethodGet { + if err := packages_model.IncrementDownloadCounter(ctx, pf.VersionID); err != nil { + log.Error("Error incrementing download counter: %v", err) } } - return s, u, pf, err + return s, u, pf, nil } // RemoveAllPackages for User diff --git a/tests/integration/api_packages_generic_test.go b/tests/integration/api_packages_generic_test.go index 5f410fc470..94e2c6072c 100644 --- a/tests/integration/api_packages_generic_test.go +++ b/tests/integration/api_packages_generic_test.go @@ -141,37 +141,25 @@ func TestPackageGeneric(t *testing.T) { t.Run("ServeDirect", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - if setting.Packages.Storage.Type != setting.MinioStorageType && setting.Packages.Storage.Type != setting.AzureBlobStorageType { - t.Skip("Test skipped for non-Minio-storage and non-AzureBlob-storage.") - return - } - if setting.Packages.Storage.Type == setting.MinioStorageType { - 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 - } + defer test.MockVariableValue(&setting.Packages.Storage.MinioConfig.ServeDirect, true)() } else if setting.Packages.Storage.Type == setting.AzureBlobStorageType { - if !setting.Packages.Storage.AzureBlobConfig.ServeDirect { - old := setting.Packages.Storage.AzureBlobConfig.ServeDirect - defer func() { - setting.Packages.Storage.AzureBlobConfig.ServeDirect = old - }() - - setting.Packages.Storage.AzureBlobConfig.ServeDirect = true - } + defer test.MockVariableValue(&setting.Packages.Storage.AzureBlobConfig.ServeDirect, true)() + } else { + t.Skip("Test skipped for non-Minio-storage and non-AzureBlob-storage.") } - req := NewRequest(t, "GET", url+"/"+filename) - resp := MakeRequest(t, req, http.StatusSeeOther) + req = NewRequest(t, "HEAD", url+"/"+filename) + resp = MakeRequest(t, req, http.StatusSeeOther) + location := resp.Header().Get("Location") + assert.NotEmpty(t, location) + checkDownloadCount(2) + req = NewRequest(t, "GET", url+"/"+filename) + resp = MakeRequest(t, req, http.StatusSeeOther) checkDownloadCount(3) - location := resp.Header().Get("Location") + location = resp.Header().Get("Location") assert.NotEmpty(t, location) resp2, err := (&http.Client{}).Get(location)