From aa4f9180e4932a9813e898bf3bd9ac91924eeb03 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 19 Feb 2021 21:36:43 +0000 Subject: [PATCH] Clarify the suffices and prefixes of setting.AppSubURL and setting.AppURL (#12999) Also removes some unnecessary uses of fmt.Sprintf and adds documentation strings Signed-off-by: Andrew Thornton --- models/action_test.go | 2 +- models/notification.go | 4 ++-- models/repo.go | 2 +- models/user_avatar.go | 2 +- modules/markup/html.go | 9 ++++---- modules/setting/setting.go | 43 ++++++++++++++++++++++++------------ routers/admin/admin.go | 22 +++++++++--------- routers/api/v1/org/member.go | 4 +--- routers/repo/search.go | 3 +-- routers/user/notification.go | 2 +- routers/utils/utils_test.go | 2 +- 11 files changed, 53 insertions(+), 42 deletions(-) diff --git a/models/action_test.go b/models/action_test.go index c6aaaaf069..6dab676a70 100644 --- a/models/action_test.go +++ b/models/action_test.go @@ -26,7 +26,7 @@ func TestAction_GetRepoLink(t *testing.T) { repo := AssertExistsAndLoadBean(t, &Repository{}).(*Repository) owner := AssertExistsAndLoadBean(t, &User{ID: repo.OwnerID}).(*User) action := &Action{RepoID: repo.ID} - setting.AppSubURL = "/suburl/" + setting.AppSubURL = "/suburl" expected := path.Join(setting.AppSubURL, owner.Name, repo.Name) assert.Equal(t, expected, action.GetRepoLink()) } diff --git a/models/notification.go b/models/notification.go index 362b490994..32f3079bf4 100644 --- a/models/notification.go +++ b/models/notification.go @@ -6,7 +6,7 @@ package models import ( "fmt" - "path" + "strconv" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -413,7 +413,7 @@ func (n *Notification) HTMLURL() string { // APIURL formats a URL-string to the notification func (n *Notification) APIURL() string { - return setting.AppURL + path.Join("api/v1/notifications/threads", fmt.Sprintf("%d", n.ID)) + return setting.AppURL + "api/v1/notifications/threads/" + strconv.FormatInt(n.ID, 10) } // NotificationList contains a list of notifications diff --git a/models/repo.go b/models/repo.go index a7bc566971..03259d9e34 100644 --- a/models/repo.go +++ b/models/repo.go @@ -318,7 +318,7 @@ func (repo *Repository) CommitLink(commitID string) (result string) { // APIURL returns the repository API URL func (repo *Repository) APIURL() string { - return setting.AppURL + path.Join("api/v1/repos", repo.FullName()) + return setting.AppURL + "api/v1/repos/" + repo.FullName() } // GetCommitsCountCacheKey returns cache key used for commits count caching. diff --git a/models/user_avatar.go b/models/user_avatar.go index 1e9f0e2da4..871e176599 100644 --- a/models/user_avatar.go +++ b/models/user_avatar.go @@ -63,7 +63,7 @@ func (u *User) generateRandomAvatar(e Engine) error { // the local explore page. Function returns immediately. // When applicable, the link is for an avatar of the indicated size (in pixels). func (u *User) SizedRelAvatarLink(size int) string { - return strings.TrimSuffix(setting.AppSubURL, "/") + "/user/avatar/" + u.Name + "/" + strconv.Itoa(size) + return setting.AppSubURL + "/user/avatar/" + u.Name + "/" + strconv.Itoa(size) } // RealSizedAvatarLink returns a link to the user's avatar. When diff --git a/modules/markup/html.go b/modules/markup/html.go index 2c2feb0b34..b254fd083f 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -89,11 +89,7 @@ func isLinkStr(link string) bool { func getIssueFullPattern() *regexp.Regexp { if issueFullPattern == nil { - appURL := setting.AppURL - if len(appURL) > 0 && appURL[len(appURL)-1] != '/' { - appURL += "/" - } - issueFullPattern = regexp.MustCompile(appURL + + issueFullPattern = regexp.MustCompile(regexp.QuoteMeta(setting.AppURL) + `\w+/\w+/(?:issues|pulls)/((?:\w{1,10}-)?[1-9][0-9]*)([\?|#]\S+.(\S+)?)?\b`) } return issueFullPattern @@ -636,6 +632,9 @@ func mentionProcessor(ctx *postProcessCtx, node *html.Node) { mention := node.Data[loc.Start:loc.End] var teams string teams, ok := ctx.metas["teams"] + // FIXME: util.URLJoin may not be necessary here: + // - setting.AppURL is defined to have a terminal '/' so unless mention[1:] + // is an AppSubURL link we can probably fallback to concatenation. // team mention should follow @orgName/teamName style if ok && strings.Contains(mention, "/") { mentionOrgAndTeam := strings.Split(mention, "/") diff --git a/modules/setting/setting.go b/modules/setting/setting.go index be7ec16e10..783e440da1 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -66,17 +66,31 @@ const ( // settings var ( - // AppVer settings - AppVer string - AppBuiltWith string - AppStartTime time.Time - AppName string - AppURL string - AppSubURL string - AppSubURLDepth int // Number of slashes - AppPath string - AppDataPath string - AppWorkPath string + // AppVer is the version of the current build of Gitea. It is set in main.go from main.Version. + AppVer string + // AppBuiltWith represents a human readable version go runtime build version and build tags. (See main.go formatBuiltWith().) + AppBuiltWith string + // AppStartTime store time gitea has started + AppStartTime time.Time + // AppName is the Application name, used in the page title. + // It maps to ini:"APP_NAME" + AppName string + // AppURL is the Application ROOT_URL. It always has a '/' suffix + // It maps to ini:"ROOT_URL" + AppURL string + // AppSubURL represents the sub-url mounting point for gitea. It is either "" or starts with '/' and ends without '/', such as '/{subpath}'. + // This value is empty if site does not have sub-url. + AppSubURL string + // AppPath represents the path to the gitea binary + AppPath string + // AppWorkPath is the "working directory" of Gitea. It maps to the environment variable GITEA_WORK_DIR. + // If that is not set it is the default set here by the linker or failing that the directory of AppPath. + // + // AppWorkPath is used as the base path for several other paths. + AppWorkPath string + // AppDataPath is the default path for storing data. + // It maps to ini:"APP_DATA_PATH" and defaults to AppWorkPath + "/data" + AppDataPath string // Server settings Protocol Scheme @@ -594,8 +608,9 @@ func NewContext() { if (Protocol == HTTP && HTTPPort != "80") || (Protocol == HTTPS && HTTPPort != "443") { defaultAppURL += ":" + HTTPPort } - AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL) - AppURL = strings.TrimSuffix(AppURL, "/") + "/" + AppURL = sec.Key("ROOT_URL").MustString(defaultAppURL + "/") + // This should be TrimRight to ensure that there is only a single '/' at the end of AppURL. + AppURL = strings.TrimRight(AppURL, "/") + "/" // Check if has app suburl. appURL, err := url.Parse(AppURL) @@ -606,7 +621,7 @@ func NewContext() { // This value is empty if site does not have sub-url. AppSubURL = strings.TrimSuffix(appURL.Path, "/") StaticURLPrefix = strings.TrimSuffix(sec.Key("STATIC_URL_PREFIX").MustString(AppSubURL), "/") - AppSubURLDepth = strings.Count(AppSubURL, "/") + // Check if Domain differs from AppURL domain than update it to AppURL's domain urlHostname := appURL.Hostname() if urlHostname != Domain && net.ParseIP(urlHostname) == nil && urlHostname != "" { diff --git a/routers/admin/admin.go b/routers/admin/admin.go index c8f9d8b35b..2b2328a93a 100644 --- a/routers/admin/admin.go +++ b/routers/admin/admin.go @@ -364,7 +364,7 @@ func WorkerCancel(ctx *context.Context) { mq.CancelWorkers(pid) ctx.Flash.Info(ctx.Tr("admin.monitor.queue.pool.cancelling")) ctx.JSON(200, map[string]interface{}{ - "redirect": setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid), + "redirect": setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10), }) } @@ -387,7 +387,7 @@ func Flush(ctx *context.Context) { log.Error("Flushing failure for %s: Error %v", mq.Name, err) } }() - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) } // AddWorkers adds workers to a worker group @@ -401,23 +401,23 @@ func AddWorkers(ctx *context.Context) { number := ctx.QueryInt("number") if number < 1 { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.pool.addworkers.mustnumbergreaterzero")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } timeout, err := time.ParseDuration(ctx.Query("timeout")) if err != nil { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.pool.addworkers.musttimeoutduration")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } if _, ok := mq.Managed.(queue.ManagedPool); !ok { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.pool.none")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } mq.AddWorkers(number, timeout) ctx.Flash.Success(ctx.Tr("admin.monitor.queue.pool.added")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) } // SetQueueSettings sets the maximum number of workers and other settings for this queue @@ -430,7 +430,7 @@ func SetQueueSettings(ctx *context.Context) { } if _, ok := mq.Managed.(queue.ManagedPool); !ok { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.pool.none")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } @@ -445,7 +445,7 @@ func SetQueueSettings(ctx *context.Context) { maxNumber, err = strconv.Atoi(maxNumberStr) if err != nil { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.settings.maxnumberworkers.error")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } if maxNumber < -1 { @@ -459,7 +459,7 @@ func SetQueueSettings(ctx *context.Context) { number, err = strconv.Atoi(numberStr) if err != nil || number < 0 { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.settings.numberworkers.error")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } } else { @@ -470,7 +470,7 @@ func SetQueueSettings(ctx *context.Context) { timeout, err = time.ParseDuration(timeoutStr) if err != nil { ctx.Flash.Error(ctx.Tr("admin.monitor.queue.settings.timeout.error")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) return } } else { @@ -479,5 +479,5 @@ func SetQueueSettings(ctx *context.Context) { mq.SetPoolSettings(maxNumber, number, timeout) ctx.Flash.Success(ctx.Tr("admin.monitor.queue.settings.changed")) - ctx.Redirect(setting.AppSubURL + fmt.Sprintf("/admin/monitor/queue/%d", qid)) + ctx.Redirect(setting.AppSubURL + "/admin/monitor/queue/" + strconv.FormatInt(qid, 10)) } diff --git a/routers/api/v1/org/member.go b/routers/api/v1/org/member.go index 75ae1191a5..5f0e36386e 100644 --- a/routers/api/v1/org/member.go +++ b/routers/api/v1/org/member.go @@ -5,7 +5,6 @@ package org import ( - "fmt" "net/http" "code.gitea.io/gitea/models" @@ -153,8 +152,7 @@ func IsMember(ctx *context.APIContext) { } } - redirectURL := fmt.Sprintf("%sapi/v1/orgs/%s/public_members/%s", - setting.AppURL, ctx.Org.Organization.Name, userToCheck.Name) + redirectURL := setting.AppURL + "api/v1/orgs/" + ctx.Org.Organization.Name + "/public_members/" + userToCheck.Name ctx.Redirect(redirectURL, 302) } diff --git a/routers/repo/search.go b/routers/repo/search.go index 42fe3d7584..481b64d184 100644 --- a/routers/repo/search.go +++ b/routers/repo/search.go @@ -40,8 +40,7 @@ func Search(ctx *context.Context) { ctx.Data["Keyword"] = keyword ctx.Data["Language"] = language ctx.Data["queryType"] = queryType - ctx.Data["SourcePath"] = setting.AppSubURL + "/" + - path.Join(ctx.Repo.Repository.Owner.Name, ctx.Repo.Repository.Name) + ctx.Data["SourcePath"] = path.Join(setting.AppSubURL, ctx.Repo.Repository.Owner.Name, ctx.Repo.Repository.Name) ctx.Data["SearchResults"] = searchResults ctx.Data["SearchResultLanguages"] = searchResultLanguages ctx.Data["RequireHighlightJS"] = true diff --git a/routers/user/notification.go b/routers/user/notification.go index 34939d1455..523e945db9 100644 --- a/routers/user/notification.go +++ b/routers/user/notification.go @@ -174,7 +174,7 @@ func NotificationStatusPost(c *context.Context) { if c.Written() { return } - c.Data["Link"] = fmt.Sprintf("%snotifications", setting.AppURL) + c.Data["Link"] = setting.AppURL + "notifications" c.HTML(http.StatusOK, tplNotificationDiv) } diff --git a/routers/utils/utils_test.go b/routers/utils/utils_test.go index ec5e69862a..78ab3d20ee 100644 --- a/routers/utils/utils_test.go +++ b/routers/utils/utils_test.go @@ -35,7 +35,7 @@ func TestIsValidSlackChannel(t *testing.T) { } func TestIsExternalURL(t *testing.T) { - setting.AppURL = "https://try.gitea.io" + setting.AppURL = "https://try.gitea.io/" type test struct { Expected bool RawURL string