Merge branch 'main' into feature/runner-logs-api-endpoint

Author: rossigee

cmd/helper.go

@@ -134,7 +134,7 @@ func PrepareConsoleLoggerLevel(defaultLevel log.Level) func(context.Context, *cl
 		if globalBool(c, "debug") || globalBool(c, "verbose") {
 			level = log.TRACE
 		}
-		log.SetConsoleLogger(log.DEFAULT, "console-default", level)
+		log.SetupStderrLogger(log.DEFAULT, "console-stderr", level)
 		return ctx, nil
 	}
 }

modules/git/catfile_batch_command.go

@@ -7,8 +7,10 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"code.gitea.io/gitea/modules/git/gitcmd"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
@@ -39,6 +41,9 @@ func (b *catFileBatchCommand) getBatch() *catFileBatchCommunicator {
 }
 
 func (b *catFileBatchCommand) QueryContent(obj string) (*CatFileObject, BufferedReader, error) {
+	if strings.Contains(obj, "\n") {
+		setting.PanicInDevOrTesting("invalid object name with newline: %q", obj)
+	}
 	_, err := b.getBatch().reqWriter.Write([]byte("contents " + obj + "\n"))
 	if err != nil {
 		return nil, nil, err
@@ -51,6 +56,9 @@ func (b *catFileBatchCommand) QueryContent(obj string) (*CatFileObject, Buffered
 }
 
 func (b *catFileBatchCommand) QueryInfo(obj string) (*CatFileObject, error) {
+	if strings.Contains(obj, "\n") {
+		setting.PanicInDevOrTesting("invalid object name with newline: %q", obj)
+	}
 	_, err := b.getBatch().reqWriter.Write([]byte("info " + obj + "\n"))
 	if err != nil {
 		return nil, err

modules/git/catfile_batch_legacy.go

@@ -8,8 +8,10 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"code.gitea.io/gitea/modules/git/gitcmd"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 )
 
@@ -50,6 +52,9 @@ func (b *catFileBatchLegacy) getBatchCheck() *catFileBatchCommunicator {
 }
 
 func (b *catFileBatchLegacy) QueryContent(obj string) (*CatFileObject, BufferedReader, error) {
+	if strings.Contains(obj, "\n") {
+		setting.PanicInDevOrTesting("invalid object name with newline: %q", obj)
+	}
 	_, err := io.WriteString(b.getBatchContent().reqWriter, obj+"\n")
 	if err != nil {
 		return nil, nil, err
@@ -62,6 +67,9 @@ func (b *catFileBatchLegacy) QueryContent(obj string) (*CatFileObject, BufferedR
 }
 
 func (b *catFileBatchLegacy) QueryInfo(obj string) (*CatFileObject, error) {
+	if strings.Contains(obj, "\n") {
+		setting.PanicInDevOrTesting("invalid object name with newline: %q", obj)
+	}
 	_, err := io.WriteString(b.getBatchCheck().reqWriter, obj+"\n")
 	if err != nil {
 		return nil, err

modules/log/logger_global.go

@@ -75,12 +75,23 @@ func IsLoggerEnabled(name string) bool {
 	return GetManager().GetLogger(name).IsEnabled()
 }
 
-func SetConsoleLogger(loggerName, writerName string, level Level) {
+func SetupStderrLogger(loggerName, writerName string, level Level) {
 	writer := NewEventWriterConsole(writerName, WriterMode{
-		Level:        level,
-		Flags:        FlagsFromBits(LstdFlags),
-		Colorize:     CanColorStdout,
-		WriterOption: WriterConsoleOption{},
+		Level:    level,
+		Flags:    FlagsFromBits(LstdFlags),
+		Colorize: CanColorStderr,
+
+		// For most CLI commands, it's better to use Stderr as log output:
+		// this logger is installed early (app.Before), before subcommands like "dump" redirect logging to stderr.
+		// If Stdout, early log output (e.g.: warning during config loading) goes to stdout
+		// and corrupts any command that writes data to stdout (e.g. "gitea dump --file -").
+		//
+		// It is inconsistent with the web server's default console logger from config
+		// (which will be initialized later and use Stdout by default), but there is no other way at the moment:
+		// many existing users depend on such behavior to collect web logs (e.g. fail2ban).
+		//
+		// Maybe need to refactor the logger system again in the future.
+		WriterOption: WriterConsoleOption{Stderr: true},
 	})
 	GetManager().GetLogger(loggerName).ReplaceAllWriters(writer)
 }

modules/setting/log.go

@@ -256,7 +256,7 @@ func initLoggerByName(manager *log.LoggerManager, rootCfg ConfigProvider, logger
 }
 
 func InitSQLLoggersForCli(level log.Level) {
-	log.SetConsoleLogger("xorm", "console", level)
+	log.SetupStderrLogger("xorm", "console-stderr", level)
 }
 
 func IsAccessLogEnabled() bool {

modules/setting/setting.go

@@ -41,9 +41,9 @@ func init() {
 		AppVer = "dev"
 	}
 
-	// We can rely on log.CanColorStdout being set properly because modules/log/console_windows.go comes before modules/setting/setting.go lexicographically
+	// FIXME: the logger shouldn't be initialized here, the app entry should initialize the logger
 	// By default set this logger at Info - we'll change it later, but we need to start with something.
-	log.SetConsoleLogger(log.DEFAULT, "console", log.INFO)
+	log.SetupStderrLogger(log.DEFAULT, "console-stderr", log.INFO)
 }
 
 // IsRunUserMatchCurrentUser returns false if configured run user does not match

routers/web/repo/setting/protected_branch.go

@@ -20,6 +20,7 @@ import (
 	"code.gitea.io/gitea/models/unit"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/glob"
+	"code.gitea.io/gitea/modules/json"
 	"code.gitea.io/gitea/modules/templates"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/web/repo"
@@ -312,10 +313,14 @@ func DeleteProtectedBranchRulePost(ctx *context.Context) {
 }
 
 func UpdateBranchProtectionPriories(ctx *context.Context) {
-	form := web.GetForm(ctx).(*forms.ProtectBranchPriorityForm)
-	repo := ctx.Repo.Repository
-
-	if err := git_model.UpdateProtectBranchPriorities(ctx, repo, form.IDs); err != nil {
+	var form struct {
+		IDs []int64 `json:"ids"`
+	}
+	if err := json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil {
+		ctx.JSONError("invalid argument")
+		return
+	}
+	if err := git_model.UpdateProtectBranchPriorities(ctx, ctx.Repo.Repository, form.IDs); err != nil {
 		ctx.ServerError("UpdateProtectBranchPriorities", err)
 		return
 	}

routers/web/web.go

@@ -1175,7 +1175,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) {
 			m.Combo("/edit").Get(repo_setting.SettingsProtectedBranch).
 				Post(web.Bind(forms.ProtectBranchForm{}), context.RepoMustNotBeArchived(), repo_setting.SettingsProtectedBranchPost)
 			m.Post("/{id}/delete", repo_setting.DeleteProtectedBranchRulePost)
-			m.Post("/priority", web.Bind(forms.ProtectBranchPriorityForm{}), context.RepoMustNotBeArchived(), repo_setting.UpdateBranchProtectionPriories)
+			m.Post("/priority", context.RepoMustNotBeArchived(), repo_setting.UpdateBranchProtectionPriories)
 		})
 
 		m.Group("/tags", func() {

services/forms/repo_form.go

@@ -202,10 +202,6 @@ func (f *ProtectBranchForm) Validate(req *http.Request, errs binding.Errors) bin
 	return middleware.Validate(errs, ctx.Data, f, ctx.Locale)
 }
 
-type ProtectBranchPriorityForm struct {
-	IDs []int64
-}
-
 // WebhookForm form for changing web hook
 type WebhookForm struct {
 	Name                     string `binding:"MaxSize(255)"`

services/pull/check.go

@@ -339,18 +339,25 @@ func getMergeCommit(ctx context.Context, pr *issues_model.PullRequest) (*git.Com
 
 	objectFormat := git.ObjectFormatFromName(pr.BaseRepo.ObjectFormatName)
 
-	// Get the commit from BaseBranch where the pull request got merged
+	// Get the commit from BaseBranch where the pull request got merged.
+	// When several PRs targeting the same base are merged in a single push,
+	// rev-list returns one line per merge commit on the ancestry path; we
+	// only want the first one (the oldest, with --reverse, i.e. the merge
+	// commit that actually introduced this PR).
 	mergeCommit, _, err := gitrepo.RunCmdString(ctx, pr.BaseRepo,
 		gitcmd.NewCommand("rev-list", "--ancestry-path", "--merges", "--reverse").
 			AddDynamicArguments(prHeadCommitID+".."+pr.BaseBranch))
 	if err != nil {
 		return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %w", err)
-	} else if len(mergeCommit) < objectFormat.FullLength() {
+	}
+
+	// only use the latest commit as merge commit if the output contains multiple commits
+	mergeCommit = strings.TrimSpace(mergeCommit)
+	mergeCommit, _, _ = strings.Cut(mergeCommit, "\n")
+	if len(mergeCommit) < objectFormat.FullLength() {
 		// PR was maybe fast-forwarded, so just use last commit of PR
 		mergeCommit = prHeadCommitID
 	}
-	mergeCommit = strings.TrimSpace(mergeCommit)
-
 	commit, err := gitRepo.GetCommit(mergeCommit)
 	if err != nil {
 		return nil, fmt.Errorf("GetMergeCommit[%s]: %w", mergeCommit, err)