From b2c29e29827c45d95441406f45ee08eb9cb36bc3 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Thu, 1 Oct 2020 09:27:22 +0100 Subject: [PATCH] Clarify the suffices and prefixes of setting.AppSubURL and setting.AppURL 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.go | 2 +- modules/markup/html.go | 9 ++++---- modules/setting/setting.go | 40 ++++++++++++++++++++++++------------ 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, 51 insertions(+), 41 deletions(-) diff --git a/models/action_test.go b/models/action_test.go index c6aaaaf069842..6dab676a706b6 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 1f51c99c4c250..31abd8f1bb657 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" @@ -464,7 +464,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 a743f657375ad..5adf65bd6b438 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() } // APIFormat converts a Repository to api.Repository diff --git a/models/user.go b/models/user.go index 63ce6ffdfcb5f..233693a4b5972 100644 --- a/models/user.go +++ b/models/user.go @@ -397,7 +397,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 bef6269a69157..7965eb0f0b54a 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 @@ -577,6 +573,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. if ok && strings.Contains(teams, ","+strings.ToLower(mention[1:])+",") { replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(setting.AppURL, "org", ctx.metas["org"], "teams", mention[1:]), mention, "mention")) } else { diff --git a/modules/setting/setting.go b/modules/setting/setting.go index 867641e61b4fe..2c313481b3666 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -63,16 +63,29 @@ const ( // settings var ( - // AppVer settings - AppVer string - AppBuiltWith string - 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 + // 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 @@ -576,8 +589,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) @@ -588,7 +602,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 5dbc321e9db2e..4a354239f3f95 100644 --- a/routers/admin/admin.go +++ b/routers/admin/admin.go @@ -367,7 +367,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), }) } @@ -390,7 +390,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 @@ -404,23 +404,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 @@ -433,7 +433,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 } @@ -448,7 +448,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 { @@ -462,7 +462,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 { @@ -473,7 +473,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 { @@ -482,5 +482,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 75ae1191a5215..5f0e36386ef30 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 e110ae2a72467..a593ea3caa24e 100644 --- a/routers/repo/search.go +++ b/routers/repo/search.go @@ -36,8 +36,7 @@ func Search(ctx *context.Context) { } ctx.Data["Keyword"] = keyword ctx.Data["Language"] = language - 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 34939d14554f9..523e945db9bbb 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 ec5e69862aa33..78ab3d20ee4f4 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