From 6275a3d7ffd3464a6d6864646393133121d6c339 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 9 Jan 2025 20:38:27 +0800 Subject: [PATCH] fix --- services/context/api.go | 3 +- services/context/repo.go | 38 ++++++------ .../integration/api_repo_get_contents_test.go | 58 +++++++++++-------- 3 files changed, 52 insertions(+), 47 deletions(-) diff --git a/services/context/api.go b/services/context/api.go index bda705cb48304..3a3cbe670e4fa 100644 --- a/services/context/api.go +++ b/services/context/api.go @@ -293,8 +293,7 @@ func RepoRefForAPI(next http.Handler) http.Handler { return } - // NOTICE: the "ref" here for internal usage only (e.g. woodpecker) - refName, _ := getRefNameLegacy(ctx.Base, ctx.Repo, 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 4de905ef2cb73..94b2972c2bd3b 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -765,35 +765,30 @@ func getRefNameFromPath(repo *Repository, path string, isExist func(string) bool return "" } -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 != "" { +func getRefNameLegacy(ctx *Base, repo *Repository, reqPath, extraRef string) (string, RepoRefType) { + reqRefPath := path.Join(extraRef, reqPath) + reqRefPathParts := strings.Split(reqRefPath, "/") + if refName := getRefName(ctx, repo, reqRefPath, RepoRefBranch); refName != "" { return refName, RepoRefBranch } - if refName := getRefName(ctx, repo, RepoRefTag); refName != "" { + if refName := getRefName(ctx, repo, reqRefPath, RepoRefTag); refName != "" { return refName, RepoRefTag } - - // For legacy support only full commit sha - parts := strings.Split(reqPath, "/") - if git.IsStringLikelyCommitID(git.ObjectFormatFromName(repo.Repository.ObjectFormatName), parts[0]) { + 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(parts[1:], "/") - return parts[0], RepoRefCommit + repo.TreePath = strings.Join(reqRefPathParts[1:], "/") + return reqRefPathParts[0], RepoRefCommit } - - if refName := getRefName(ctx, repo, RepoRefBlob); len(refName) > 0 { + if refName := getRefName(ctx, repo, reqPath, RepoRefBlob); refName != "" { return refName, RepoRefBlob } + // 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, RepoRefBranch } -func getRefName(ctx *Base, repo *Repository, pathType RepoRefType) string { - path := ctx.PathParam("*") +func getRefName(ctx *Base, repo *Repository, path string, pathType RepoRefType) string { switch pathType { case RepoRefBranch: ref := getRefNameFromPath(repo, path, repo.GitRepo.IsBranchExist) @@ -889,7 +884,8 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func } // Get default branch. - if len(ctx.PathParam("*")) == 0 { + reqPath := ctx.PathParam("*") + if reqPath == "" { refName = ctx.Repo.Repository.DefaultBranch if !ctx.Repo.GitRepo.IsBranchExist(refName) { brs, _, err := ctx.Repo.GitRepo.GetBranches(0, 1) @@ -914,12 +910,12 @@ func RepoRefByType(detectRefType RepoRefType, opts ...RepoRefByTypeOptions) func return } ctx.Repo.IsViewBranch = true - } else { + } else { // there is a path in request guessLegacyPath := refType == RepoRefUnknown if guessLegacyPath { - refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo) + refName, refType = getRefNameLegacy(ctx.Base, ctx.Repo, reqPath, "") } else { - refName = getRefName(ctx.Base, ctx.Repo, refType) + refName = getRefName(ctx.Base, ctx.Repo, reqPath, refType) } ctx.Repo.RefName = refName isRenamedBranch, has := ctx.Data["IsRenamedBranch"].(bool) diff --git a/tests/integration/api_repo_get_contents_test.go b/tests/integration/api_repo_get_contents_test.go index 68a8608117208..d0f61da0c0402 100644 --- a/tests/integration/api_repo_get_contents_test.go +++ b/tests/integration/api_repo_get_contents_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" repo_service "code.gitea.io/gitea/services/repository" + "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" ) @@ -168,28 +169,37 @@ func testAPIGetContents(t *testing.T, u *url.URL) { } func TestAPIGetContentsRefFormats(t *testing.T) { - onGiteaRun(t, func(t *testing.T, u *url.URL) { - file := "README.md" - sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" - content := "# repo1\n\nDescription for repo1" - - noRef := setting.AppURL + "api/v1/repos/user2/repo1/raw/" + file - refInPath := setting.AppURL + "api/v1/repos/user2/repo1/raw/" + sha + "/" + file - refInQuery := setting.AppURL + "api/v1/repos/user2/repo1/raw/" + file + "?ref=" + sha - - resp := MakeRequest(t, NewRequest(t, http.MethodGet, noRef), http.StatusOK) - raw, err := io.ReadAll(resp.Body) - assert.NoError(t, err) - assert.EqualValues(t, content, string(raw)) - - resp = MakeRequest(t, NewRequest(t, http.MethodGet, refInPath), http.StatusOK) - raw, err = io.ReadAll(resp.Body) - assert.NoError(t, err) - assert.EqualValues(t, content, string(raw)) - - resp = MakeRequest(t, NewRequest(t, http.MethodGet, refInQuery), http.StatusOK) - raw, err = io.ReadAll(resp.Body) - assert.NoError(t, err) - assert.EqualValues(t, content, string(raw)) - }) + defer tests.PrepareTestEnv(t)() + + file := "README.md" + sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d" + content := "# repo1\n\nDescription for repo1" + + resp := MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/"+file), http.StatusOK) + raw, err := io.ReadAll(resp.Body) + assert.NoError(t, err) + assert.EqualValues(t, content, string(raw)) + + resp = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/"+sha+"/"+file), http.StatusOK) + raw, err = io.ReadAll(resp.Body) + assert.NoError(t, err) + assert.EqualValues(t, content, string(raw)) + + resp = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/"+file+"?ref="+sha), http.StatusOK) + raw, err = io.ReadAll(resp.Body) + assert.NoError(t, err) + assert.EqualValues(t, content, string(raw)) + + resp = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/"+file+"?ref=master"), http.StatusOK) + raw, err = io.ReadAll(resp.Body) + assert.NoError(t, err) + assert.EqualValues(t, content, string(raw)) + + _ = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/docs/README.md?ref=main"), http.StatusNotFound) + _ = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/README.md?ref=main"), http.StatusOK) + _ = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/docs/README.md?ref=sub-home-md-img-check"), http.StatusOK) + _ = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/README.md?ref=sub-home-md-img-check"), http.StatusNotFound) + + // FIXME: this is an incorrect behavior, non-existing branch falls back to default branch + _ = MakeRequest(t, NewRequest(t, http.MethodGet, "/api/v1/repos/user2/repo1/raw/README.md?ref=no-such"), http.StatusOK) }