Skip to content

Commit 7cd4704

Browse files
zeripathguillep2k
zeripath
and
guillep2k
authored
Handle push rejection in branch and upload (#10854)
* Handle push rejections and push out-of-date in branch creation and file upload. * Remove the duplicated sanitize from services/pull/merge * Move the errors Err(Merge)PushOutOfDate and ErrPushRejected to modules/git * Handle errors better in the upload file dialogs Fix #10460 Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
1 parent cac30ab commit 7cd4704

File tree

12 files changed

+197
-120
lines changed

12 files changed

+197
-120
lines changed

‎models/error.go‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package models
77

88
import (
99
"fmt"
10-
"strings"
1110

1211
"code.gitea.io/gitea/modules/git"
1312
)
@@ -1388,71 +1387,6 @@ func (err ErrMergeUnrelatedHistories) Error() string {
13881387
return fmt.Sprintf("Merge UnrelatedHistories Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
13891388
}
13901389

1391-
// ErrMergePushOutOfDate represents an error if merging fails due to unrelated histories
1392-
type ErrMergePushOutOfDate struct {
1393-
Style MergeStyle
1394-
StdOut string
1395-
StdErr string
1396-
Err error
1397-
}
1398-
1399-
// IsErrMergePushOutOfDate checks if an error is a ErrMergePushOutOfDate.
1400-
func IsErrMergePushOutOfDate(err error) bool {
1401-
_, ok := err.(ErrMergePushOutOfDate)
1402-
return ok
1403-
}
1404-
1405-
func (err ErrMergePushOutOfDate) Error() string {
1406-
return fmt.Sprintf("Merge PushOutOfDate Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
1407-
}
1408-
1409-
// ErrPushRejected represents an error if merging fails due to rejection from a hook
1410-
type ErrPushRejected struct {
1411-
Style MergeStyle
1412-
Message string
1413-
StdOut string
1414-
StdErr string
1415-
Err error
1416-
}
1417-
1418-
// IsErrPushRejected checks if an error is a ErrPushRejected.
1419-
func IsErrPushRejected(err error) bool {
1420-
_, ok := err.(ErrPushRejected)
1421-
return ok
1422-
}
1423-
1424-
func (err ErrPushRejected) Error() string {
1425-
return fmt.Sprintf("Merge PushRejected Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
1426-
}
1427-
1428-
// GenerateMessage generates the remote message from the stderr
1429-
func (err *ErrPushRejected) GenerateMessage() {
1430-
messageBuilder := &strings.Builder{}
1431-
i := strings.Index(err.StdErr, "remote: ")
1432-
if i < 0 {
1433-
err.Message = ""
1434-
return
1435-
}
1436-
for {
1437-
if len(err.StdErr) <= i+8 {
1438-
break
1439-
}
1440-
if err.StdErr[i:i+8] != "remote: " {
1441-
break
1442-
}
1443-
i += 8
1444-
nl := strings.IndexByte(err.StdErr[i:], '\n')
1445-
if nl >= 0 {
1446-
messageBuilder.WriteString(err.StdErr[i : i+nl+1])
1447-
i = i + nl + 1
1448-
} else {
1449-
messageBuilder.WriteString(err.StdErr[i:])
1450-
i = len(err.StdErr)
1451-
}
1452-
}
1453-
err.Message = strings.TrimSpace(messageBuilder.String())
1454-
}
1455-
14561390
// ErrRebaseConflicts represents an error if rebase fails with a conflict
14571391
type ErrRebaseConflicts struct {
14581392
Style MergeStyle

‎modules/git/error.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package git
66

77
import (
88
"fmt"
9+
"strings"
910
"time"
1011
)
1112

@@ -85,3 +86,76 @@ func IsErrBranchNotExist(err error) bool {
8586
func (err ErrBranchNotExist) Error() string {
8687
return fmt.Sprintf("branch does not exist [name: %s]", err.Name)
8788
}
89+
90+
// ErrPushOutOfDate represents an error if merging fails due to unrelated histories
91+
type ErrPushOutOfDate struct {
92+
StdOut string
93+
StdErr string
94+
Err error
95+
}
96+
97+
// IsErrPushOutOfDate checks if an error is a ErrPushOutOfDate.
98+
func IsErrPushOutOfDate(err error) bool {
99+
_, ok := err.(*ErrPushOutOfDate)
100+
return ok
101+
}
102+
103+
func (err *ErrPushOutOfDate) Error() string {
104+
return fmt.Sprintf("PushOutOfDate Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
105+
}
106+
107+
// Unwrap unwraps the underlying error
108+
func (err *ErrPushOutOfDate) Unwrap() error {
109+
return fmt.Errorf("%v - %s", err.Err, err.StdErr)
110+
}
111+
112+
// ErrPushRejected represents an error if merging fails due to rejection from a hook
113+
type ErrPushRejected struct {
114+
Message string
115+
StdOut string
116+
StdErr string
117+
Err error
118+
}
119+
120+
// IsErrPushRejected checks if an error is a ErrPushRejected.
121+
func IsErrPushRejected(err error) bool {
122+
_, ok := err.(*ErrPushRejected)
123+
return ok
124+
}
125+
126+
func (err *ErrPushRejected) Error() string {
127+
return fmt.Sprintf("PushRejected Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
128+
}
129+
130+
// Unwrap unwraps the underlying error
131+
func (err *ErrPushRejected) Unwrap() error {
132+
return fmt.Errorf("%v - %s", err.Err, err.StdErr)
133+
}
134+
135+
// GenerateMessage generates the remote message from the stderr
136+
func (err *ErrPushRejected) GenerateMessage() {
137+
messageBuilder := &strings.Builder{}
138+
i := strings.Index(err.StdErr, "remote: ")
139+
if i < 0 {
140+
err.Message = ""
141+
return
142+
}
143+
for {
144+
if len(err.StdErr) <= i+8 {
145+
break
146+
}
147+
if err.StdErr[i:i+8] != "remote: " {
148+
break
149+
}
150+
i += 8
151+
nl := strings.IndexByte(err.StdErr[i:], '\n')
152+
if nl >= 0 {
153+
messageBuilder.WriteString(err.StdErr[i : i+nl+1])
154+
i = i + nl + 1
155+
} else {
156+
messageBuilder.WriteString(err.StdErr[i:])
157+
i = len(err.StdErr)
158+
}
159+
}
160+
err.Message = strings.TrimSpace(messageBuilder.String())
161+
}

‎modules/git/repo.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,31 @@ func Push(repoPath string, opts PushOptions) error {
255255
cmd.AddArguments("-f")
256256
}
257257
cmd.AddArguments("--", opts.Remote, opts.Branch)
258-
_, err := cmd.RunInDirWithEnv(repoPath, opts.Env)
258+
var outbuf, errbuf strings.Builder
259+
260+
err := cmd.RunInDirTimeoutEnvPipeline(opts.Env, -1, repoPath, &outbuf, &errbuf)
261+
if err != nil {
262+
if strings.Contains(errbuf.String(), "non-fast-forward") {
263+
return &ErrPushOutOfDate{
264+
StdOut: outbuf.String(),
265+
StdErr: errbuf.String(),
266+
Err: err,
267+
}
268+
} else if strings.Contains(errbuf.String(), "! [remote rejected]") {
269+
err := &ErrPushRejected{
270+
StdOut: outbuf.String(),
271+
StdErr: errbuf.String(),
272+
Err: err,
273+
}
274+
err.GenerateMessage()
275+
return err
276+
}
277+
}
278+
279+
if errbuf.Len() > 0 && err != nil {
280+
return fmt.Errorf("%v - %s", err, errbuf.String())
281+
}
282+
259283
return err
260284
}
261285

‎modules/repofiles/temp_repo.go‎

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -242,30 +242,21 @@ func (t *TemporaryUploadRepository) CommitTreeWithDate(author, committer *models
242242
func (t *TemporaryUploadRepository) Push(doer *models.User, commitHash string, branch string) error {
243243
// Because calls hooks we need to pass in the environment
244244
env := models.PushingEnvironment(doer, t.repo)
245-
stdout := &strings.Builder{}
246-
stderr := &strings.Builder{}
247-
248-
if err := git.NewCommand("push", t.repo.RepoPath(), strings.TrimSpace(commitHash)+":refs/heads/"+strings.TrimSpace(branch)).RunInDirTimeoutEnvPipeline(env, -1, t.basePath, stdout, stderr); err != nil {
249-
errString := stderr.String()
250-
if strings.Contains(errString, "non-fast-forward") {
251-
return models.ErrMergePushOutOfDate{
252-
StdOut: stdout.String(),
253-
StdErr: errString,
254-
Err: err,
255-
}
256-
} else if strings.Contains(errString, "! [remote rejected]") {
257-
log.Error("Unable to push back to repo from temporary repo due to rejection: %s (%s)\nStdout: %s\nStderr: %s\nError: %v",
258-
t.repo.FullName(), t.basePath, stdout, errString, err)
259-
err := models.ErrPushRejected{
260-
StdOut: stdout.String(),
261-
StdErr: errString,
262-
Err: err,
263-
}
264-
err.GenerateMessage()
245+
if err := git.Push(t.basePath, git.PushOptions{
246+
Remote: t.repo.RepoPath(),
247+
Branch: strings.TrimSpace(commitHash) + ":refs/heads/" + strings.TrimSpace(branch),
248+
Env: env,
249+
}); err != nil {
250+
if git.IsErrPushOutOfDate(err) {
251+
return err
252+
} else if git.IsErrPushRejected(err) {
253+
rejectErr := err.(*git.ErrPushRejected)
254+
log.Info("Unable to push back to repo from temporary repo due to rejection: %s (%s)\nStdout: %s\nStderr: %s\nError: %v",
255+
t.repo.FullName(), t.basePath, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err)
265256
return err
266257
}
267-
log.Error("Unable to push back to repo from temporary repo: %s (%s)\nStdout: %s\nError: %v",
268-
t.repo.FullName(), t.basePath, stdout, err)
258+
log.Error("Unable to push back to repo from temporary repo: %s (%s)\nError: %v",
259+
t.repo.FullName(), t.basePath, err)
269260
return fmt.Errorf("Unable to push back to repo from temporary repo: %s (%s) Error: %v",
270261
t.repo.FullName(), t.basePath, err)
271262
}

‎modules/repofiles/update.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ func CreateOrUpdateRepoFile(repo *models.Repository, doer *models.User, opts *Up
446446

447447
// Then push this tree to NewBranch
448448
if err := t.Push(doer, commitHash, opts.NewBranch); err != nil {
449+
log.Error("%T %v", err, err)
449450
return nil, err
450451
}
451452

‎modules/repository/branch.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ func CreateNewBranch(doer *models.User, repo *models.Repository, oldBranchName,
109109
Branch: branchName,
110110
Env: models.PushingEnvironment(doer, repo),
111111
}); err != nil {
112+
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
113+
return err
114+
}
112115
return fmt.Errorf("Push: %v", err)
113116
}
114117

@@ -156,6 +159,9 @@ func CreateNewBranchFromCommit(doer *models.User, repo *models.Repository, commi
156159
Branch: branchName,
157160
Env: models.PushingEnvironment(doer, repo),
158161
}); err != nil {
162+
if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
163+
return err
164+
}
159165
return fmt.Errorf("Push: %v", err)
160166
}
161167

‎routers/api/v1/repo/pull.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,11 +678,11 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
678678
} else if models.IsErrMergeUnrelatedHistories(err) {
679679
conflictError := err.(models.ErrMergeUnrelatedHistories)
680680
ctx.JSON(http.StatusConflict, conflictError)
681-
} else if models.IsErrMergePushOutOfDate(err) {
681+
} else if git.IsErrPushOutOfDate(err) {
682682
ctx.Error(http.StatusConflict, "Merge", "merge push out of date")
683683
return
684-
} else if models.IsErrPushRejected(err) {
685-
errPushRej := err.(models.ErrPushRejected)
684+
} else if git.IsErrPushRejected(err) {
685+
errPushRej := err.(*git.ErrPushRejected)
686686
if len(errPushRej.Message) == 0 {
687687
ctx.Error(http.StatusConflict, "Merge", "PushRejected without remote error message")
688688
return

‎routers/repo/branch.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"code.gitea.io/gitea/modules/repofiles"
1818
repo_module "code.gitea.io/gitea/modules/repository"
1919
"code.gitea.io/gitea/modules/util"
20+
"code.gitea.io/gitea/routers/utils"
2021
)
2122

2223
const (
@@ -335,9 +336,8 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
335336
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
336337
return
337338
}
338-
if models.IsErrBranchAlreadyExists(err) {
339-
e := err.(models.ErrBranchAlreadyExists)
340-
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", e.BranchName))
339+
if models.IsErrBranchAlreadyExists(err) || git.IsErrPushOutOfDate(err) {
340+
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", form.NewBranchName))
341341
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
342342
return
343343
}
@@ -347,6 +347,16 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
347347
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
348348
return
349349
}
350+
if git.IsErrPushRejected(err) {
351+
e := err.(*git.ErrPushRejected)
352+
if len(e.Message) == 0 {
353+
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected_no_message"))
354+
} else {
355+
ctx.Flash.Error(ctx.Tr("repo.editor.push_rejected", utils.SanitizeFlashErrorString(e.Message)))
356+
}
357+
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
358+
return
359+
}
350360

351361
ctx.ServerError("CreateNewBranch", err)
352362
return

0 commit comments

Comments
 (0)