From e9288c24773157411edec17c9bbcc8c1567e91ee Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 11 Feb 2023 14:34:11 +0800 Subject: [PATCH] Fix improper HTMLURL usages in Go code (#22839) In Go code, HTMLURL should be only used for external systems, like API/webhook/mail/notification, etc. If a URL is used by `Redirect` or rendered in a template, it should be a relative URL (aka `Link()` in Gitea) Co-authored-by: Lunny Xiao --- models/repo/repo.go | 2 +- modules/context/repo.go | 6 +++--- routers/web/repo/actions/actions.go | 2 +- routers/web/repo/issue.go | 14 +++++++------- routers/web/repo/issue_dependency.go | 4 ++-- routers/web/repo/issue_lock.go | 10 +++++----- routers/web/repo/issue_stopwatch.go | 4 ++-- routers/web/repo/issue_timetrack.go | 4 ++-- routers/web/repo/issue_watch.go | 2 +- routers/web/repo/pull_review.go | 2 +- routers/web/repo/release.go | 2 +- routers/web/repo/repo.go | 2 +- routers/web/repo/search.go | 2 +- routers/web/repo/view.go | 4 ++-- routers/web/user/package.go | 2 +- routers/web/user/profile.go | 2 +- services/actions/commit_status.go | 2 +- 17 files changed, 33 insertions(+), 33 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 5d3753620..e7ec93a22 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -274,7 +274,7 @@ func (repo *Repository) CommitLink(commitID string) (result string) { if commitID == "" || commitID == "0000000000000000000000000000000000000000" { result = "" } else { - result = repo.HTMLURL() + "/commit/" + url.PathEscape(commitID) + result = repo.Link() + "/commit/" + url.PathEscape(commitID) } return result } diff --git a/modules/context/repo.go b/modules/context/repo.go index 38c1d8454..a38376ff4 100644 --- a/modules/context/repo.go +++ b/modules/context/repo.go @@ -743,9 +743,9 @@ func RepoAssignment(ctx *Context) (cancel context.CancelFunc) { if ctx.FormString("go-get") == "1" { ctx.Data["GoGetImport"] = ComposeGoGetImport(owner.Name, repo.Name) - prefix := repo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(ctx.Repo.BranchName) - ctx.Data["GoDocDirectory"] = prefix + "{/dir}" - ctx.Data["GoDocFile"] = prefix + "{/dir}/{file}#L{line}" + fullURLPrefix := repo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(ctx.Repo.BranchName) + ctx.Data["GoDocDirectory"] = fullURLPrefix + "{/dir}" + ctx.Data["GoDocFile"] = fullURLPrefix + "{/dir}/{file}#L{line}" } return cancel } diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index 146bf27da..e5496676a 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -70,7 +70,7 @@ func List(ctx *context.Context) { } ctx.Data["workflows"] = workflows - ctx.Data["RepoLink"] = ctx.Repo.Repository.HTMLURL() + ctx.Data["RepoLink"] = ctx.Repo.Repository.Link() page := ctx.FormInt("page") if page <= 0 { diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 2193da511..3cd29c2b6 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -100,7 +100,7 @@ func MustAllowUserComment(ctx *context.Context) { if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin { ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked")) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } } @@ -927,7 +927,7 @@ func NewIssueChooseTemplate(ctx *context.Context) { if len(issueTemplates) == 0 { // The "issues/new" and "issues/new/choose" share the same query parameters "project" and "milestone", if no template here, just redirect to the "issues/new" page with these parameters. - ctx.Redirect(fmt.Sprintf("%s/issues/new?%s", ctx.Repo.Repository.HTMLURL(), ctx.Req.URL.RawQuery), http.StatusSeeOther) + ctx.Redirect(fmt.Sprintf("%s/issues/new?%s", ctx.Repo.Repository.Link(), ctx.Req.URL.RawQuery), http.StatusSeeOther) return } @@ -950,11 +950,11 @@ func DeleteIssue(ctx *context.Context) { } if issue.IsPull { - ctx.Redirect(fmt.Sprintf("%s/pulls", ctx.Repo.Repository.HTMLURL()), http.StatusSeeOther) + ctx.Redirect(fmt.Sprintf("%s/pulls", ctx.Repo.Repository.Link()), http.StatusSeeOther) return } - ctx.Redirect(fmt.Sprintf("%s/issues", ctx.Repo.Repository.HTMLURL()), http.StatusSeeOther) + ctx.Redirect(fmt.Sprintf("%s/issues", ctx.Repo.Repository.Link()), http.StatusSeeOther) } // ValidateRepoMetas check and returns repository's meta information @@ -1425,7 +1425,7 @@ func ViewIssue(ctx *context.Context) { return } // Add link to the issue of the already running stopwatch - ctx.Data["OtherStopwatchURL"] = otherIssue.HTMLURL() + ctx.Data["OtherStopwatchURL"] = otherIssue.Link() } } ctx.Data["CanUseTimetracker"] = ctx.Repo.CanUseTimetracker(issue, ctx.Doer) @@ -2658,7 +2658,7 @@ func NewComment(ctx *context.Context) { if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin { ctx.Flash.Error(ctx.Tr("repo.issues.comment_on_locked")) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } @@ -2669,7 +2669,7 @@ func NewComment(ctx *context.Context) { if ctx.HasError() { ctx.Flash.Error(ctx.Data["ErrorMsg"].(string)) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } diff --git a/routers/web/repo/issue_dependency.go b/routers/web/repo/issue_dependency.go index 41c127be9..365d9609d 100644 --- a/routers/web/repo/issue_dependency.go +++ b/routers/web/repo/issue_dependency.go @@ -34,7 +34,7 @@ func AddDependency(ctx *context.Context) { } // Redirect - defer ctx.Redirect(issue.HTMLURL()) + defer ctx.Redirect(issue.Link()) // Dependency dep, err := issues_model.GetIssueByID(ctx, depID) @@ -124,5 +124,5 @@ func RemoveDependency(ctx *context.Context) { } // Redirect - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) } diff --git a/routers/web/repo/issue_lock.go b/routers/web/repo/issue_lock.go index 10db968a2..08b76e555 100644 --- a/routers/web/repo/issue_lock.go +++ b/routers/web/repo/issue_lock.go @@ -21,13 +21,13 @@ func LockIssue(ctx *context.Context) { if issue.IsLocked { ctx.Flash.Error(ctx.Tr("repo.issues.lock_duplicate")) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } if !form.HasValidReason() { ctx.Flash.Error(ctx.Tr("repo.issues.lock.unknown_reason")) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } @@ -40,7 +40,7 @@ func LockIssue(ctx *context.Context) { return } - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) } // UnlockIssue unlocks a previously locked issue. @@ -52,7 +52,7 @@ func UnlockIssue(ctx *context.Context) { if !issue.IsLocked { ctx.Flash.Error(ctx.Tr("repo.issues.unlock_error")) - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) return } @@ -64,5 +64,5 @@ func UnlockIssue(ctx *context.Context) { return } - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) } diff --git a/routers/web/repo/issue_stopwatch.go b/routers/web/repo/issue_stopwatch.go index d2a7a12a1..3d20b08b4 100644 --- a/routers/web/repo/issue_stopwatch.go +++ b/routers/web/repo/issue_stopwatch.go @@ -40,7 +40,7 @@ func IssueStopwatch(c *context.Context) { c.Flash.Success(c.Tr("repo.issues.tracker_auto_close")) } - url := issue.HTMLURL() + url := issue.Link() c.Redirect(url, http.StatusSeeOther) } @@ -72,7 +72,7 @@ func CancelStopwatch(c *context.Context) { }) } - url := issue.HTMLURL() + url := issue.Link() c.Redirect(url, http.StatusSeeOther) } diff --git a/routers/web/repo/issue_timetrack.go b/routers/web/repo/issue_timetrack.go index 6e9d3673c..7dc7d0797 100644 --- a/routers/web/repo/issue_timetrack.go +++ b/routers/web/repo/issue_timetrack.go @@ -26,7 +26,7 @@ func AddTimeManually(c *context.Context) { c.NotFound("CanUseTimetracker", nil) return } - url := issue.HTMLURL() + url := issue.Link() if c.HasError() { c.Flash.Error(c.GetErrMsg()) @@ -83,5 +83,5 @@ func DeleteTime(c *context.Context) { } c.Flash.Success(c.Tr("repo.issues.del_time_history", util.SecToTime(t.Time))) - c.Redirect(issue.HTMLURL()) + c.Redirect(issue.Link()) } diff --git a/routers/web/repo/issue_watch.go b/routers/web/repo/issue_watch.go index c23dbf062..1837c2b63 100644 --- a/routers/web/repo/issue_watch.go +++ b/routers/web/repo/issue_watch.go @@ -52,5 +52,5 @@ func IssueWatch(ctx *context.Context) { return } - ctx.Redirect(issue.HTMLURL()) + ctx.Redirect(issue.Link()) } diff --git a/routers/web/repo/pull_review.go b/routers/web/repo/pull_review.go index 9f4cdde86..d43a786c5 100644 --- a/routers/web/repo/pull_review.go +++ b/routers/web/repo/pull_review.go @@ -98,7 +98,7 @@ func CreateCodeComment(ctx *context.Context) { renderConversation(ctx, comment) return } - ctx.Redirect(comment.HTMLURL()) + ctx.Redirect(comment.Link()) } // UpdateResolveConversation add or remove an Conversation resolved mark diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 5204b5fd0..e969fdc5a 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -295,7 +295,7 @@ func LatestRelease(ctx *context.Context) { return } - ctx.Redirect(release.HTMLURL()) + ctx.Redirect(release.Link()) } // NewRelease render creating or edit release page diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index 0a51dfa73..9f2add1fe 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -344,7 +344,7 @@ func acceptOrRejectRepoTransfer(ctx *context.Context, accept bool) error { ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected")) } - ctx.Redirect(ctx.Repo.Repository.HTMLURL()) + ctx.Redirect(ctx.Repo.Repository.Link()) return nil } diff --git a/routers/web/repo/search.go b/routers/web/repo/search.go index 137f38d40..a04319847 100644 --- a/routers/web/repo/search.go +++ b/routers/web/repo/search.go @@ -54,7 +54,7 @@ func Search(ctx *context.Context) { ctx.Data["CodeIndexerUnavailable"] = !code_indexer.IsAvailable() } - ctx.Data["SourcePath"] = ctx.Repo.Repository.HTMLURL() + ctx.Data["SourcePath"] = ctx.Repo.Repository.Link() ctx.Data["SearchResults"] = searchResults ctx.Data["SearchResultLanguages"] = searchResultLanguages diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index f31490237..70f2a941b 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -318,7 +318,7 @@ func renderReadmeFile(ctx *context.Context, readmeFile *namedBlob, readmeTreelin if fInfo.isLFSFile { filenameBase64 := base64.RawURLEncoding.EncodeToString([]byte(readmeFile.name)) - ctx.Data["RawFileLink"] = fmt.Sprintf("%s.git/info/lfs/objects/%s/%s", ctx.Repo.Repository.HTMLURL(), url.PathEscape(fInfo.lfsMeta.Oid), url.PathEscape(filenameBase64)) + ctx.Data["RawFileLink"] = fmt.Sprintf("%s.git/info/lfs/objects/%s/%s", ctx.Repo.Repository.Link(), url.PathEscape(fInfo.lfsMeta.Oid), url.PathEscape(filenameBase64)) } if !fInfo.isTextFile { @@ -738,7 +738,7 @@ func Home(ctx *context.Context) { } ctx.Data["EnableFeed"] = true - ctx.Data["FeedURL"] = ctx.Repo.Repository.HTMLURL() + ctx.Data["FeedURL"] = ctx.Repo.Repository.Link() } checkHomeCodeViewable(ctx) diff --git a/routers/web/user/package.go b/routers/web/user/package.go index ed4f0dd79..a9acc5281 100644 --- a/routers/web/user/package.go +++ b/routers/web/user/package.go @@ -376,7 +376,7 @@ func PackageSettingsPost(ctx *context.Context) { ctx.Flash.Success(ctx.Tr("packages.settings.delete.success")) } - ctx.Redirect(ctx.Package.Owner.HTMLURL() + "/-/packages") + ctx.Redirect(ctx.Package.Owner.HomeLink() + "/-/packages") return } } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 0e342991d..4f0a81656 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -47,7 +47,7 @@ func Profile(ctx *context.Context) { } // advertise feed via meta tag - ctx.Data["FeedURL"] = ctx.ContextUser.HTMLURL() + ctx.Data["FeedURL"] = ctx.ContextUser.HomeLink() // Show OpenID URIs openIDs, err := user_model.GetUserOpenIDs(ctx.ContextUser.ID) diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index c17f8ef15..efb5ec6d4 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -59,7 +59,7 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er Creator: creator, CommitStatus: &git_model.CommitStatus{ SHA: sha, - TargetURL: run.HTMLURL(), + TargetURL: run.Link(), Description: "", Context: ctxname, CreatorID: payload.Pusher.ID,