From 1322cd7a58f6c2041624eddaf9393cc8947e5a5d Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sat, 4 Mar 2023 00:38:11 -0500 Subject: [PATCH 1/3] Use correct README link to render the README (#23152) (#23264) Backport #23152 `renderReadmeFile` needs `readmeTreelink` as parameter but gets `treeLink`. The values of them look like as following: `treeLink`: `/{OwnerName}/{RepoName}/src/branch/{BranchName}` `readmeTreelink`: `/{OwnerName}/{RepoName}/src/branch/{BranchName}/{ReadmeFileName}` `path.Dir` in https://github.com/go-gitea/gitea/blob/8540fc45b11eff9a73753ca139f8ea5c38509bf5/routers/web/repo/view.go#L316 should convert `readmeTreelink` into `/{OwnerName}/{RepoName}/src/branch/{BranchName}` instead of the current `/{OwnerName}/{RepoName}/src/branch`. Fixes #23151 Co-authored-by: yp05327 <576951401@qq.com> Co-authored-by: Jason Song Co-authored-by: John Olheiser Co-authored-by: silverwind --- models/fixtures/repository.yml | 2 +- routers/web/repo/view.go | 2 +- .../40/3d76c604cb569323864e06a07b85d466924802 | Bin 0 -> 68 bytes .../78/fb907e3a3309eae4fe8fef030874cebbf1cd5e | Bin 0 -> 158 bytes .../f3/fa0f5cc797fc4c02a1b8bec9de4b2072fcdbdf | Bin 0 -> 53 bytes .../repo1.git/refs/heads/home-md-img-check | 1 + tests/integration/repo_test.go | 20 ++++++++++++++++++ 7 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/objects/40/3d76c604cb569323864e06a07b85d466924802 create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/objects/78/fb907e3a3309eae4fe8fef030874cebbf1cd5e create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/objects/f3/fa0f5cc797fc4c02a1b8bec9de4b2072fcdbdf create mode 100644 tests/gitea-repositories-meta/user2/repo1.git/refs/heads/home-md-img-check diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml index 58f9b919acfbc..dd8facb7a3134 100644 --- a/models/fixtures/repository.yml +++ b/models/fixtures/repository.yml @@ -25,7 +25,7 @@ fork_id: 0 is_template: false template_id: 0 - size: 6708 + size: 7028 is_fsck_enabled: true close_issues_via_commit_in_any_branch: false diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index e3c61fa408bdb..8663e11382085 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -186,7 +186,7 @@ func renderDirectory(ctx *context.Context, treeLink string) { return } - renderReadmeFile(ctx, readmeFile, treeLink) + renderReadmeFile(ctx, readmeFile, fmt.Sprintf("%s/%s", treeLink, readmeFile.name)) } // localizedExtensions prepends the provided language code with and without a diff --git a/tests/gitea-repositories-meta/user2/repo1.git/objects/40/3d76c604cb569323864e06a07b85d466924802 b/tests/gitea-repositories-meta/user2/repo1.git/objects/40/3d76c604cb569323864e06a07b85d466924802 new file mode 100644 index 0000000000000000000000000000000000000000..ea0bf76d0cc85bb9d51720cae227d440a93a3388 GIT binary patch literal 68 zcmV-K0K5Nq0ZYosPf{?mU{F>lN-fAYEpczHPS& zC9f~@aEDl;W009N39E*sXI<5;4X;{IO4!g5>D~|jp1>4Yqlns~AsAelglXl4oEcm7 zp{8}D{+yrt0l#;or3hS3t5L3)uaC!@?b|zK77<1wfmdvZ>7?h|^#4_o MxNRTl4+U^QjwQiQZvX%Q literal 0 HcmV?d00001 diff --git a/tests/gitea-repositories-meta/user2/repo1.git/objects/f3/fa0f5cc797fc4c02a1b8bec9de4b2072fcdbdf b/tests/gitea-repositories-meta/user2/repo1.git/objects/f3/fa0f5cc797fc4c02a1b8bec9de4b2072fcdbdf new file mode 100644 index 0000000000000000000000000000000000000000..9b20f8af3f2d52f51ef5053a55a15ed946fda5a7 GIT binary patch literal 53 zcmb Date: Sat, 4 Mar 2023 00:38:31 -0500 Subject: [PATCH 2/3] Fix GetFilesChangedBetween if the file name may be escaped (#23272) (#23279) Backport #23272 The code for GetFilesChangedBetween uses `git diff --name-only base..head` to get the names of files changed between base and head however this forgets that git will escape certain values. This PR simply switches to use `-z` which has the `NUL` character as the separator. Ref https://github.com/go-gitea/gitea/pull/22568#discussion_r1123138096 Signed-off-by: Andrew Thornton Co-authored-by: zeripath Co-authored-by: techknowlogick --- modules/git/repo_compare.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/modules/git/repo_compare.go b/modules/git/repo_compare.go index 9a4d66f2f521b..439455e3c295c 100644 --- a/modules/git/repo_compare.go +++ b/modules/git/repo_compare.go @@ -277,11 +277,18 @@ func (repo *Repository) GetPatch(base, head string, w io.Writer) error { // GetFilesChangedBetween returns a list of all files that have been changed between the given commits func (repo *Repository) GetFilesChangedBetween(base, head string) ([]string, error) { - stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only").AddDynamicArguments(base + ".." + head).RunStdString(&RunOpts{Dir: repo.Path}) + stdout, _, err := NewCommand(repo.Ctx, "diff", "--name-only", "-z").AddDynamicArguments(base + ".." + head).RunStdString(&RunOpts{Dir: repo.Path}) if err != nil { return nil, err } - return strings.Split(stdout, "\n"), err + split := strings.Split(stdout, "\000") + + // Because Git will always emit filenames with a terminal NUL ignore the last entry in the split - which will always be empty. + if len(split) > 0 { + split = split[:len(split)-1] + } + + return split, err } // GetDiffFromMergeBase generates and return patch data from merge base to head From a4158d1904b04c4f4b499cf8b9505aca3bb70d11 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Sat, 4 Mar 2023 09:23:49 -0500 Subject: [PATCH 3/3] Avoid panic caused by broken payload when creating commit status (#23216) (#23294) Backport #23216 When creating commit status for Actons jobs, a payload with nil `HeadCommit` will cause panic. Reported at: https://gitea.com/gitea/act_runner/issues/28#issuecomment-732166 Although the `HeadCommit` probably can not be nil after #23215, `CreateCommitStatus` should protect itself, to avoid being broken in the future. In addition, it's enough to print error log instead of returning err when `CreateCommitStatus` failed. Co-authored-by: Jason Song Co-authored-by: delvh --- routers/api/actions/runner/runner.go | 2 +- routers/web/repo/actions/view.go | 22 +++++++++++++++------- services/actions/clear_tasks.go | 22 +++++++++++++++++----- services/actions/commit_status.go | 10 ++++++++++ services/actions/notifier_helper.go | 3 ++- 5 files changed, 45 insertions(+), 14 deletions(-) diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index d0bfb2363e0c5..839f776fe8de7 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -150,7 +150,7 @@ func (s *Service) UpdateTask( } if err := actions_service.CreateCommitStatus(ctx, task.Job); err != nil { - log.Error("Update commit status failed: %v", err) + log.Error("Update commit status for job %v failed: %v", task.Job.ID, err) // go on } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 5370310e8d9dc..86894ce55b119 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/actions" context_module "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -207,15 +208,18 @@ func Rerun(ctx *context_module.Context) { job.Stopped = 0 if err := db.WithTx(ctx, func(ctx context.Context) error { - if _, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": status}, "task_id", "status", "started", "stopped"); err != nil { - return err - } - return actions_service.CreateCommitStatus(ctx, job) + _, err := actions_model.UpdateRunJob(ctx, job, builder.Eq{"status": status}, "task_id", "status", "started", "stopped") + return err }); err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return } + if err := actions_service.CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + ctx.JSON(http.StatusOK, struct{}{}) } @@ -248,9 +252,6 @@ func Cancel(ctx *context_module.Context) { if err := actions_model.StopTask(ctx, job.TaskID, actions_model.StatusCancelled); err != nil { return err } - if err := actions_service.CreateCommitStatus(ctx, job); err != nil { - return err - } } return nil }); err != nil { @@ -258,6 +259,13 @@ func Cancel(ctx *context_module.Context) { return } + for _, job := range jobs { + if err := actions_service.CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + } + ctx.JSON(http.StatusOK, struct{}{}) } diff --git a/services/actions/clear_tasks.go b/services/actions/clear_tasks.go index 583e588de4054..6f8e95218d678 100644 --- a/services/actions/clear_tasks.go +++ b/services/actions/clear_tasks.go @@ -43,6 +43,7 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { return fmt.Errorf("find tasks: %w", err) } + jobs := make([]*actions_model.ActionRunJob, 0, len(tasks)) for _, task := range tasks { if err := db.WithTx(ctx, func(ctx context.Context) error { if err := actions_model.StopTask(ctx, task.ID, actions_model.StatusFailure); err != nil { @@ -51,7 +52,8 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { if err := task.LoadJob(ctx); err != nil { return err } - return CreateCommitStatus(ctx, task.Job) + jobs = append(jobs, task.Job) + return nil }); err != nil { log.Warn("Cannot stop task %v: %v", task.ID, err) // go on @@ -61,6 +63,14 @@ func stopTasks(ctx context.Context, opts actions_model.FindTaskOptions) error { remove() } } + + for _, job := range jobs { + if err := CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } + } + return nil } @@ -80,14 +90,16 @@ func CancelAbandonedJobs(ctx context.Context) error { job.Status = actions_model.StatusCancelled job.Stopped = now if err := db.WithTx(ctx, func(ctx context.Context) error { - if _, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped"); err != nil { - return err - } - return CreateCommitStatus(ctx, job) + _, err := actions_model.UpdateRunJob(ctx, job, nil, "status", "stopped") + return err }); err != nil { log.Warn("cancel abandoned job %v: %v", job.ID, err) // go on } + if err := CreateCommitStatus(ctx, job); err != nil { + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on + } } return nil diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index efb5ec6d44a3c..4f313493523ee 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -30,6 +30,16 @@ func CreateCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er return fmt.Errorf("GetPushEventPayload: %w", err) } + // Since the payload comes from json data, we should check if it's broken, or it will cause panic + switch { + case payload.Repo == nil: + return fmt.Errorf("repo is missing in event payload") + case payload.Pusher == nil: + return fmt.Errorf("pusher is missing in event payload") + case payload.HeadCommit == nil: + return fmt.Errorf("head commit is missing in event payload") + } + creator, err := user_model.GetUserByID(ctx, payload.Pusher.ID) if err != nil { return fmt.Errorf("GetUserByID: %w", err) diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index df67d2fa116da..f4f6a3aa52879 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -180,7 +180,8 @@ func notify(ctx context.Context, input *notifyInput) error { } else { for _, job := range jobs { if err := CreateCommitStatus(ctx, job); err != nil { - log.Error("CreateCommitStatus: %v", err) + log.Error("Update commit status for job %v failed: %v", job.ID, err) + // go on } } }