From 42919ccb7cd32ab67d0878baf2bac6cd007899a8 Mon Sep 17 00:00:00 2001 From: JakobDev Date: Wed, 12 Apr 2023 11:05:23 +0200 Subject: [PATCH] Make Release Download URLs predictable (#23891) As promised in #23817, I have this made a PR to make Release Download URLs predictable. It currently follows the schema `/releases/download//`. this already works, but it is nowhere shown in the UI or the API. The Problem is, that it is currently possible to have multiple files with the same name (why do we even allow this) for a release. I had written some Code to check, if a Release has 2 or more files with the same Name. If yes, it uses the old `attachments/` URlL if no it uses the new fancy URL. I had also changed `/releases/download//` to directly serve the File instead of redirecting, so people who who use automatic update checker don't end up with the `attachments/` URL. Fixes #10919 --------- Co-authored-by: a1012112796 <1012112796@qq.com> --- models/repo/attachment.go | 27 ++++++++++++++++----------- models/repo/release.go | 29 +++++++++++++++++++++++++++++ routers/web/repo/attachment.go | 11 ++++++++--- routers/web/repo/release.go | 6 ++++++ routers/web/repo/repo.go | 2 +- 5 files changed, 60 insertions(+), 15 deletions(-) diff --git a/models/repo/attachment.go b/models/repo/attachment.go index cb05386d9..93b83aae8 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -18,17 +18,18 @@ import ( // Attachment represent a attachment of issue/comment/release. type Attachment struct { - ID int64 `xorm:"pk autoincr"` - UUID string `xorm:"uuid UNIQUE"` - RepoID int64 `xorm:"INDEX"` // this should not be zero - IssueID int64 `xorm:"INDEX"` // maybe zero when creating - ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating - UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added - CommentID int64 - Name string - DownloadCount int64 `xorm:"DEFAULT 0"` - Size int64 `xorm:"DEFAULT 0"` - CreatedUnix timeutil.TimeStamp `xorm:"created"` + ID int64 `xorm:"pk autoincr"` + UUID string `xorm:"uuid UNIQUE"` + RepoID int64 `xorm:"INDEX"` // this should not be zero + IssueID int64 `xorm:"INDEX"` // maybe zero when creating + ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating + UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added + CommentID int64 + Name string + DownloadCount int64 `xorm:"DEFAULT 0"` + Size int64 `xorm:"DEFAULT 0"` + CreatedUnix timeutil.TimeStamp `xorm:"created"` + CustomDownloadURL string `xorm:"-"` } func init() { @@ -57,6 +58,10 @@ func (a *Attachment) RelativePath() string { // DownloadURL returns the download url of the attached file func (a *Attachment) DownloadURL() string { + if a.CustomDownloadURL != "" { + return a.CustomDownloadURL + } + return setting.AppURL + "attachments/" + url.PathEscape(a.UUID) } diff --git a/models/repo/release.go b/models/repo/release.go index c8dd7fbc7..75eb27f07 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -7,6 +7,7 @@ package repo import ( "context" "fmt" + "net/url" "sort" "strconv" "strings" @@ -372,6 +373,34 @@ func GetReleaseAttachments(ctx context.Context, rels ...*Release) (err error) { sortedRels.Rel[currentIndex].Attachments = append(sortedRels.Rel[currentIndex].Attachments, attachment) } + // Makes URL's predictable + for _, release := range rels { + // If we have no Repo, we don't need to execute this loop + if release.Repo == nil { + continue + } + + // Check if there are two or more attachments with the same name + hasDuplicates := false + foundNames := make(map[string]bool) + for _, attachment := range release.Attachments { + _, found := foundNames[attachment.Name] + if found { + hasDuplicates = true + break + } else { + foundNames[attachment.Name] = true + } + } + + // If the names unique, use the URL with the Name instead of the UUID + if !hasDuplicates { + for _, attachment := range release.Attachments { + attachment.CustomDownloadURL = release.Repo.HTMLURL() + "/releases/download/" + url.PathEscape(release.TagName) + "/" + url.PathEscape(attachment.Name) + } + } + } + return err } diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index c6d8828fa..9fb9cb00b 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -86,9 +86,9 @@ func DeleteAttachment(ctx *context.Context) { }) } -// GetAttachment serve attachments -func GetAttachment(ctx *context.Context) { - attach, err := repo_model.GetAttachmentByUUID(ctx, ctx.Params(":uuid")) +// GetAttachment serve attachments with the given UUID +func ServeAttachment(ctx *context.Context, uuid string) { + attach, err := repo_model.GetAttachmentByUUID(ctx, uuid) if err != nil { if repo_model.IsErrAttachmentNotExist(err) { ctx.Error(http.StatusNotFound) @@ -153,3 +153,8 @@ func GetAttachment(ctx *context.Context) { return } } + +// GetAttachment serve attachments +func GetAttachment(ctx *context.Context) { + ServeAttachment(ctx, ctx.Params(":uuid")) +} diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 14ef1372c..e8caa2cbb 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -142,6 +142,10 @@ func releasesOrTags(ctx *context.Context, isTagList bool) { return } + for _, release := range releases { + release.Repo = ctx.Repo.Repository + } + if err = repo_model.GetReleaseAttachments(ctx, releases...); err != nil { ctx.ServerError("GetReleaseAttachments", err) return @@ -248,6 +252,8 @@ func SingleRelease(ctx *context.Context) { ctx.Data["Title"] = release.Title } + release.Repo = ctx.Repo.Repository + err = repo_model.GetReleaseAttachments(ctx, release) if err != nil { ctx.ServerError("GetReleaseAttachments", err) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 9b80e8532..5a97c5190 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -373,7 +373,7 @@ func RedirectDownload(ctx *context.Context) { return } if att != nil { - ctx.Redirect(att.DownloadURL()) + ServeAttachment(ctx, att.UUID) return } }