Introduce ctx.PathParamRaw to avoid incorrect unescaping (go-gitea#26392

) Fix go-gitea#26389 And complete an old TODO: `ctx.Params does un-escaping,..., which is incorrect.`
GiteaBot · Aug 9, 2023 · 992357c · 992357c
1 parent df55581
commit 992357c
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 17 deletions.
diff --git a/modules/context/base.go b/modules/context/base.go
@@ -143,6 +143,10 @@ func (b *Base) Params(p string) string {
 	return s
 }
 
+func (b *Base) PathParamRaw(p string) string {
+	return chi.URLParam(b.Req, strings.TrimPrefix(p, ":"))
+}
+
 // ParamsInt64 returns the param on route as int64
 func (b *Base) ParamsInt64(p string) int64 {
 	v, _ := strconv.ParseInt(b.Params(p), 10, 64)

diff --git a/routers/api/v1/repo/wiki.go b/routers/api/v1/repo/wiki.go
@@ -127,7 +127,7 @@ func EditWikiPage(ctx *context.APIContext) {
 
 	form := web.GetForm(ctx).(*api.CreateWikiPageOptions)
 
-	oldWikiName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
+	oldWikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
 	newWikiName := wiki_service.UserTitleToWebPath("", form.Title)
 
 	if len(newWikiName) == 0 {
@@ -231,7 +231,7 @@ func DeleteWikiPage(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	wikiName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
+	wikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
 
 	if err := wiki_service.DeleteWikiPage(ctx, ctx.Doer, ctx.Repo.Repository, wikiName); err != nil {
 		if err.Error() == "file does not exist" {
@@ -359,7 +359,7 @@ func GetWikiPage(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	// get requested pagename
-	pageName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
+	pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
 
 	wikiPage := getWikiPage(ctx, pageName)
 	if !ctx.Written() {
@@ -409,7 +409,7 @@ func ListPageRevisions(ctx *context.APIContext) {
 	}
 
 	// get requested pagename
-	pageName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
+	pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
 	if len(pageName) == 0 {
 		pageName = "Home"
 	}

diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go
@@ -186,7 +186,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
 	ctx.Data["Pages"] = pages
 
 	// get requested page name
-	pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	if len(pageName) == 0 {
 		pageName = "Home"
 	}
@@ -333,7 +333,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
 	}
 
 	// get requested pagename
-	pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	if len(pageName) == 0 {
 		pageName = "Home"
 	}
@@ -416,7 +416,7 @@ func renderEditPage(ctx *context.Context) {
 	}()
 
 	// get requested pagename
-	pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	if len(pageName) == 0 {
 		pageName = "Home"
 	}
@@ -648,7 +648,7 @@ func WikiRaw(ctx *context.Context) {
 		return
 	}
 
-	providedWebPath := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	providedWebPath := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	providedGitPath := wiki_service.WebPathToGitPath(providedWebPath)
 	var entry *git.TreeEntry
 	if commit != nil {
@@ -760,7 +760,7 @@ func EditWikiPost(ctx *context.Context) {
 		return
 	}
 
-	oldWikiName := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	oldWikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	newWikiName := wiki_service.UserTitleToWebPath("", form.Title)
 
 	if len(form.Message) == 0 {
@@ -779,7 +779,7 @@ func EditWikiPost(ctx *context.Context) {
 
 // DeleteWikiPagePost delete wiki page
 func DeleteWikiPagePost(ctx *context.Context) {
-	wikiName := wiki_service.WebPathFromRequest(ctx.Params("*"))
+	wikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
 	if len(wikiName) == 0 {
 		wikiName = "Home"
 	}

diff --git a/services/wiki/wiki_path.go b/services/wiki/wiki_path.go
@@ -22,15 +22,16 @@ import (
 //   - "/wiki/100%25+Free"
 //   - "/wiki/2000-01-02+meeting.-"
 //   - If a segment has a suffix "DashMarker(.-)", it means that there is no dash-space conversion for this segment.
-//   - If a WebPath is a "*.md" pattern, then use it directly as GitPath, to make users can access the raw file.
+//   - If a WebPath is a "*.md" pattern, then use the unescaped value directly as GitPath, to make users can access the raw file.
 // * Git Path (only space doesn't need to be escaped):
 //   - "/.wiki.git/Home-Page.md"
 //   - "/.wiki.git/100%25 Free.md"
 //   - "/.wiki.git/2000-01-02 meeting.-.md"
 // TODO: support subdirectory in the future
 //
-// Although this package now has the ablity to support subdirectory, but the route package doesn't:
+// Although this package now has the ability to support subdirectory, but the route package doesn't:
 // * Double-escaping problem: the URL "/wiki/abc%2Fdef" becomes "/wiki/abc/def" by ctx.Params, which is incorrect
+//   * This problem should have been 99% fixed, but it needs more tests.
 // * The old wiki code's behavior is always using %2F, instead of subdirectory, so there are a lot of legacy "%2F" files in user wikis.
 
 type WebPath string
@@ -91,7 +92,8 @@ func WebPathSegments(s WebPath) []string {
 
 func WebPathToGitPath(s WebPath) string {
 	if strings.HasSuffix(string(s), ".md") {
-		return string(s)
+		ret, _ := url.PathUnescape(string(s))
+		return util.PathJoinRelX(ret)
 	}
 
 	a := strings.Split(string(s), "/")
@@ -124,7 +126,10 @@ func GitPathToWebPath(s string) (wp WebPath, err error) {
 func WebPathToUserTitle(s WebPath) (dir, display string) {
 	dir = path.Dir(string(s))
 	display = path.Base(string(s))
-	display = strings.TrimSuffix(display, ".md")
+	if strings.HasSuffix(display, ".md") {
+		display = strings.TrimSuffix(display, ".md")
+		display, _ = url.PathUnescape(display)
+	}
 	display, _ = unescapeSegment(display)
 	return dir, display
 }
@@ -141,8 +146,7 @@ func WebPathFromRequest(s string) WebPath {
 }
 
 func UserTitleToWebPath(base, title string) WebPath {
-	// TODO: ctx.Params does un-escaping, so the URL "/wiki/abc%2Fdef" becomes "wiki path = `abc/def`", which is incorrect.
-	// And the old wiki code's behavior is always using %2F, instead of subdirectory.
+	// TODO: no support for subdirectory, because the old wiki code's behavior is always using %2F, instead of subdirectory.
 	// So we do not add the support for writing slashes in title at the moment.
 	title = strings.TrimSpace(title)
 	title = util.PathJoinRelX(base, escapeSegToWeb(title, false))

diff --git a/services/wiki/wiki_test.go b/services/wiki/wiki_test.go
@@ -59,6 +59,7 @@ func TestWebPathToDisplayName(t *testing.T) {
 		{"name with / slash", "name-with %2F slash"},
 		{"name with % percent", "name-with %25 percent"},
 		{"2000-01-02 meeting", "2000-01-02+meeting.-.md"},
+		{"a b", "a%20b.md"},
 	} {
 		_, displayName := WebPathToUserTitle(test.WebPath)
 		assert.EqualValues(t, test.Expected, displayName)
@@ -73,7 +74,8 @@ func TestWebPathToGitPath(t *testing.T) {
 	for _, test := range []test{
 		{"wiki-name.md", "wiki%20name"},
 		{"wiki-name.md", "wiki+name"},
-		{"wiki%20name.md", "wiki%20name.md"},
+		{"wiki name.md", "wiki%20name.md"},
+		{"wiki%20name.md", "wiki%2520name.md"},
 		{"2000-01-02-meeting.md", "2000-01-02+meeting"},
 		{"2000-01-02 meeting.-.md", "2000-01-02%20meeting.-"},
 	} {