Skip to content

Commit 2d1a7e1

Browse files
GiteaBotwxiaoguang
andauthored
Introduce ctx.PathParamRaw to avoid incorrect unescaping (#26392) (#26405)
Backport #26392 by @wxiaoguang Fix #26389 And complete an old TODO: `ctx.Params does un-escaping,..., which is incorrect.` Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent df55581 commit 2d1a7e1

File tree

5 files changed

+27
-17
lines changed

5 files changed

+27
-17
lines changed

modules/context/base.go

+4
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ func (b *Base) Params(p string) string {
143143
return s
144144
}
145145

146+
func (b *Base) PathParamRaw(p string) string {
147+
return chi.URLParam(b.Req, strings.TrimPrefix(p, ":"))
148+
}
149+
146150
// ParamsInt64 returns the param on route as int64
147151
func (b *Base) ParamsInt64(p string) int64 {
148152
v, _ := strconv.ParseInt(b.Params(p), 10, 64)

routers/api/v1/repo/wiki.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ func EditWikiPage(ctx *context.APIContext) {
127127

128128
form := web.GetForm(ctx).(*api.CreateWikiPageOptions)
129129

130-
oldWikiName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
130+
oldWikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
131131
newWikiName := wiki_service.UserTitleToWebPath("", form.Title)
132132

133133
if len(newWikiName) == 0 {
@@ -231,7 +231,7 @@ func DeleteWikiPage(ctx *context.APIContext) {
231231
// "404":
232232
// "$ref": "#/responses/notFound"
233233

234-
wikiName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
234+
wikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
235235

236236
if err := wiki_service.DeleteWikiPage(ctx, ctx.Doer, ctx.Repo.Repository, wikiName); err != nil {
237237
if err.Error() == "file does not exist" {
@@ -359,7 +359,7 @@ func GetWikiPage(ctx *context.APIContext) {
359359
// "$ref": "#/responses/notFound"
360360

361361
// get requested pagename
362-
pageName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
362+
pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
363363

364364
wikiPage := getWikiPage(ctx, pageName)
365365
if !ctx.Written() {
@@ -409,7 +409,7 @@ func ListPageRevisions(ctx *context.APIContext) {
409409
}
410410

411411
// get requested pagename
412-
pageName := wiki_service.WebPathFromRequest(ctx.Params(":pageName"))
412+
pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw(":pageName"))
413413
if len(pageName) == 0 {
414414
pageName = "Home"
415415
}

routers/web/repo/wiki.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
186186
ctx.Data["Pages"] = pages
187187

188188
// get requested page name
189-
pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
189+
pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
190190
if len(pageName) == 0 {
191191
pageName = "Home"
192192
}
@@ -333,7 +333,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
333333
}
334334

335335
// get requested pagename
336-
pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
336+
pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
337337
if len(pageName) == 0 {
338338
pageName = "Home"
339339
}
@@ -416,7 +416,7 @@ func renderEditPage(ctx *context.Context) {
416416
}()
417417

418418
// get requested pagename
419-
pageName := wiki_service.WebPathFromRequest(ctx.Params("*"))
419+
pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
420420
if len(pageName) == 0 {
421421
pageName = "Home"
422422
}
@@ -648,7 +648,7 @@ func WikiRaw(ctx *context.Context) {
648648
return
649649
}
650650

651-
providedWebPath := wiki_service.WebPathFromRequest(ctx.Params("*"))
651+
providedWebPath := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
652652
providedGitPath := wiki_service.WebPathToGitPath(providedWebPath)
653653
var entry *git.TreeEntry
654654
if commit != nil {
@@ -760,7 +760,7 @@ func EditWikiPost(ctx *context.Context) {
760760
return
761761
}
762762

763-
oldWikiName := wiki_service.WebPathFromRequest(ctx.Params("*"))
763+
oldWikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
764764
newWikiName := wiki_service.UserTitleToWebPath("", form.Title)
765765

766766
if len(form.Message) == 0 {
@@ -779,7 +779,7 @@ func EditWikiPost(ctx *context.Context) {
779779

780780
// DeleteWikiPagePost delete wiki page
781781
func DeleteWikiPagePost(ctx *context.Context) {
782-
wikiName := wiki_service.WebPathFromRequest(ctx.Params("*"))
782+
wikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
783783
if len(wikiName) == 0 {
784784
wikiName = "Home"
785785
}

services/wiki/wiki_path.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,16 @@ import (
2222
// - "/wiki/100%25+Free"
2323
// - "/wiki/2000-01-02+meeting.-"
2424
// - If a segment has a suffix "DashMarker(.-)", it means that there is no dash-space conversion for this segment.
25-
// - If a WebPath is a "*.md" pattern, then use it directly as GitPath, to make users can access the raw file.
25+
// - If a WebPath is a "*.md" pattern, then use the unescaped value directly as GitPath, to make users can access the raw file.
2626
// * Git Path (only space doesn't need to be escaped):
2727
// - "/.wiki.git/Home-Page.md"
2828
// - "/.wiki.git/100%25 Free.md"
2929
// - "/.wiki.git/2000-01-02 meeting.-.md"
3030
// TODO: support subdirectory in the future
3131
//
32-
// Although this package now has the ablity to support subdirectory, but the route package doesn't:
32+
// Although this package now has the ability to support subdirectory, but the route package doesn't:
3333
// * Double-escaping problem: the URL "/wiki/abc%2Fdef" becomes "/wiki/abc/def" by ctx.Params, which is incorrect
34+
// * This problem should have been 99% fixed, but it needs more tests.
3435
// * 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.
3536

3637
type WebPath string
@@ -91,7 +92,8 @@ func WebPathSegments(s WebPath) []string {
9192

9293
func WebPathToGitPath(s WebPath) string {
9394
if strings.HasSuffix(string(s), ".md") {
94-
return string(s)
95+
ret, _ := url.PathUnescape(string(s))
96+
return util.PathJoinRelX(ret)
9597
}
9698

9799
a := strings.Split(string(s), "/")
@@ -124,7 +126,10 @@ func GitPathToWebPath(s string) (wp WebPath, err error) {
124126
func WebPathToUserTitle(s WebPath) (dir, display string) {
125127
dir = path.Dir(string(s))
126128
display = path.Base(string(s))
127-
display = strings.TrimSuffix(display, ".md")
129+
if strings.HasSuffix(display, ".md") {
130+
display = strings.TrimSuffix(display, ".md")
131+
display, _ = url.PathUnescape(display)
132+
}
128133
display, _ = unescapeSegment(display)
129134
return dir, display
130135
}
@@ -141,8 +146,7 @@ func WebPathFromRequest(s string) WebPath {
141146
}
142147

143148
func UserTitleToWebPath(base, title string) WebPath {
144-
// TODO: ctx.Params does un-escaping, so the URL "/wiki/abc%2Fdef" becomes "wiki path = `abc/def`", which is incorrect.
145-
// And the old wiki code's behavior is always using %2F, instead of subdirectory.
149+
// TODO: no support for subdirectory, because the old wiki code's behavior is always using %2F, instead of subdirectory.
146150
// So we do not add the support for writing slashes in title at the moment.
147151
title = strings.TrimSpace(title)
148152
title = util.PathJoinRelX(base, escapeSegToWeb(title, false))

services/wiki/wiki_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ func TestWebPathToDisplayName(t *testing.T) {
5959
{"name with / slash", "name-with %2F slash"},
6060
{"name with % percent", "name-with %25 percent"},
6161
{"2000-01-02 meeting", "2000-01-02+meeting.-.md"},
62+
{"a b", "a%20b.md"},
6263
} {
6364
_, displayName := WebPathToUserTitle(test.WebPath)
6465
assert.EqualValues(t, test.Expected, displayName)
@@ -73,7 +74,8 @@ func TestWebPathToGitPath(t *testing.T) {
7374
for _, test := range []test{
7475
{"wiki-name.md", "wiki%20name"},
7576
{"wiki-name.md", "wiki+name"},
76-
{"wiki%20name.md", "wiki%20name.md"},
77+
{"wiki name.md", "wiki%20name.md"},
78+
{"wiki%20name.md", "wiki%2520name.md"},
7779
{"2000-01-02-meeting.md", "2000-01-02+meeting"},
7880
{"2000-01-02 meeting.-.md", "2000-01-02%20meeting.-"},
7981
} {

0 commit comments

Comments
 (0)