Multiple Escaping Improvements

cmd/web.go

@@ -67,6 +67,7 @@ func runHTTPRedirector() {
 		if len(r.URL.RawQuery) > 0 {
 			target += "?" + r.URL.RawQuery
 		}
+		// FIXME: Why aren't we adding the Fragment?
 		http.Redirect(w, r, target, http.StatusTemporaryRedirect)
 	})
 

integrations/issue_test.go

@@ -65,7 +65,7 @@ func TestViewIssuesSortByType(t *testing.T) {
 	repo := db.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
 
 	session := loginUser(t, user.Name)
-	req := NewRequest(t, "GET", repo.RelLink()+"/issues?type=created_by")
+	req := NewRequest(t, "GET", repo.Link()+"/issues?type=created_by")
 	resp := session.MakeRequest(t, req, http.StatusOK)
 
 	htmlDoc := NewHTMLParser(t, resp.Body)
@@ -97,7 +97,7 @@ func TestViewIssuesKeyword(t *testing.T) {
 	issues.UpdateIssueIndexer(issue)
 	time.Sleep(time.Second * 1)
 	const keyword = "first"
-	req := NewRequestf(t, "GET", "%s/issues?q=%s", repo.RelLink(), keyword)
+	req := NewRequestf(t, "GET", "%s/issues?q=%s", repo.Link(), keyword)
 	resp := MakeRequest(t, req, http.StatusOK)
 
 	htmlDoc := NewHTMLParser(t, resp.Body)

integrations/links_test.go

@@ -156,7 +156,7 @@ func testLinksAsUser(userName string, t *testing.T) {
 		"/releases",
 		"/releases/new",
 		//"/wiki/_pages",
-		"/wiki/_new",
+		"/wiki/?action=_new",
 	}
 
 	for _, repo := range apiRepos {

integrations/nonascii_branches_test.go

@@ -63,17 +63,17 @@ func TestNonasciiBranches(t *testing.T) {
 		},
 		{
 			from:   "ГлавнаяВетка",
-			to:     "branch/%d0%93%d0%bb%d0%b0%d0%b2%d0%bd%d0%b0%d1%8f%d0%92%d0%b5%d1%82%d0%ba%d0%b0",
+			to:     "branch/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0%D1%8F%D0%92%D0%B5%D1%82%D0%BA%D0%B0",
 			status: http.StatusOK,
 		},
 		{
 			from:   "а/б/в",
-			to:     "branch/%d0%b0/%d0%b1/%d0%b2",
+			to:     "branch/%D0%B0/%D0%B1/%D0%B2",
 			status: http.StatusOK,
 		},
 		{
 			from:   "Grüßen/README.md",
-			to:     "branch/Gr%c3%bc%c3%9fen/README.md",
+			to:     "branch/Gr%C3%BC%C3%9Fen/README.md",
 			status: http.StatusOK,
 		},
 		{
@@ -83,7 +83,7 @@ func TestNonasciiBranches(t *testing.T) {
 		},
 		{
 			from:   "Plus+Is+Not+Space/Файл.md",
-			to:     "branch/Plus+Is+Not+Space/%d0%a4%d0%b0%d0%b9%d0%bb.md",
+			to:     "branch/Plus+Is+Not+Space/%D0%A4%D0%B0%D0%B9%D0%BB.md",
 			status: http.StatusOK,
 		},
 		{
@@ -93,28 +93,28 @@ func TestNonasciiBranches(t *testing.T) {
 		},
 		{
 			from:   "ブランチ",
-			to:     "branch/%e3%83%96%e3%83%a9%e3%83%b3%e3%83%81",
+			to:     "branch/%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81",
 			status: http.StatusOK,
 		},
 		// Tags
 		{
 			from:   "Тэг",
-			to:     "tag/%d0%a2%d1%8d%d0%b3",
+			to:     "tag/%D0%A2%D1%8D%D0%B3",
 			status: http.StatusOK,
 		},
 		{
 			from:   "Ё/人",
-			to:     "tag/%d0%81/%e4%ba%ba",
+			to:     "tag/%D0%81/%E4%BA%BA",
 			status: http.StatusOK,
 		},
 		{
 			from:   "タグ",
-			to:     "tag/%e3%82%bf%e3%82%b0",
+			to:     "tag/%E3%82%BF%E3%82%B0",
 			status: http.StatusOK,
 		},
 		{
 			from:   "タグ/ファイル.md",
-			to:     "tag/%e3%82%bf%e3%82%b0/%e3%83%95%e3%82%a1%e3%82%a4%e3%83%ab.md",
+			to:     "tag/%E3%82%BF%E3%82%B0/%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB.md",
 			status: http.StatusOK,
 		},
 		// Files
@@ -125,38 +125,38 @@ func TestNonasciiBranches(t *testing.T) {
 		},
 		{
 			from:   "Файл.md",
-			to:     "branch/Plus+Is+Not+Space/%d0%a4%d0%b0%d0%b9%d0%bb.md",
+			to:     "branch/Plus+Is+Not+Space/%D0%A4%D0%B0%D0%B9%D0%BB.md",
 			status: http.StatusOK,
 		},
 		{
 			from:   "ファイル.md",
-			to:     "branch/Plus+Is+Not+Space/%e3%83%95%e3%82%a1%e3%82%a4%e3%83%ab.md",
+			to:     "branch/Plus+Is+Not+Space/%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB.md",
 			status: http.StatusNotFound, // it's not on default branch
 		},
 		// Same but url-encoded (few tests)
 		{
 			from:   "%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81",
-			to:     "branch/%e3%83%96%e3%83%a9%e3%83%b3%e3%83%81",
+			to:     "branch/%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81",
 			status: http.StatusOK,
 		},
 		{
 			from:   "%E3%82%BF%E3%82%b0",
-			to:     "tag/%e3%82%bf%e3%82%b0",
+			to:     "tag/%E3%82%BF%E3%82%B0",
 			status: http.StatusOK,
 		},
 		{
 			from:   "%D0%A4%D0%B0%D0%B9%D0%BB.md",
-			to:     "branch/Plus+Is+Not+Space/%d0%a4%d0%b0%d0%b9%d0%bb.md",
+			to:     "branch/Plus+Is+Not+Space/%D0%A4%D0%B0%D0%B9%D0%BB.md",
 			status: http.StatusOK,
 		},
 		{
 			from:   "%D0%81%2F%E4%BA%BA",
-			to:     "tag/%d0%81/%e4%ba%ba",
+			to:     "tag/%D0%81/%E4%BA%BA",
 			status: http.StatusOK,
 		},
 		{
 			from:   "Ё%2F%E4%BA%BA",
-			to:     "tag/%d0%81/%e4%ba%ba",
+			to:     "tag/%D0%81/%E4%BA%BA",
 			status: http.StatusOK,
 		},
 	}

models/action.go

@@ -7,6 +7,7 @@ package models
 
 import (
 	"fmt"
+	"net/url"
 	"path"
 	"strconv"
 	"strings"
@@ -185,10 +186,8 @@ func (a *Action) ShortRepoPath() string {
 
 // GetRepoLink returns relative link to action repository.
 func (a *Action) GetRepoLink() string {
-	if len(setting.AppSubURL) > 0 {
-		return path.Join(setting.AppSubURL, a.GetRepoPath())
-	}
-	return "/" + a.GetRepoPath()
+	// path.Join will skip empty strings
+	return path.Join(setting.AppSubURL, "/", url.PathEscape(a.GetRepoUserName()), url.PathEscape(a.GetRepoName()))
 }
 
 // GetRepositoryFromMatch returns a *Repository from a username and repo strings

models/attachment.go

@@ -7,6 +7,7 @@ package models
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"path"
 
 	"code.gitea.io/gitea/models/db"
@@ -58,7 +59,7 @@ func (a *Attachment) RelativePath() string {
 
 // DownloadURL returns the download url of the attached file
 func (a *Attachment) DownloadURL() string {
-	return fmt.Sprintf("%sattachments/%s", setting.AppURL, a.UUID)
+	return setting.AppURL + "attachments/" + url.PathEscape(a.UUID)
 }
 
 // LinkedRepository returns the linked repo if any

models/avatars/avatar.go

@@ -112,15 +112,15 @@ func GenerateUserAvatarFastLink(userName string, size int) string {
 	if size < 0 {
 		size = 0
 	}
-	return setting.AppSubURL + "/user/avatar/" + userName + "/" + strconv.Itoa(size)
+	return setting.AppSubURL + "/user/avatar/" + url.PathEscape(userName) + "/" + strconv.Itoa(size)
 }
 
 // GenerateUserAvatarImageLink returns a link for `User.Avatar` image file: "/avatars/${User.Avatar}"
 func GenerateUserAvatarImageLink(userAvatar string, size int) string {
 	if size > 0 {
-		return setting.AppSubURL + "/avatars/" + userAvatar + "?size=" + strconv.Itoa(size)
+		return setting.AppSubURL + "/avatars/" + url.PathEscape(userAvatar) + "?size=" + strconv.Itoa(size)
 	}
-	return setting.AppSubURL + "/avatars/" + userAvatar
+	return setting.AppSubURL + "/avatars/" + url.PathEscape(userAvatar)
 }
 
 // generateRecognizedAvatarURL generate a recognized avatar (Gravatar/Libravatar) URL, it modifies the URL so the parameter is passed by a copy
@@ -155,7 +155,7 @@ func generateEmailAvatarLink(email string, size int, final bool) string {
 			return generateRecognizedAvatarURL(*avatarURL, size)
 		}
 		// for non-final link, we should return fast (use a 302 redirection link)
-		urlStr := setting.AppSubURL + "/avatar/" + emailHash
+		urlStr := setting.AppSubURL + "/avatar/" + url.PathEscape(emailHash)
 		if size > 0 {
 			urlStr += "?size=" + strconv.Itoa(size)
 		}

models/commit_status.go

@@ -7,6 +7,7 @@ package models
 import (
 	"crypto/sha1"
 	"fmt"
+	"net/url"
 	"strings"
 	"time"
 
@@ -137,8 +138,7 @@ func (status *CommitStatus) loadAttributes(e db.Engine) (err error) {
 // APIURL returns the absolute APIURL to this commit-status.
 func (status *CommitStatus) APIURL() string {
 	_ = status.loadAttributes(db.GetEngine(db.DefaultContext))
-	return fmt.Sprintf("%sapi/v1/repos/%s/statuses/%s",
-		setting.AppURL, status.Repo.FullName(), status.SHA)
+	return status.Repo.APIURL() + "/statuses/" + url.PathEscape(status.SHA)
 }
 
 // CalcCommitStatus returns commit status state via some status, the commit statues should order by id desc

models/issue.go

@@ -372,6 +372,17 @@ func (issue *Issue) HTMLURL() string {
 	return fmt.Sprintf("%s/%s/%d", issue.Repo.HTMLURL(), path, issue.Index)
 }
 
+// Link returns the Link URL to this issue.
+func (issue *Issue) Link() string {
+	var path string
+	if issue.IsPull {
+		path = "pulls"
+	} else {
+		path = "issues"
+	}
+	return fmt.Sprintf("%s/%s/%d", issue.Repo.Link(), path, issue.Index)
+}
+
 // DiffURL returns the absolute URL to this diff
 func (issue *Issue) DiffURL() string {
 	if issue.IsPull {

models/notification.go

@@ -6,6 +6,7 @@ package models
 
 import (
 	"fmt"
+	"net/url"
 	"strconv"
 
 	"code.gitea.io/gitea/models/db"
@@ -468,7 +469,7 @@ func (n *Notification) HTMLURL() string {
 		}
 		return n.Issue.HTMLURL()
 	case NotificationSourceCommit:
-		return n.Repository.HTMLURL() + "/commit/" + n.CommitID
+		return n.Repository.HTMLURL() + "/commit/" + url.PathEscape(n.CommitID)
 	case NotificationSourceRepository:
 		return n.Repository.HTMLURL()
 	}

models/release.go

@@ -10,10 +10,10 @@ import (
 	"errors"
 	"fmt"
 	"sort"
+	"strconv"
 	"strings"
 
 	"code.gitea.io/gitea/models/db"
-	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/util"
@@ -78,23 +78,22 @@ func (r *Release) LoadAttributes() error {
 
 // APIURL the api url for a release. release must have attributes loaded
 func (r *Release) APIURL() string {
-	return fmt.Sprintf("%sapi/v1/repos/%s/releases/%d",
-		setting.AppURL, r.Repo.FullName(), r.ID)
+	return r.Repo.APIURL() + "/releases/" + strconv.FormatInt(r.ID, 10)
 }
 
 // ZipURL the zip url for a release. release must have attributes loaded
 func (r *Release) ZipURL() string {
-	return fmt.Sprintf("%s/archive/%s.zip", r.Repo.HTMLURL(), r.TagName)
+	return r.Repo.HTMLURL() + "/archive/" + util.PathEscapeSegments(r.TagName) + ".zip"
 }
 
 // TarURL the tar.gz url for a release. release must have attributes loaded
 func (r *Release) TarURL() string {
-	return fmt.Sprintf("%s/archive/%s.tar.gz", r.Repo.HTMLURL(), r.TagName)
+	return r.Repo.HTMLURL() + "/archive/" + util.PathEscapeSegments(r.TagName) + ".tar.gz"
 }
 
 // HTMLURL the url for a release on the web UI. release must have attributes loaded
 func (r *Release) HTMLURL() string {
-	return fmt.Sprintf("%s/releases/tag/%s", r.Repo.HTMLURL(), r.TagName)
+	return r.Repo.HTMLURL() + "/releases/tag/" + util.PathEscapeSegments(r.TagName)
 }
 
 // IsReleaseExist returns true if release with given tag name already exists.

models/repo.go

@@ -312,7 +312,7 @@ func (repo *Repository) FullName() string {
 
 // HTMLURL returns the repository HTML URL
 func (repo *Repository) HTMLURL() string {
-	return setting.AppURL + repo.FullName()
+	return setting.AppURL + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name)
 }
 
 // CommitLink make link to by commit full ID
@@ -321,14 +321,14 @@ func (repo *Repository) CommitLink(commitID string) (result string) {
 	if commitID == "" || commitID == "0000000000000000000000000000000000000000" {
 		result = ""
 	} else {
-		result = repo.HTMLURL() + "/commit/" + commitID
+		result = repo.HTMLURL() + "/commit/" + url.PathEscape(commitID)
 	}
 	return
 }
 
 // APIURL returns the repository API URL
 func (repo *Repository) APIURL() string {
-	return setting.AppURL + "api/v1/repos/" + repo.FullName()
+	return setting.AppURL + "api/v1/repos/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name)
 }
 
 // GetCommitsCountCacheKey returns cache key used for commits count caching.
@@ -707,19 +707,14 @@ func (repo *Repository) GitConfigPath() string {
 	return GitConfigPath(repo.RepoPath())
 }
 
-// RelLink returns the repository relative link
-func (repo *Repository) RelLink() string {
-	return "/" + repo.FullName()
-}
-
 // Link returns the repository link
 func (repo *Repository) Link() string {
-	return setting.AppSubURL + "/" + repo.FullName()
+	return setting.AppSubURL + "/" + url.PathEscape(repo.OwnerName) + "/" + url.PathEscape(repo.Name)
 }
 
 // ComposeCompareURL returns the repository comparison URL
 func (repo *Repository) ComposeCompareURL(oldCommitID, newCommitID string) string {
-	return fmt.Sprintf("%s/compare/%s...%s", repo.FullName(), oldCommitID, newCommitID)
+	return fmt.Sprintf("%s/%s/compare/%s...%s", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name), util.PathEscapeSegments(oldCommitID), util.PathEscapeSegments(newCommitID))
 }
 
 // UpdateDefaultBranch updates the default branch
@@ -928,11 +923,11 @@ func (repo *Repository) cloneLink(isWiki bool) *CloneLink {
 	}
 
 	if setting.SSH.Port != 22 {
-		cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, net.JoinHostPort(setting.SSH.Domain, strconv.Itoa(setting.SSH.Port)), repo.OwnerName, repoName)
+		cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, net.JoinHostPort(setting.SSH.Domain, strconv.Itoa(setting.SSH.Port)), url.PathEscape(repo.OwnerName), url.PathEscape(repoName))
 	} else if setting.Repository.UseCompatSSHURI {
-		cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, sshDomain, repo.OwnerName, repoName)
+		cl.SSH = fmt.Sprintf("ssh://%s@%s/%s/%s.git", sshUser, sshDomain, url.PathEscape(repo.OwnerName), url.PathEscape(repoName))
 	} else {
-		cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", sshUser, sshDomain, repo.OwnerName, repoName)
+		cl.SSH = fmt.Sprintf("%s@%s:%s/%s.git", sshUser, sshDomain, url.PathEscape(repo.OwnerName), url.PathEscape(repoName))
 	}
 	cl.HTTPS = ComposeHTTPSCloneURL(repo.OwnerName, repoName)
 	return cl

models/repo_avatar.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"image/png"
 	"io"
+	"net/url"
 	"strconv"
 	"strings"
 
@@ -96,7 +97,7 @@ func (repo *Repository) relAvatarLink(e db.Engine) string {
 			return ""
 		}
 	}
-	return setting.AppSubURL + "/repo-avatars/" + repo.Avatar
+	return setting.AppSubURL + "/repo-avatars/" + url.PathEscape(repo.Avatar)
 }
 
 // AvatarLink returns a link to the repository's avatar.