Fix "redirect link" handling (#33440)

`a%2fb` should not redirect to `a/b`

---------

Co-authored-by: delvh <dev.lh@web.de>
pull/33447/head^2
wxiaoguang 16 hours ago committed by GitHub
parent f88dbf86b3
commit f24d73ab5f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -93,7 +93,7 @@ func Branches(ctx *context.Context) {
// DeleteBranchPost responses for delete merged branch
func DeleteBranchPost(ctx *context.Context) {
defer redirect(ctx)
defer jsonRedirectBranches(ctx)
branchName := ctx.FormString("name")
if err := repo_service.DeleteBranch(ctx, ctx.Doer, ctx.Repo.Repository, ctx.Repo.GitRepo, branchName, nil); err != nil {
@ -120,7 +120,7 @@ func DeleteBranchPost(ctx *context.Context) {
// RestoreBranchPost responses for delete merged branch
func RestoreBranchPost(ctx *context.Context) {
defer redirect(ctx)
defer jsonRedirectBranches(ctx)
branchID := ctx.FormInt64("branch_id")
branchName := ctx.FormString("name")
@ -170,7 +170,7 @@ func RestoreBranchPost(ctx *context.Context) {
ctx.Flash.Success(ctx.Tr("repo.branch.restore_success", deletedBranch.Name))
}
func redirect(ctx *context.Context) {
func jsonRedirectBranches(ctx *context.Context) {
ctx.JSONRedirect(ctx.Repo.RepoLink + "/branches?page=" + url.QueryEscape(ctx.FormString("page")))
}

@ -413,8 +413,19 @@ func Home(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplRepoHome)
}
// HomeRedirect redirects from /tree/* to /src/* in order to maintain a similar URL structure.
func HomeRedirect(ctx *context.Context) {
remainder := ctx.PathParam("*")
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + util.PathEscapeSegments(remainder))
func RedirectRepoTreeToSrc(ctx *context.Context) {
// Redirect "/owner/repo/tree/*" requests to "/owner/repo/src/*",
// then use the deprecated "/src/*" handler to guess the ref type and render a file list page.
// This is done intentionally so that Gitea's repo URL structure matches other forges (GitHub/GitLab) provide,
// allowing us to construct submodule URLs across forges easily.
// For example, when viewing a submodule, we can simply construct the link as:
// * "https://gitea/owner/repo/tree/{CommitID}"
// * "https://github/owner/repo/tree/{CommitID}"
// * "https://gitlab/owner/repo/tree/{CommitID}"
// Then no matter which forge the submodule is using, the link works.
redirect := ctx.Repo.RepoLink + "/src/" + ctx.PathParamRaw("*")
if ctx.Req.URL.RawQuery != "" {
redirect += "?" + ctx.Req.URL.RawQuery
}
ctx.Redirect(redirect)
}

@ -1583,13 +1583,7 @@ func registerRoutes(m *web.Router) {
m.Get("/commit/*", context.RepoRefByType(git.RefTypeCommit), repo.Home)
m.Get("/*", context.RepoRefByType(""), repo.Home) // "/*" route is deprecated, and kept for backward compatibility
}, repo.SetEditorconfigIfExists)
// Add a /tree/* path to redirect to the /src/* path, which
// will redirect to the canonical URL for that ref. This is
// included so that Gitea's repo URL structure matches what
// other forges provide, allowing clients to construct URLs
// that work across forges.
m.Get("/tree/*", repo.HomeRedirect)
m.Get("/tree/*", repo.RedirectRepoTreeToSrc) // redirect "/owner/repo/tree/*" requests to "/owner/repo/src/*"
m.Get("/forks", context.RepoRef(), repo.Forks)
m.Get("/commit/{sha:([a-f0-9]{7,64})}.{ext:patch|diff}", repo.MustBeNotEmpty, repo.RawDiff)

@ -293,7 +293,7 @@ func RepoRefForAPI(next http.Handler) http.Handler {
return
}
refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref"))
refName, _, _ := getRefNameLegacy(ctx.Base, ctx.Repo, ctx.PathParam("*"), ctx.FormTrim("ref"))
var err error
if ctx.Repo.GitRepo.IsBranchExist(refName) {

@ -686,24 +686,24 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool
return ""
}
func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, git.RefType) {
func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (refName string, refType git.RefType, fallbackDefaultBranch bool) {
reqRefPath := path.Join(extraRef, reqPath)
reqRefPathParts := strings.Split(reqRefPath, "/")
if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeBranch); refName != "" {
return refName, git.RefTypeBranch
return refName, git.RefTypeBranch, false
}
if refName := getRefName(ctx, repo, reqRefPath, git.RefTypeTag); refName != "" {
return refName, git.RefTypeTag
return refName, git.RefTypeTag, false
}
if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), reqRefPathParts[0]) {
// FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists
repo.TreePath = strings.Join(reqRefPathParts[1:], "/")
return reqRefPathParts[0], git.RefTypeCommit
return reqRefPathParts[0], git.RefTypeCommit, false
}
// FIXME: the old code falls back to default branch if "ref" doesn't exist, there could be an edge case:
// "README?ref=no-such" would read the README file from the default branch, but the user might expect a 404
repo.TreePath = reqPath
return repo.Repository.DefaultBranch, git.RefTypeBranch
return repo.Repository.DefaultBranch, git.RefTypeBranch, true
}
func getRefName(ctx *Base, repo *Repository, path string, refType git.RefType) string {
@ -838,8 +838,9 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) {
}
} else { // there is a path in request
guessLegacyPath := refType == ""
fallbackDefaultBranch := false
if guessLegacyPath {
refShortName, refType = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "")
refShortName, refType, fallbackDefaultBranch = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "")
} else {
refShortName = getRefName(ctx.Base, ctx.Repo, reqPath, refType)
}
@ -897,12 +898,24 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) {
if guessLegacyPath {
// redirect from old URL scheme to new URL scheme
prefix := strings.TrimPrefix(setting.AppSubURL+strings.ToLower(strings.TrimSuffix(ctx.Req.URL.Path, ctx.PathParam("*"))), strings.ToLower(ctx.Repo.RepoLink))
redirect := path.Join(
ctx.Repo.RepoLink,
util.PathEscapeSegments(prefix),
ctx.Repo.RefTypeNameSubURL(),
util.PathEscapeSegments(ctx.Repo.TreePath))
// * /user2/repo1/commits/master => /user2/repo1/commits/branch/master
// * /user2/repo1/src/master => /user2/repo1/src/branch/master
// * /user2/repo1/src/README.md => /user2/repo1/src/branch/master/README.md (fallback to default branch)
var redirect string
refSubPath := "src"
// remove the "/subpath/owner/repo/" prefix, the names are case-insensitive
remainingLowerPath, cut := strings.CutPrefix(setting.AppSubURL+strings.ToLower(ctx.Req.URL.Path), strings.ToLower(ctx.Repo.RepoLink)+"/")
if cut {
refSubPath, _, _ = strings.Cut(remainingLowerPath, "/") // it could be "src" or "commits"
}
if fallbackDefaultBranch {
redirect = fmt.Sprintf("%s/%s/%s/%s/%s", ctx.Repo.RepoLink, refSubPath, refType, util.PathEscapeSegments(refShortName), ctx.PathParamRaw("*"))
} else {
redirect = fmt.Sprintf("%s/%s/%s/%s", ctx.Repo.RepoLink, refSubPath, refType, ctx.PathParamRaw("*"))
}
if ctx.Req.URL.RawQuery != "" {
redirect += "?" + ctx.Req.URL.RawQuery
}
ctx.Redirect(redirect)
return
}

@ -52,9 +52,11 @@ func TestRedirectsNoLogin(t *testing.T) {
redirects := []struct{ from, to string }{
{"/user2/repo1/commits/master", "/user2/repo1/commits/branch/master"},
{"/user2/repo1/src/master", "/user2/repo1/src/branch/master"},
{"/user2/repo1/src/master/file.txt", "/user2/repo1/src/branch/master/file.txt"},
{"/user2/repo1/src/master/directory/file.txt", "/user2/repo1/src/branch/master/directory/file.txt"},
{"/user/avatar/Ghost/-1", "/assets/img/avatar_default.png"},
{"/user2/repo1/src/master/a%2fb.txt", "/user2/repo1/src/branch/master/a%2fb.txt"},
{"/user2/repo1/src/master/directory/file.txt?a=1", "/user2/repo1/src/branch/master/directory/file.txt?a=1"},
{"/user2/repo1/tree/a%2fb?a=1", "/user2/repo1/src/a%2fb?a=1"},
{"/user/avatar/GhosT/-1", "/assets/img/avatar_default.png"},
{"/user/avatar/Gitea-ActionS/0", "/assets/img/avatar_default.png"},
{"/api/v1/swagger", "/api/swagger"},
}
for _, c := range redirects {

@ -46,21 +46,21 @@ func TestNonAsciiBranches(t *testing.T) {
{
from: "master/badfile",
to: "branch/master/badfile",
status: http.StatusNotFound, // it does not exists
status: http.StatusNotFound, // it does not exist
},
{
from: "ГлавнаяВетка",
to: "branch/%D0%93%D0%BB%D0%B0%D0%B2%D0%BD%D0%B0%D1%8F%D0%92%D0%B5%D1%82%D0%BA%D0%B0",
to: "branch/%d0%93%d0%bb%d0%b0%d0%b2%d0%bd%d0%b0%d1%8f%d0%92%d0%b5%d1%82%d0%ba%d0%b0",
status: http.StatusOK,
},
{
from: "а/б/в",
to: "branch/%D0%B0/%D0%B1/%D0%B2",
to: "branch/%d0%b0/%d0%b1/%d0%b2",
status: http.StatusOK,
},
{
from: "Grüßen/README.md",
to: "branch/Gr%C3%BC%C3%9Fen/README.md",
to: "branch/Gr%c3%bc%c3%9fen/README.md",
status: http.StatusOK,
},
{
@ -70,7 +70,7 @@ func TestNonAsciiBranches(t *testing.T) {
},
{
from: "Plus+Is+Not+Space/Файл.md",
to: "branch/Plus+Is+Not+Space/%D0%A4%D0%B0%D0%B9%D0%BB.md",
to: "branch/Plus+Is+Not+Space/%d0%a4%d0%b0%d0%b9%d0%bb.md",
status: http.StatusOK,
},
{
@ -80,29 +80,29 @@ func TestNonAsciiBranches(t *testing.T) {
},
{
from: "ブランチ",
to: "branch/%E3%83%96%E3%83%A9%E3%83%B3%E3%83%81",
to: "branch/%e3%83%96%e3%83%a9%e3%83%b3%e3%83%81",
status: http.StatusOK,
},
// Tags
{
from: "Тэг",
to: "tag/%D0%A2%D1%8D%D0%B3",
to: "tag/%d0%a2%d1%8d%d0%b3",
status: http.StatusOK,
},
{
from: "Ё/人",
to: "tag/%D0%81/%E4%BA%BA",
to: "tag/%d0%81/%e4%ba%ba",
status: http.StatusOK,
},
{
from: "タグ",
to: "tag/%E3%82%BF%E3%82%B0",
to: "tag/%e3%82%bf%e3%82%b0",
status: http.StatusOK,
},
{
from: "タグ/ファイル.md",
to: "tag/%E3%82%BF%E3%82%B0/%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB.md",
to: "tag/%e3%82%bf%e3%82%b0/%e3%83%95%e3%82%a1%e3%82%a4%e3%83%ab.md",
status: http.StatusOK,
},
@ -114,12 +114,12 @@ func TestNonAsciiBranches(t *testing.T) {
},
{
from: "Файл.md",
to: "branch/Plus+Is+Not+Space/%D0%A4%D0%B0%D0%B9%D0%BB.md",
to: "branch/Plus+Is+Not+Space/%d0%a4%d0%b0%d0%b9%d0%bb.md",
status: http.StatusOK,
},
{
from: "ファイル.md",
to: "branch/Plus+Is+Not+Space/%E3%83%95%E3%82%A1%E3%82%A4%E3%83%AB.md",
to: "branch/Plus+Is+Not+Space/%e3%83%95%e3%82%a1%e3%82%a4%e3%83%ab.md",
status: http.StatusNotFound, // it's not on default branch
},
@ -131,7 +131,7 @@ func TestNonAsciiBranches(t *testing.T) {
},
{
from: "%E3%82%BF%E3%82%b0",
to: "tag/%E3%82%BF%E3%82%B0",
to: "tag/%E3%82%BF%E3%82%b0",
status: http.StatusOK,
},
{
@ -141,12 +141,12 @@ func TestNonAsciiBranches(t *testing.T) {
},
{
from: "%D0%81%2F%E4%BA%BA",
to: "tag/%D0%81/%E4%BA%BA",
to: "tag/%D0%81%2F%E4%BA%BA",
status: http.StatusOK,
},
{
from: "Ё%2F%E4%BA%BA",
to: "tag/%D0%81/%E4%BA%BA",
to: "tag/%d0%81%2F%E4%BA%BA",
status: http.StatusOK,
},
{

Loading…
Cancel
Save