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 <art27@cantab.net>

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