diff --git a/routers/web/repo/branch.go b/routers/web/repo/branch.go index 8747526f72..6f8d4d9959 100644 --- a/routers/web/repo/branch.go +++ b/routers/web/repo/branch.go @@ -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"))) } diff --git a/routers/web/repo/view_home.go b/routers/web/repo/view_home.go index 6c6e007b50..e7255cde0a 100644 --- a/routers/web/repo/view_home.go +++ b/routers/web/repo/view_home.go @@ -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) } diff --git a/routers/web/web.go b/routers/web/web.go index 096f1e6bbe..b02bbe037f 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -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) diff --git a/services/context/api.go b/services/context/api.go index 3a3cbe670e..bdeff0af63 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -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) { diff --git a/services/context/repo.go b/services/context/repo.go index b0cfd78cf5..1cb35b9b83 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -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 } diff --git a/tests/integration/links_test.go b/tests/integration/links_test.go index b54f670c23..1bfb3b83d2 100644 --- a/tests/integration/links_test.go +++ b/tests/integration/links_test.go @@ -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 { diff --git a/tests/integration/nonascii_branches_test.go b/tests/integration/nonascii_branches_test.go index ae348d8173..cc71acf002 100644 --- a/tests/integration/nonascii_branches_test.go +++ b/tests/integration/nonascii_branches_test.go @@ -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, }, {