ynkmr/gitea
1
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2026-09-21 12:13:38 +09:00
Code Issues Projects Releases Wiki Activity

fix: pass merge commit messages to git via stdin (#39269)

Browse Source 
`git commit --message=` passes the merge message as a single argument,
which Linux caps at 128 KiB and Windows at 32 KiB for the whole command
line. Long messages failed with `argument list too long` and the merge
box toast showed the raw HTML 500 page.

Pass the message via `--file=-` on stdin instead, and answer
fetch-action requests with JSON on server errors so the toast shows the
error text. Limits merge commit messages to 512KB which could be
extended or made configurable later.

Fixes: https://github.com/go-gitea/gitea/issues/39261
Fixes: https://github.com/go-gitea/gitea/issues/30276
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
silverwind
2026-09-19 13:04:53 +00:00
committed by GitHub
co-authored by wxiaoguang
parent cc34c26172
commit cdf786ce92
21 changed files with 175 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files
services/context/base.go
+1 -1
View File
@@ -161,7 +161,7 @@ func (b *Base) Redirect(location string, status ...int) {
}
// In case the request is made by "fetch-action" module, make JS redirect to the new location
// Otherwise, the JS fetch will follow the redirection and read a "login" page, embed it to the current page, which is not expected.
if b.Req.Header.Get("X-Gitea-Fetch-Action") != "" {
if httplib.IsGiteaFetchActionRequest(b.Req) {
b.JSON(http.StatusOK, map[string]any{"redirect": location})
return
}
services/context/base_test.go
+6 -1
View File
@@ -6,6 +6,7 @@ package context
import (
"net/http"
"net/http/httptest"
"os"
"testing"
"gitea.dev/modules/setting"
@@ -13,8 +14,12 @@ import (
"github.com/stretchr/testify/assert"
)
func TestRedirect(t *testing.T) {
func TestMain(m *testing.M) {
setting.IsInTesting = true
os.Exit(m.Run())
}
func TestRedirect(t *testing.T) {
req, _ := http.NewRequest(http.MethodGet, "/", nil)
cases := []struct {
services/context/context.go
+6 -2
View File
@@ -270,8 +270,12 @@ func (ctx *Context) JSONErrorAuto(err error) {
ctx.JSON(httpCode, buildJsonErrorMap(errMsg))
return
}
log.ErrorWithSkip(1, "JSONErrorAuto: server internal error: %v", err)
ctx.JSON(http.StatusInternalServerError, buildJsonErrorMap(ctx.Locale.TrString("error.occurred")))
logLevel := util.Iif(httplib.IsClientOrNetworkError(ctx, err), log.DEBUG, log.ERROR)
log.Log(1, logLevel, "JSONErrorAuto: server internal error: %v", err)
userErrorMsg := ctx.buildUserErrorMessage("internal server error", err)
ctx.JSON(http.StatusInternalServerError, buildJsonErrorMap(userErrorMsg))
}
func (ctx *Context) JSONError[T string | template.HTML](msg T) {
services/context/context_response.go
+28 -21
View File
@@ -7,13 +7,11 @@ import (
"errors"
"fmt"
"html/template"
"net"
"net/http"
"net/url"
"path"
"strconv"
"strings"
"syscall"
"time"
user_model "gitea.dev/models/user"
@@ -90,7 +88,7 @@ func (ctx *Context) HTML(status int, name templates.TplName) {
}
err := ctx.Render.HTML(ctx.Resp, status, name, ctx.Data, ctx.TemplateContext)
if err == nil || errors.Is(err, syscall.EPIPE) {
if err == nil || httplib.IsClientOrNetworkError(ctx, err) {
return
}
@@ -126,13 +124,13 @@ func (ctx *Context) RenderWithErrDeprecated(msg any, tpl templates.TplName, form
// NotFound displays a 404 (Not Found) page and prints the given error, if any.
func (ctx *Context) NotFound(logErr error) {
ctx.notFoundInternal("", logErr)
ctx.notFoundInternal(1, "", logErr)
}
func (ctx *Context) notFoundInternal(logMsg string, logErr error) {
func (ctx *Context) notFoundInternal(skip int, logMsg string, logErr error) {
// TODO: it's safe to show the error message to end users if the error is fully controlled by our error system
if logErr != nil {
log.Log(2, log.DEBUG, "%s: %v", logMsg, logErr)
log.Log(skip+1, log.DEBUG, "%s: %v", logMsg, logErr)
}
// response simple message if Accept isn't text/html
@@ -155,32 +153,41 @@ func (ctx *Context) notFoundInternal(logMsg string, logErr error) {
ctx.HTML(http.StatusNotFound, "status/404")
}
func (ctx *Context) buildUserErrorMessage(msg string, err error) (userErrorMsg string) {
// it's safe to show internal error to admin users, and it helps
if !setting.IsProd || (ctx.Doer != nil && ctx.Doer.IsAdmin) {
userErrorMsg = msg
if err != nil {
userErrorMsg += ", error: " + err.Error()
}
}
return util.IfZero(userErrorMsg, ctx.Locale.TrString("error.occurred"))
}
// ServerError displays a 500 (Internal Server Error) page and prints the given error, if any.
// If the error is controlled by our error system, a related 404 page can be displayed instead.
func (ctx *Context) ServerError(logMsg string, logErr error) {
if errors.Is(logErr, util.ErrNotExist) {
ctx.notFoundInternal(logMsg, logErr)
ctx.notFoundInternal(1, logMsg, logErr)
return
}
ctx.serverErrorInternal(logMsg, logErr)
ctx.serverErrorInternal(1, logMsg, logErr)
}
func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
func (ctx *Context) serverErrorInternal(skip int, logMsg string, logErr error) {
if logErr != nil {
log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) {
// This is an error within the underlying connection
// and further rendering will not work so just return
return
}
logLevel := util.Iif(httplib.IsClientOrNetworkError(ctx, logErr), log.DEBUG, log.ERROR)
log.Log(skip+1, logLevel, "%s: %v", logMsg, logErr)
}
// it's safe to show internal error to admin users, and it helps
if !setting.IsProd || (ctx.Doer != nil && ctx.Doer.IsAdmin) {
ctx.Data["ErrorMsg"] = fmt.Sprintf("%s, %s", logMsg, logErr)
}
userErrorMsg := ctx.buildUserErrorMessage(logMsg, logErr)
if httplib.IsGiteaFetchActionRequest(ctx.Req) {
ctx.JSON(http.StatusInternalServerError, buildJsonErrorMap(userErrorMsg))
return
}
ctx.Data["Title"] = "Internal Server Error"
ctx.Data["ErrorMsg"] = userErrorMsg
ctx.HTML(http.StatusInternalServerError, tplStatus500)
}
@@ -190,8 +197,8 @@ func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
// TODO: remove the "errCheck" and use util.ErrNotFound to check
func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, logErr error) {
if errCheck(logErr) {
ctx.notFoundInternal(logMsg, logErr)
ctx.notFoundInternal(1, logMsg, logErr)
return
}
ctx.serverErrorInternal(logMsg, logErr)
ctx.serverErrorInternal(1, logMsg, logErr)
}
services/context/context_test.go
+12 -2
View File
@@ -4,6 +4,7 @@
package context
import (
"errors"
"net/http"
"net/http/httptest"
"net/url"
@@ -27,8 +28,18 @@ func TestRemoveSessionCookieHeader(t *testing.T) {
assert.Contains(t, "other=bar", w.Header().Get("Set-Cookie"))
}
func TestServerErrorFetchActionRespondsJSON(t *testing.T) {
req, _ := http.NewRequest(http.MethodPost, "/", nil)
req.Header.Add("X-Gitea-Fetch-Action", "1")
resp := httptest.NewRecorder()
ctx := NewWebContext(NewBaseContextForTest(t, resp, req), nil, nil)
ctx.ServerError("test", errors.New("boom"))
assert.Equal(t, http.StatusInternalServerError, resp.Code)
assert.Contains(t, resp.Header().Get("Content-Type"), "application/json")
assert.JSONEq(t, `{"errorMessage":"test, error: boom","renderFormat":"text"}`, resp.Body.String())
}
func TestRedirectToCurrentSite(t *testing.T) {
setting.IsInTesting = true
defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
cases := []struct {
@@ -53,7 +64,6 @@ func TestRedirectToCurrentSite(t *testing.T) {
}
func TestAppFullLink(t *testing.T) {
setting.IsInTesting = true
defer test.MockVariableValue(&setting.AppURL, "https://gitea.example.com/sub/")()
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
defer test.MockVariableValue(&setting.PublicURLDetection, setting.PublicURLNever)()
services/pull/merge.go
+4 -1
View File
@@ -460,7 +460,10 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
}
func commitAndSignNoAuthor(ctx *mergeContext, message string) error {
cmdCommit := gitcmd.NewCommand("commit").AddOptionFormat("--message=%s", message)
cmdCommit := gitcmd.NewCommand("commit")
if err := git.AddObjectMessageArgument(cmdCommit, git.ObjectCommit, message); err != nil {
return err
}
addCommitSigningOptions(cmdCommit, ctx.signKey)
if err := ctx.PrepareGitCmd(cmdCommit).RunWithStderr(ctx); err != nil {
return fmt.Errorf("git commit %v: %w\n%s", ctx.pr, err, ctx.outbuf.String())
services/pull/merge_rebase.go
+4 -2
View File
@@ -73,8 +73,10 @@ func doMergeRebaseFastForward(ctx *mergeContext) error {
}
if newMessage != "" {
cmdCommit := gitcmd.NewCommand("commit", "--amend").
AddOptionFormat("--message=%s", newMessage)
cmdCommit := gitcmd.NewCommand("commit", "--amend")
if err = git.AddObjectMessageArgument(cmdCommit, git.ObjectCommit, newMessage); err != nil {
return err
}
addCommitSigningOptions(cmdCommit, ctx.signKey)
if err := cmdCommit.WithRepo(ctx.tmpRepo).Run(ctx); err != nil {
log.Error("Unable to amend commit message: %v", err)
services/pull/merge_squash.go
+5 -4
View File
@@ -67,10 +67,11 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error {
if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() {
message = AddCommitMessageTailer(message, git.CoAuthoredByTrailer, sig.String())
}
cmdCommit := gitcmd.NewCommand("commit").
AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email).
AddOptionFormat("--message=%s", message).
AddArguments("--allow-empty")
cmdCommit := gitcmd.NewCommand("commit", "--allow-empty").
AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email)
if err = git.AddObjectMessageArgument(cmdCommit, git.ObjectCommit, message); err != nil {
return err
}
addCommitSigningOptions(cmdCommit, ctx.signKey)
if err := ctx.PrepareGitCmd(cmdCommit).RunWithStderr(ctx); err != nil {
log.Error("git commit %-v: %v\n%s\n%s", ctx.pr, err, ctx.outbuf.String(), err.Stderr())