Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce ctx.PathParamRaw to avoid incorrect unescaping #26392

Merged
merged 4 commits into from
Aug 9, 2023
Merged

Introduce ctx.PathParamRaw to avoid incorrect unescaping #26392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4102473
fix
wxiaoguang Aug 8, 2023
43f46db
fix typo
wxiaoguang Aug 8, 2023
1aee8d0
add more comments
wxiaoguang Aug 8, 2023
9f6d3eb
Merge branch 'main' into fix-wiki-path-param
wxiaoguang Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions modules/context/base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,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)
Expand Down
8 changes: 4 additions & 4 deletions routers/api/v1/repo/wiki.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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" {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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"
}
Expand Down
12 changes: 6 additions & 6 deletions routers/web/repo/wiki.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,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"
}
Expand Down Expand Up @@ -332,7 +332,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"
}
Expand Down Expand Up @@ -415,7 +415,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"
}
Expand Down Expand Up @@ -647,7 +647,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 {
Expand Down Expand Up @@ -759,7 +759,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 {
Expand All @@ -778,7 +778,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"
}
Expand Down
16 changes: 10 additions & 6 deletions services/wiki/wiki_path.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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), "/")
Expand Down Expand Up @@ -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
}
Expand All @@ -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))
Expand Down
4 changes: 3 additions & 1 deletion services/wiki/wiki_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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.-"},
} {
Expand Down