From c52f81eecc3e1ad4a4fbf2acba6cae4ffd08a247 Mon Sep 17 00:00:00 2001
From: zeripath <art27@cantab.net>
Date: Mon, 8 Jun 2020 20:00:12 +0100
Subject: [PATCH] Ensure rejected push to refs/pull/index/head fails nicely
 (#11724) (#11809)

Backport #11724

A pre-receive hook that rejects pushes to refs/pull/index/head
will cause a broken PR which causes an internal server error
whenever it is viewed. This PR handles prevents the internal server
error by handling non-existent pr heads and sends a flash error
informing the creator there was a problem.

Signed-off-by: Andrew Thornton <art27@cantab.net>
---
 routers/repo/pull.go  | 32 +++++++++++++++++++++++++++-----
 services/pull/pull.go | 10 ++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index d23c93d0b..ec82d14f1 100644
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -428,6 +428,20 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 
 	sha, err := baseGitRepo.GetRefCommitID(pull.GetGitRefName())
 	if err != nil {
+		if git.IsErrNotExist(err) {
+			ctx.Data["IsPullRequestBroken"] = true
+			if pull.IsSameRepo() {
+				ctx.Data["HeadTarget"] = pull.HeadBranch
+			} else if pull.HeadRepo == nil {
+				ctx.Data["HeadTarget"] = "<deleted>:" + pull.HeadBranch
+			} else {
+				ctx.Data["HeadTarget"] = pull.HeadRepo.OwnerName + ":" + pull.HeadBranch
+			}
+			ctx.Data["BaseTarget"] = pull.BaseBranch
+			ctx.Data["NumCommits"] = 0
+			ctx.Data["NumFiles"] = 0
+			return nil
+		}
 		ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
 		return nil
 	}
@@ -462,12 +476,10 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 		ctx.Data["IsPullRequestBroken"] = true
 		if pull.IsSameRepo() {
 			ctx.Data["HeadTarget"] = pull.HeadBranch
+		} else if pull.HeadRepo == nil {
+			ctx.Data["HeadTarget"] = "<deleted>:" + pull.HeadBranch
 		} else {
-			if pull.HeadRepo == nil {
-				ctx.Data["HeadTarget"] = "<deleted>:" + pull.HeadBranch
-			} else {
-				ctx.Data["HeadTarget"] = pull.HeadRepo.OwnerName + ":" + pull.HeadBranch
-			}
+			ctx.Data["HeadTarget"] = pull.HeadRepo.OwnerName + ":" + pull.HeadBranch
 		}
 	}
 
@@ -950,6 +962,16 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 		if models.IsErrUserDoesNotHaveAccessToRepo(err) {
 			ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error())
 			return
+		} else if git.IsErrPushRejected(err) {
+			pushrejErr := err.(*git.ErrPushRejected)
+			message := pushrejErr.Message
+			if len(message) == 0 {
+				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected_no_message"))
+			} else {
+				ctx.Flash.Error(ctx.Tr("repo.pulls.push_rejected", utils.SanitizeFlashErrorString(pushrejErr.Message)))
+			}
+			ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pullIssue.Index))
+			return
 		}
 		ctx.ServerError("NewPullRequest", err)
 		return
diff --git a/services/pull/pull.go b/services/pull/pull.go
index fb4af0637..aeb0c009f 100644
--- a/services/pull/pull.go
+++ b/services/pull/pull.go
@@ -399,6 +399,16 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) {
 		// Use InternalPushingEnvironment here because we know that pre-receive and post-receive do not run on a refs/pulls/...
 		Env: models.InternalPushingEnvironment(pr.Issue.Poster, pr.BaseRepo),
 	}); err != nil {
+		if git.IsErrPushOutOfDate(err) {
+			// This should not happen as we're using force!
+			log.Error("Unable to push PR head for %s#%d (%-v:%s) due to ErrPushOfDate: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, err)
+			return err
+		} else if git.IsErrPushRejected(err) {
+			rejectErr := err.(*git.ErrPushRejected)
+			log.Info("Unable to push PR head for %s#%d (%-v:%s) due to rejection:\nStdout: %s\nStderr: %s\nError: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, rejectErr.StdOut, rejectErr.StdErr, rejectErr.Err)
+			return err
+		}
+		log.Error("Unable to push PR head for %s#%d (%-v:%s) due to Error: %v", pr.BaseRepo.FullName(), pr.Index, pr.BaseRepo, headFile, err)
 		return fmt.Errorf("Push: %s:%s %s:%s %v", pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseRepo.FullName(), headFile, err)
 	}