aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormrsdizzie <info@mrsdizzie.com>2019-03-18 10:00:23 -0400
committertechknowlogick <matti@mdranta.net>2019-03-18 10:00:23 -0400
commitca463856377cf1ed40f8d8bd6f76739bf3711bb1 (patch)
tree21193c00fea251dcd3e812eae7d75c1ff1cc6ebe
parentc151682fae3b77a696af56a232671b41f828d54f (diff)
downloadgitea-ca463856377cf1ed40f8d8bd6f76739bf3711bb1.tar.gz
gitea-ca463856377cf1ed40f8d8bd6f76739bf3711bb1.zip
Clean up various use of escape/unescape functions for URL generation (#6334)
* Use PathUnescape instead of QueryUnescape when working with branch names Currently branch names with a '+' fail in certain situations because QueryUnescape replaces the + character with a blank space. Using PathUnescape should be better since it is defined as: // PathUnescape is identical to QueryUnescape except that it does not // unescape '+' to ' ' (space). Fixes #6333 * Change error to match new function name * Add new util function PathEscapeSegments This function simply runs PathEscape on each segment of a path without touching the forward slash itself. We want to use this instead of PathEscape/QueryEscape in most cases because a forward slash is a valid name for a branch etc... and we don't want that escaped in a URL. Putting this in new file url.go and also moving a couple similar functions into that file as well. * Use EscapePathSegments where appropriate Replace various uses of EscapePath/EscapeQuery with new EscapePathSegments. Also remove uncessary uses of various escape/unescape functions when the text had already been escaped or was not escaped. * Reformat comment to make drone build happy * Remove no longer used url library * Requested code changes
-rw-r--r--cmd/hook.go4
-rw-r--r--integrations/internal_test.go4
-rw-r--r--models/repo.go2
-rw-r--r--modules/context/auth.go8
-rw-r--r--modules/context/context.go3
-rw-r--r--modules/context/repo.go2
-rw-r--r--modules/private/branch.go4
-rw-r--r--modules/util/url.go59
-rw-r--r--modules/util/util.go40
-rw-r--r--routers/home.go3
-rw-r--r--routers/private/repository.go13
-rw-r--r--routers/repo/pull.go6
12 files changed, 80 insertions, 68 deletions
diff --git a/cmd/hook.go b/cmd/hook.go
index bde449ccdc..d3a1a56b01 100644
--- a/cmd/hook.go
+++ b/cmd/hook.go
@@ -8,7 +8,6 @@ import (
"bufio"
"bytes"
"fmt"
- "net/url"
"os"
"strconv"
"strings"
@@ -18,6 +17,7 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/private"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/util"
"github.com/urfave/cli"
)
@@ -239,7 +239,7 @@ func runHookPostReceive(c *cli.Context) error {
branch = fmt.Sprintf("%s:%s", repo.OwnerName, branch)
}
fmt.Fprintf(os.Stderr, "Create a new pull request for '%s':\n", branch)
- fmt.Fprintf(os.Stderr, " %s/compare/%s...%s\n", baseRepo.HTMLURL(), url.QueryEscape(baseRepo.DefaultBranch), url.QueryEscape(branch))
+ fmt.Fprintf(os.Stderr, " %s/compare/%s...%s\n", baseRepo.HTMLURL(), util.PathEscapeSegments(baseRepo.DefaultBranch), util.PathEscapeSegments(branch))
} else {
fmt.Fprint(os.Stderr, "Visit the existing pull request:\n")
fmt.Fprintf(os.Stderr, " %s/pulls/%d\n", baseRepo.HTMLURL(), pr.Index)
diff --git a/integrations/internal_test.go b/integrations/internal_test.go
index 3c8657e642..ee0c0d18f1 100644
--- a/integrations/internal_test.go
+++ b/integrations/internal_test.go
@@ -8,17 +8,17 @@ import (
"encoding/json"
"fmt"
"net/http"
- "net/url"
"testing"
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/util"
"github.com/stretchr/testify/assert"
)
func assertProtectedBranch(t *testing.T, repoID int64, branchName string, isErr, canPush bool) {
- reqURL := fmt.Sprintf("/api/internal/branch/%d/%s", repoID, url.QueryEscape(branchName))
+ reqURL := fmt.Sprintf("/api/internal/branch/%d/%s", repoID, util.PathEscapeSegments(branchName))
req := NewRequest(t, "GET", reqURL)
t.Log(reqURL)
req.Header.Add("Authorization", fmt.Sprintf("Bearer %s", setting.InternalToken))
diff --git a/models/repo.go b/models/repo.go
index 2822f7d718..cafde29363 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -838,7 +838,7 @@ type CloneLink struct {
// ComposeHTTPSCloneURL returns HTTPS clone URL based on given owner and repository name.
func ComposeHTTPSCloneURL(owner, repo string) string {
- return fmt.Sprintf("%s%s/%s.git", setting.AppURL, url.QueryEscape(owner), url.QueryEscape(repo))
+ return fmt.Sprintf("%s%s/%s.git", setting.AppURL, url.PathEscape(owner), url.PathEscape(repo))
}
func (repo *Repository) cloneLink(e Engine, isWiki bool) *CloneLink {
diff --git a/modules/context/auth.go b/modules/context/auth.go
index 9d9bd81c75..ca897de6ed 100644
--- a/modules/context/auth.go
+++ b/modules/context/auth.go
@@ -5,8 +5,6 @@
package context
import (
- "net/url"
-
"code.gitea.io/gitea/modules/auth"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
@@ -48,7 +46,7 @@ func Toggle(options *ToggleOptions) macaron.Handler {
if ctx.Req.URL.Path != "/user/settings/change_password" {
ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password"
- ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
+ ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.RequestURI, 0, setting.AppSubURL)
ctx.Redirect(setting.AppSubURL + "/user/settings/change_password")
return
}
@@ -82,7 +80,7 @@ func Toggle(options *ToggleOptions) macaron.Handler {
return
}
- ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
+ ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.RequestURI, 0, setting.AppSubURL)
ctx.Redirect(setting.AppSubURL + "/user/login")
return
} else if !ctx.User.IsActive && setting.Service.RegisterEmailConfirm {
@@ -95,7 +93,7 @@ func Toggle(options *ToggleOptions) macaron.Handler {
// Redirect to log in page if auto-signin info is provided and has not signed in.
if !options.SignOutRequired && !ctx.IsSigned && !auth.IsAPIPath(ctx.Req.URL.Path) &&
len(ctx.GetCookie(setting.CookieUserName)) > 0 {
- ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
+ ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.RequestURI, 0, setting.AppSubURL)
ctx.Redirect(setting.AppSubURL + "/user/login")
return
}
diff --git a/modules/context/context.go b/modules/context/context.go
index 5d4a2cad54..5c7254de22 100644
--- a/modules/context/context.go
+++ b/modules/context/context.go
@@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/util"
"github.com/Unknwon/com"
"github.com/go-macaron/cache"
"github.com/go-macaron/csrf"
@@ -211,7 +212,7 @@ func Contexter() macaron.Handler {
if err == nil && len(repo.DefaultBranch) > 0 {
branchName = repo.DefaultBranch
}
- prefix := setting.AppURL + path.Join(url.QueryEscape(ownerName), url.QueryEscape(repoName), "src", "branch", branchName)
+ prefix := setting.AppURL + path.Join(url.PathEscape(ownerName), url.PathEscape(repoName), "src", "branch", util.PathEscapeSegments(branchName))
c.Header().Set("Content-Type", "text/html")
c.WriteHeader(http.StatusOK)
c.Write([]byte(com.Expand(`<!doctype html>
diff --git a/modules/context/repo.go b/modules/context/repo.go
index 8f2622fa82..e0ee802f7d 100644
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -172,7 +172,7 @@ func RetrieveBaseRepo(ctx *Context, repo *models.Repository) {
// ComposeGoGetImport returns go-get-import meta content.
func ComposeGoGetImport(owner, repo string) string {
- return path.Join(setting.Domain, setting.AppSubURL, url.QueryEscape(owner), url.QueryEscape(repo))
+ return path.Join(setting.Domain, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo))
}
// EarlyResponseForGoGetMeta responses appropriate go-get meta with status 200
diff --git a/modules/private/branch.go b/modules/private/branch.go
index b6b119e871..bbd0d4b697 100644
--- a/modules/private/branch.go
+++ b/modules/private/branch.go
@@ -7,17 +7,17 @@ package private
import (
"encoding/json"
"fmt"
- "net/url"
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
+ "code.gitea.io/gitea/modules/util"
)
// GetProtectedBranchBy get protected branch information
func GetProtectedBranchBy(repoID int64, branchName string) (*models.ProtectedBranch, error) {
// Ask for running deliver hook and test pull request tasks.
- reqURL := setting.LocalURL + fmt.Sprintf("api/internal/branch/%d/%s", repoID, url.PathEscape(branchName))
+ reqURL := setting.LocalURL + fmt.Sprintf("api/internal/branch/%d/%s", repoID, util.PathEscapeSegments(branchName))
log.GitLogger.Trace("GetProtectedBranchBy: %s", reqURL)
resp, err := newInternalRequest(reqURL, "GET").Response()
diff --git a/modules/util/url.go b/modules/util/url.go
new file mode 100644
index 0000000000..381e8b935b
--- /dev/null
+++ b/modules/util/url.go
@@ -0,0 +1,59 @@
+// Copyright 2019 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package util
+
+import (
+ "net/url"
+ "path"
+ "strings"
+
+ "code.gitea.io/gitea/modules/log"
+ "code.gitea.io/gitea/modules/setting"
+)
+
+// PathEscapeSegments escapes segments of a path while not escaping forward slash
+func PathEscapeSegments(path string) string {
+ slice := strings.Split(path, "/")
+ for index := range slice {
+ slice[index] = url.PathEscape(slice[index])
+ }
+ escapedPath := strings.Join(slice, "/")
+ return escapedPath
+}
+
+// URLJoin joins url components, like path.Join, but preserving contents
+func URLJoin(base string, elems ...string) string {
+ if !strings.HasSuffix(base, "/") {
+ base += "/"
+ }
+ baseURL, err := url.Parse(base)
+ if err != nil {
+ log.Error(4, "URLJoin: Invalid base URL %s", base)
+ return ""
+ }
+ joinedPath := path.Join(elems...)
+ argURL, err := url.Parse(joinedPath)
+ if err != nil {
+ log.Error(4, "URLJoin: Invalid arg %s", joinedPath)
+ return ""
+ }
+ joinedURL := baseURL.ResolveReference(argURL).String()
+ if !baseURL.IsAbs() && !strings.HasPrefix(base, "/") {
+ return joinedURL[1:] // Removing leading '/' if needed
+ }
+ return joinedURL
+}
+
+// IsExternalURL checks if rawURL points to an external URL like http://example.com
+func IsExternalURL(rawURL string) bool {
+ parsed, err := url.Parse(rawURL)
+ if err != nil {
+ return true
+ }
+ if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(setting.Domain, "www.", "", 1) {
+ return true
+ }
+ return false
+}
diff --git a/modules/util/util.go b/modules/util/util.go
index b2067c8df6..4203b5eb51 100644
--- a/modules/util/util.go
+++ b/modules/util/util.go
@@ -5,12 +5,7 @@
package util
import (
- "net/url"
- "path"
"strings"
-
- "code.gitea.io/gitea/modules/log"
- "code.gitea.io/gitea/modules/setting"
)
// OptionalBool a boolean that can be "null"
@@ -56,41 +51,6 @@ func Max(a, b int) int {
return a
}
-// URLJoin joins url components, like path.Join, but preserving contents
-func URLJoin(base string, elems ...string) string {
- if !strings.HasSuffix(base, "/") {
- base += "/"
- }
- baseURL, err := url.Parse(base)
- if err != nil {
- log.Error(4, "URLJoin: Invalid base URL %s", base)
- return ""
- }
- joinedPath := path.Join(elems...)
- argURL, err := url.Parse(joinedPath)
- if err != nil {
- log.Error(4, "URLJoin: Invalid arg %s", joinedPath)
- return ""
- }
- joinedURL := baseURL.ResolveReference(argURL).String()
- if !baseURL.IsAbs() && !strings.HasPrefix(base, "/") {
- return joinedURL[1:] // Removing leading '/' if needed
- }
- return joinedURL
-}
-
-// IsExternalURL checks if rawURL points to an external URL like http://example.com
-func IsExternalURL(rawURL string) bool {
- parsed, err := url.Parse(rawURL)
- if err != nil {
- return true
- }
- if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(setting.Domain, "www.", "", 1) {
- return true
- }
- return false
-}
-
// Min min of two ints
func Min(a, b int) int {
if a > b {
diff --git a/routers/home.go b/routers/home.go
index 437c569a79..a9c4774867 100644
--- a/routers/home.go
+++ b/routers/home.go
@@ -7,7 +7,6 @@ package routers
import (
"bytes"
- "net/url"
"strings"
"code.gitea.io/gitea/models"
@@ -48,7 +47,7 @@ func Home(ctx *context.Context) {
} else if ctx.User.MustChangePassword {
ctx.Data["Title"] = ctx.Tr("auth.must_change_password")
ctx.Data["ChangePasscodeLink"] = setting.AppSubURL + "/user/change_password"
- ctx.SetCookie("redirect_to", url.QueryEscape(setting.AppSubURL+ctx.Req.RequestURI), 0, setting.AppSubURL)
+ ctx.SetCookie("redirect_to", setting.AppSubURL+ctx.Req.RequestURI, 0, setting.AppSubURL)
ctx.Redirect(setting.AppSubURL + "/user/settings/change_password")
} else {
user.Dashboard(ctx)
diff --git a/routers/private/repository.go b/routers/private/repository.go
index 0769e1f250..9f451bcf1d 100644
--- a/routers/private/repository.go
+++ b/routers/private/repository.go
@@ -6,7 +6,6 @@ package private
import (
"net/http"
- "net/url"
"code.gitea.io/gitea/models"
@@ -56,18 +55,18 @@ func GetRepository(ctx *macaron.Context) {
func GetActivePullRequest(ctx *macaron.Context) {
baseRepoID := ctx.QueryInt64("baseRepoID")
headRepoID := ctx.QueryInt64("headRepoID")
- baseBranch, err := url.QueryUnescape(ctx.QueryTrim("baseBranch"))
- if err != nil {
+ baseBranch := ctx.QueryTrim("baseBranch")
+ if len(baseBranch) == 0 {
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
- "err": err.Error(),
+ "err": "QueryTrim failed",
})
return
}
- headBranch, err := url.QueryUnescape(ctx.QueryTrim("headBranch"))
- if err != nil {
+ headBranch := ctx.QueryTrim("headBranch")
+ if len(headBranch) == 0 {
ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
- "err": err.Error(),
+ "err": "QueryTrim failed",
})
return
}
diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index 043fc5c93a..c61ae905f7 100644
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -10,7 +10,6 @@ import (
"container/list"
"fmt"
"io"
- "net/url"
"path"
"strings"
@@ -633,10 +632,7 @@ func ParseCompareInfo(ctx *context.Context) (*models.User, *models.Repository, *
infoPath string
err error
)
- infoPath, err = url.QueryUnescape(ctx.Params("*"))
- if err != nil {
- ctx.NotFound("QueryUnescape", err)
- }
+ infoPath = ctx.Params("*")
infos := strings.Split(infoPath, "...")
if len(infos) != 2 {
log.Trace("ParseCompareInfo[%d]: not enough compared branches information %s", baseRepo.ID, infos)