Introduce ctx.PathParamRaw to avoid incorrect unescaping (#26392)

Fix #26389

And complete an old TODO: `ctx.Params does un-escaping,..., which is
incorrect.`
This commit is contained in:
wxiaoguang 2023-08-09 14:57:45 +08:00 committed by GitHub
parent 906e253d5e
commit c2e0143bfe
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 27 additions and 17 deletions

View file

@ -147,6 +147,10 @@ func (b *Base) Params(p string) string {
return s 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 // ParamsInt64 returns the param on route as int64
func (b *Base) ParamsInt64(p string) int64 { func (b *Base) ParamsInt64(p string) int64 {
v, _ := strconv.ParseInt(b.Params(p), 10, 64) v, _ := strconv.ParseInt(b.Params(p), 10, 64)

View file

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

View file

@ -185,7 +185,7 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) {
ctx.Data["Pages"] = pages ctx.Data["Pages"] = pages
// get requested page name // get requested page name
pageName := wiki_service.WebPathFromRequest(ctx.Params("*")) pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
if len(pageName) == 0 { if len(pageName) == 0 {
pageName = "Home" pageName = "Home"
} }
@ -332,7 +332,7 @@ func renderRevisionPage(ctx *context.Context) (*git.Repository, *git.TreeEntry)
} }
// get requested pagename // get requested pagename
pageName := wiki_service.WebPathFromRequest(ctx.Params("*")) pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
if len(pageName) == 0 { if len(pageName) == 0 {
pageName = "Home" pageName = "Home"
} }
@ -415,7 +415,7 @@ func renderEditPage(ctx *context.Context) {
}() }()
// get requested pagename // get requested pagename
pageName := wiki_service.WebPathFromRequest(ctx.Params("*")) pageName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
if len(pageName) == 0 { if len(pageName) == 0 {
pageName = "Home" pageName = "Home"
} }
@ -647,7 +647,7 @@ func WikiRaw(ctx *context.Context) {
return return
} }
providedWebPath := wiki_service.WebPathFromRequest(ctx.Params("*")) providedWebPath := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
providedGitPath := wiki_service.WebPathToGitPath(providedWebPath) providedGitPath := wiki_service.WebPathToGitPath(providedWebPath)
var entry *git.TreeEntry var entry *git.TreeEntry
if commit != nil { if commit != nil {
@ -759,7 +759,7 @@ func EditWikiPost(ctx *context.Context) {
return return
} }
oldWikiName := wiki_service.WebPathFromRequest(ctx.Params("*")) oldWikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
newWikiName := wiki_service.UserTitleToWebPath("", form.Title) newWikiName := wiki_service.UserTitleToWebPath("", form.Title)
if len(form.Message) == 0 { if len(form.Message) == 0 {
@ -778,7 +778,7 @@ func EditWikiPost(ctx *context.Context) {
// DeleteWikiPagePost delete wiki page // DeleteWikiPagePost delete wiki page
func DeleteWikiPagePost(ctx *context.Context) { func DeleteWikiPagePost(ctx *context.Context) {
wikiName := wiki_service.WebPathFromRequest(ctx.Params("*")) wikiName := wiki_service.WebPathFromRequest(ctx.PathParamRaw("*"))
if len(wikiName) == 0 { if len(wikiName) == 0 {
wikiName = "Home" wikiName = "Home"
} }

View file

@ -22,15 +22,16 @@ import (
// - "/wiki/100%25+Free" // - "/wiki/100%25+Free"
// - "/wiki/2000-01-02+meeting.-" // - "/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 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): // * Git Path (only space doesn't need to be escaped):
// - "/.wiki.git/Home-Page.md" // - "/.wiki.git/Home-Page.md"
// - "/.wiki.git/100%25 Free.md" // - "/.wiki.git/100%25 Free.md"
// - "/.wiki.git/2000-01-02 meeting.-.md" // - "/.wiki.git/2000-01-02 meeting.-.md"
// TODO: support subdirectory in the future // 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 // * 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. // * 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 type WebPath string
@ -91,7 +92,8 @@ func WebPathSegments(s WebPath) []string {
func WebPathToGitPath(s WebPath) string { func WebPathToGitPath(s WebPath) string {
if strings.HasSuffix(string(s), ".md") { if strings.HasSuffix(string(s), ".md") {
return string(s) ret, _ := url.PathUnescape(string(s))
return util.PathJoinRelX(ret)
} }
a := strings.Split(string(s), "/") a := strings.Split(string(s), "/")
@ -124,7 +126,10 @@ func GitPathToWebPath(s string) (wp WebPath, err error) {
func WebPathToUserTitle(s WebPath) (dir, display string) { func WebPathToUserTitle(s WebPath) (dir, display string) {
dir = path.Dir(string(s)) dir = path.Dir(string(s))
display = path.Base(string(s)) display = path.Base(string(s))
if strings.HasSuffix(display, ".md") {
display = strings.TrimSuffix(display, ".md") display = strings.TrimSuffix(display, ".md")
display, _ = url.PathUnescape(display)
}
display, _ = unescapeSegment(display) display, _ = unescapeSegment(display)
return dir, display return dir, display
} }
@ -141,8 +146,7 @@ func WebPathFromRequest(s string) WebPath {
} }
func UserTitleToWebPath(base, title 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. // TODO: no support for subdirectory, because the old wiki code's behavior is always using %2F, instead of subdirectory.
// And 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. // So we do not add the support for writing slashes in title at the moment.
title = strings.TrimSpace(title) title = strings.TrimSpace(title)
title = util.PathJoinRelX(base, escapeSegToWeb(title, false)) title = util.PathJoinRelX(base, escapeSegToWeb(title, false))

View file

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