From 61c35590c793449d3133b00f58302b5cce90d69c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 5 Nov 2024 14:35:54 +0800 Subject: [PATCH] Refactor RepoRefByType (#32413) 1. clarify the "filepath" could(should) contain "{ref}" 2. remove unclear RepoRefLegacy and RepoRefAny, use RepoRefUnknown to guess 3. by the way, avoid using AppURL --- routers/api/v1/repo/file.go | 8 +- routers/web/goget.go | 2 +- routers/web/web.go | 12 +-- services/context/api.go | 21 +---- services/context/repo.go | 138 +++++++++++++++++--------------- templates/swagger/v1_json.tmpl | 8 +- tests/integration/links_test.go | 20 ++--- 7 files changed, 100 insertions(+), 109 deletions(-) diff --git a/routers/api/v1/repo/file.go b/routers/api/v1/repo/file.go index 97f7a49390..05650cc9be 100644 --- a/routers/api/v1/repo/file.go +++ b/routers/api/v1/repo/file.go @@ -56,12 +56,12 @@ func GetRawFile(ctx *context.APIContext) { // required: true // - name: filepath // in: path - // description: filepath of the file to get + // description: path of the file to get, it should be "{ref}/{filepath}". If there is no ref could be inferred, it will be treated as the default branch // type: string // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default the repository’s default branch" // type: string // required: false // responses: @@ -109,12 +109,12 @@ func GetRawFileOrLFS(ctx *context.APIContext) { // required: true // - name: filepath // in: path - // description: filepath of the file to get + // description: path of the file to get, it should be "{ref}/{filepath}". If there is no ref could be inferred, it will be treated as the default branch // type: string // required: true // - name: ref // in: query - // description: "The name of the commit/branch/tag. Default the repository’s default branch (usually master)" + // description: "The name of the commit/branch/tag. Default the repository’s default branch" // type: string // required: false // responses: diff --git a/routers/web/goget.go b/routers/web/goget.go index 8d5612ebfe..3714dd8eb0 100644 --- a/routers/web/goget.go +++ b/routers/web/goget.go @@ -65,7 +65,7 @@ func goGet(ctx *context.Context) { insecure = "--insecure " } - goGetImport := context.ComposeGoGetImport(ownerName, trimmedRepoName) + goGetImport := context.ComposeGoGetImport(ctx, ownerName, trimmedRepoName) var cloneURL string if setting.Repository.GoGetCloneURLProtocol == "ssh" { diff --git a/routers/web/web.go b/routers/web/web.go index 83d116babd..69ed4bd8e4 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1323,7 +1323,7 @@ func registerRoutes(m *web.Router) { m.Get(".rss", feedEnabled, repo.TagsListFeedRSS) m.Get(".atom", feedEnabled, repo.TagsListFeedAtom) }, ctxDataSet("EnableFeed", setting.Other.EnableFeed), - repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, true)) + repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) m.Post("/tags/delete", repo.DeleteTag, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoCodeWriter, context.RepoRef()) }, ignSignIn, context.RepoAssignment, reqRepoCodeReader) @@ -1337,7 +1337,7 @@ func registerRoutes(m *web.Router) { m.Get(".rss", feedEnabled, repo.ReleasesFeedRSS) m.Get(".atom", feedEnabled, repo.ReleasesFeedAtom) }, ctxDataSet("EnableFeed", setting.Other.EnableFeed), - repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, true)) + repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, context.RepoRefByTypeOptions{IgnoreNotExistErr: true})) m.Get("/releases/attachments/{uuid}", repo.MustBeNotEmpty, repo.GetAttachment) m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload) m.Group("/releases", func() { @@ -1535,7 +1535,7 @@ func registerRoutes(m *web.Router) { m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownloadOrLFS) m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByIDOrLFS) // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownloadOrLFS) + m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownloadOrLFS) }, repo.MustBeNotEmpty) m.Group("/raw", func() { @@ -1544,7 +1544,7 @@ func registerRoutes(m *web.Router) { m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload) m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByID) // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload) + m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.SingleDownload) }, repo.MustBeNotEmpty) m.Group("/render", func() { @@ -1559,7 +1559,7 @@ func registerRoutes(m *web.Router) { m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RefCommits) m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.RefCommits) // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.RefCommits) + m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.RefCommits) }, repo.MustBeNotEmpty) m.Group("/blame", func() { @@ -1582,7 +1582,7 @@ func registerRoutes(m *web.Router) { m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home) m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home) m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home) - m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.Home) // "/*" route is deprecated, and kept for backward compatibility + m.Get("/*", context.RepoRefByType(context.RepoRefUnknown), repo.Home) // "/*" route is deprecated, and kept for backward compatibility }, repo.SetEditorconfigIfExists) m.Get("/forks", context.RepoRef(), repo.Forks) diff --git a/services/context/api.go b/services/context/api.go index 00cfd6afd9..b45e80a329 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -14,7 +14,6 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/cache" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/httpcache" "code.gitea.io/gitea/modules/log" @@ -306,24 +305,8 @@ func RepoRefForAPI(next http.Handler) http.Handler { return } - if ref := ctx.FormTrim("ref"); len(ref) > 0 { - commit, err := ctx.Repo.GitRepo.GetCommit(ref) - if err != nil { - if git.IsErrNotExist(err) { - ctx.NotFound() - } else { - ctx.Error(http.StatusInternalServerError, "GetCommit", err) - } - return - } - ctx.Repo.Commit = commit - ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - ctx.Repo.TreePath = ctx.PathParam("*") - next.ServeHTTP(w, req) - return - } - - refName := getRefName(ctx.Base, ctx.Repo, RepoRefAny) + // NOTICE: the "ref" here for internal usage only (e.g. woodpecker) + refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, 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 2df2b7ea40..e7b32d6283 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/cache" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/httplib" code_indexer "code.gitea.io/gitea/modules/indexer/code" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/optional" @@ -306,11 +307,9 @@ func RetrieveTemplateRepo(ctx *Context, repo *repo_model.Repository) { } // ComposeGoGetImport returns go-get-import meta content. -func ComposeGoGetImport(owner, repo string) string { - /// setting.AppUrl is guaranteed to be parse as url - appURL, _ := url.Parse(setting.AppURL) - - return path.Join(appURL.Host, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo)) +func ComposeGoGetImport(ctx context.Context, owner, repo string) string { + curAppURL, _ := url.Parse(httplib.GuessCurrentAppURL(ctx)) + return path.Join(curAppURL.Host, setting.AppSubURL, url.PathEscape(owner), url.PathEscape(repo)) } // EarlyResponseForGoGetMeta responses appropriate go-get meta with status 200 @@ -332,7 +331,7 @@ func EarlyResponseForGoGetMeta(ctx *Context) { } else { cloneURL = repo_model.ComposeHTTPSCloneURL(username, reponame) } - goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(username, reponame), cloneURL) + goImportContent := fmt.Sprintf("%s git %s", ComposeGoGetImport(ctx, username, reponame), cloneURL) htmlMeta := fmt.Sprintf(``, html.EscapeString(goImportContent)) ctx.PlainText(http.StatusOK, htmlMeta) } @@ -744,7 +743,7 @@ func RepoAssignment(ctx *Context) context.CancelFunc { } if ctx.FormString("go-get") == "1" { - ctx.Data["GoGetImport"] = ComposeGoGetImport(owner.Name, repo.Name) + ctx.Data["GoGetImport"] = ComposeGoGetImport(ctx, owner.Name, repo.Name) fullURLPrefix := repo.HTMLURL() + "/src/branch/" + util.PathEscapeSegments(ctx.Repo.BranchName) ctx.Data["GoDocDirectory"] = fullURLPrefix + "{/dir}" ctx.Data["GoDocFile"] = fullURLPrefix + "{/dir}/{file}#L{line}" @@ -756,19 +755,11 @@ func RepoAssignment(ctx *Context) context.CancelFunc { type RepoRefType int const ( - // RepoRefLegacy unknown type, make educated guess and redirect. - // for backward compatibility with previous URL scheme - RepoRefLegacy RepoRefType = iota - // RepoRefAny is for usage where educated guess is needed - // but redirect can not be made - RepoRefAny - // RepoRefBranch branch + // RepoRefUnknown is for legacy support, makes the code to "guess" the ref type + RepoRefUnknown RepoRefType = iota RepoRefBranch - // RepoRefTag tag RepoRefTag - // RepoRefCommit commit RepoRefCommit - // RepoRefBlob blob RepoRefBlob ) @@ -781,22 +772,6 @@ func RepoRef() func(*Context) context.CancelFunc { return RepoRefByType(RepoRefBranch) } -// RefTypeIncludesBranches returns true if ref type can be a branch -func (rt RepoRefType) RefTypeIncludesBranches() bool { - if rt == RepoRefLegacy || rt == RepoRefAny || rt == RepoRefBranch { - return true - } - return false -} - -// RefTypeIncludesTags returns true if ref type can be a tag -func (rt RepoRefType) RefTypeIncludesTags() bool { - if rt == RepoRefLegacy || rt == RepoRefAny || rt == RepoRefTag { - return true - } - return false -} - func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool) string { refName := "" parts := strings.Split(path, "/") @@ -810,28 +785,50 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool return "" } +func isStringLikelyCommitID(objFmt git.ObjectFormat, s string, minLength ...int) bool { + minLen := util.OptionalArg(minLength, objFmt.FullLength()) + if len(s) < minLen || len(s) > objFmt.FullLength() { + return false + } + for _, c := range s { + isHex := (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') + if !isHex { + return false + } + } + return true +} + +func getRefNameLegacy(ctx *Base, repo *Repository, optionalExtraRef ...string) (string, RepoRefType) { + extraRef := util.OptionalArg(optionalExtraRef) + reqPath := ctx.PathParam("*") + reqPath = path.Join(extraRef, reqPath) + + if refName := getRefName(ctx, repo, RepoRefBranch); refName != "" { + return refName, RepoRefBranch + } + if refName := getRefName(ctx, repo, RepoRefTag); refName != "" { + return refName, RepoRefTag + } + + // For legacy support only full commit sha + parts := strings.Split(reqPath, "/") + if isStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[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(parts[1:], "/") + return parts[0], RepoRefCommit + } + + if refName := getRefName(ctx, repo, RepoRefBlob); len(refName) > 0 { + return refName, RepoRefBlob + } + repo.TreePath = reqPath + return repo.Repository.DefaultBranch, RepoRefBranch +} + func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { path := ctx.PathParam("*") switch pathType { - case RepoRefLegacy, RepoRefAny: - if refName := getRefName(ctx, repo, RepoRefBranch); len(refName) > 0 { - return refName - } - if refName := getRefName(ctx, repo, RepoRefTag); len(refName) > 0 { - return refName - } - // For legacy and API support only full commit sha - parts := strings.Split(path, "/") - - if len(parts) > 0 && len(parts[0]) == git.ObjectFormatFromName(repo.Repository.ObjectFormatName).FullLength() { - repo.TreePath = strings.Join(parts[1:], "/") - return parts[0] - } - if refName := getRefName(ctx, repo, RepoRefBlob); len(refName) > 0 { - return refName - } - repo.TreePath = path - return repo.Repository.DefaultBranch case RepoRefBranch: ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist) if len(ref) == 0 { @@ -866,13 +863,13 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { return getRefNameFromPath(repo, path, repo.GitRepo.IsTagExist) case RepoRefCommit: parts := strings.Split(path, "/") - - if len(parts) > 0 && len(parts[0]) >= 7 && len(parts[0]) <= repo.GetObjectFormat().FullLength() { + if isStringLikelyCommitID(repo.GetObjectFormat(), parts[0], 7) { + // FIXME: this logic is different from other types. Ideally, it should also try to GetCommit to check if it exists repo.TreePath = strings.Join(parts[1:], "/") return parts[0] } - if len(parts) > 0 && parts[0] == headRefName { + if parts[0] == headRefName { // HEAD ref points to last default branch commit commit, err := repo.GitRepo.GetBranchCommit(repo.Repository.DefaultBranch) if err != nil { @@ -888,15 +885,21 @@ func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { } return path default: - log.Error("Unrecognized path type: %v", path) + panic(fmt.Sprintf("Unrecognized path type: %v", pathType)) } return "" } +type RepoRefByTypeOptions struct { + IgnoreNotExistErr bool +} + // RepoRefByType handles repository reference name for a specific type // of repository reference -func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context) context.CancelFunc { +func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func(*Context) context.CancelFunc { + opt := util.OptionalArg(opts) return func(ctx *Context) (cancel context.CancelFunc) { + refType := detectRefType // Empty repository does not have reference information. if ctx.Repo.Repository.IsEmpty { // assume the user is viewing the (non-existent) default branch @@ -956,7 +959,12 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context } ctx.Repo.IsViewBranch = true } else { - refName = getRefName(ctx.Base, ctx.Repo, refType) + guessLegacyPath := refType == RepoRefUnknown + if guessLegacyPath { + refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo) + } else { + refName = getRefName(ctx.Base, ctx.Repo, refType) + } ctx.Repo.RefName = refName isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool) if isRenamedBranch && has { @@ -967,7 +975,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context return cancel } - if refType.RefTypeIncludesBranches() && ctx.Repo.GitRepo.IsBranchExist(refName) { + if refType == RepoRefBranch && ctx.Repo.GitRepo.IsBranchExist(refName) { ctx.Repo.IsViewBranch = true ctx.Repo.BranchName = refName @@ -977,7 +985,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if refType.RefTypeIncludesTags() && ctx.Repo.GitRepo.IsTagExist(refName) { + } else if refType == RepoRefTag && ctx.Repo.GitRepo.IsTagExist(refName) { ctx.Repo.IsViewTag = true ctx.Repo.TagName = refName @@ -991,7 +999,7 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context return cancel } ctx.Repo.CommitID = ctx.Repo.Commit.ID.String() - } else if len(refName) >= 7 && len(refName) <= ctx.Repo.GetObjectFormat().FullLength() { + } else if isStringLikelyCommitID(ctx.Repo.GetObjectFormat(), refName, 7) { ctx.Repo.IsViewCommit = true ctx.Repo.CommitID = refName @@ -1002,18 +1010,18 @@ func RepoRefByType(refType RepoRefType, ignoreNotExistErr ...bool) func(*Context } // If short commit ID add canonical link header if len(refName) < ctx.Repo.GetObjectFormat().FullLength() { - ctx.RespHeader().Set("Link", fmt.Sprintf("<%s>; rel=\"canonical\"", - util.URLJoin(setting.AppURL, strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1)))) + canonicalURL := util.URLJoin(httplib.GuessCurrentAppURL(ctx), strings.Replace(ctx.Req.URL.RequestURI(), util.PathEscapeSegments(refName), url.PathEscape(ctx.Repo.Commit.ID.String()), 1)) + ctx.RespHeader().Set("Link", fmt.Sprintf(`<%s>; rel="canonical"`, canonicalURL)) } } else { - if len(ignoreNotExistErr) > 0 && ignoreNotExistErr[0] { + if opt.IgnoreNotExistErr { return cancel } ctx.NotFound("RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName)) return cancel } - if refType == RepoRefLegacy { + 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( diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4cbf511aac..92a8888f32 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -10727,14 +10727,14 @@ }, { "type": "string", - "description": "filepath of the file to get", + "description": "path of the file to get, it should be \"{ref}/{filepath}\". If there is no ref could be inferred, it will be treated as the default branch", "name": "filepath", "in": "path", "required": true }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default the repository’s default branch", "name": "ref", "in": "query" } @@ -12818,14 +12818,14 @@ }, { "type": "string", - "description": "filepath of the file to get", + "description": "path of the file to get, it should be \"{ref}/{filepath}\". If there is no ref could be inferred, it will be treated as the default branch", "name": "filepath", "in": "path", "required": true }, { "type": "string", - "description": "The name of the commit/branch/tag. Default the repository’s default branch (usually master)", + "description": "The name of the commit/branch/tag. Default the repository’s default branch", "name": "ref", "in": "query" } diff --git a/tests/integration/links_test.go b/tests/integration/links_test.go index d3b30448fc..b54f670c23 100644 --- a/tests/integration/links_test.go +++ b/tests/integration/links_test.go @@ -49,18 +49,18 @@ func TestLinksNoLogin(t *testing.T) { func TestRedirectsNoLogin(t *testing.T) { defer tests.PrepareTestEnv(t)() - redirects := map[string]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", - "/api/v1/swagger": "/api/swagger", + 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"}, + {"/api/v1/swagger", "/api/swagger"}, } - for link, redirectLink := range redirects { - req := NewRequest(t, "GET", link) + for _, c := range redirects { + req := NewRequest(t, "GET", c.from) resp := MakeRequest(t, req, http.StatusSeeOther) - assert.EqualValues(t, path.Join(setting.AppSubURL, redirectLink), test.RedirectURL(resp)) + assert.EqualValues(t, path.Join(setting.AppSubURL, c.to), test.RedirectURL(resp)) } }