From 71fead5b71503387bedd4deee498bf73f0f13270 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 00:40:20 +0200 Subject: [PATCH 01/14] improve code quality --- modules/git/repo_stats.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index c0c91c6fc618..fb294f5a29dd 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -63,7 +63,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) if len(branch) == 0 { args = append(args, "--branches=*") } else { - args = append(args, "--first-parent", branch) + args = append(args, "--first-parent", "--", branch) } stderr := new(strings.Builder) From 9a14836cf5649d31db8fa71658f4476c7d4e76ac Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:12:46 +0200 Subject: [PATCH 02/14] make searchCommits more explicite --- modules/git/repo_commit.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index e6fec4d1a32e..a3ed0a0f91cc 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -103,23 +103,23 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add authors if present in search query if len(opts.Authors) > 0 { for _, v := range opts.Authors { - args = append(args, "--author="+v) + args = append(args, "--author='"+v+"'") } } // add committers if present in search query if len(opts.Committers) > 0 { for _, v := range opts.Committers { - args = append(args, "--committer="+v) + args = append(args, "--committer='"+v+"'") } } // add time constraints if present in search query if len(opts.After) > 0 { - args = append(args, "--after="+opts.After) + args = append(args, "--after='"+opts.After+"'") } if len(opts.Before) > 0 { - args = append(args, "--before="+opts.Before) + args = append(args, "--before='"+opts.Before+"'") } // pretend that all refs along with HEAD were listed on command line as @@ -133,7 +133,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // note this is done only for command created above if len(opts.Keywords) > 0 { for _, v := range opts.Keywords { - cmd.AddArguments("--grep=" + v) + cmd.AddArguments("--grep='" + v + "'") } } @@ -158,7 +158,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments(v) + hashCmd.AddArguments("--end-of-options", "'"+v+"'") // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) From 79edb072907e9aac4c25251a2a7ab5c4d8b40cc4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:36:18 +0200 Subject: [PATCH 03/14] reorder stuff --- modules/git/repo_commit.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index a3ed0a0f91cc..14678aae8e7d 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -208,9 +208,10 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( }() go func() { stderr := strings.Builder{} - err := NewCommand(repo.Ctx, "log", revision, "--follow", + err := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - prettyLogFormat, "--", file). + "--end-of-options", revision, + "--", file). Run(&RunOpts{ Dir: repo.Path, Stdout: stdoutWriter, From 58810b209ba31d44a457cfba77246a45712c4dd1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 01:58:19 +0200 Subject: [PATCH 04/14] some more trimming --- modules/git/commit.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 82b5e0b25d08..31f0a9ebd78d 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -250,15 +250,15 @@ func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommits for _, k := range fields { switch { case strings.HasPrefix(k, "author:"): - authors = append(authors, strings.TrimPrefix(k, "author:")) + authors = append(authors, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "author:")), "'", "")) case strings.HasPrefix(k, "committer:"): - committers = append(committers, strings.TrimPrefix(k, "committer:")) + committers = append(committers, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "committer:")), "'", "")) case strings.HasPrefix(k, "after:"): - after = strings.TrimPrefix(k, "after:") + after = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "after:")), "'", "") case strings.HasPrefix(k, "before:"): - before = strings.TrimPrefix(k, "before:") + before = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "before:")), "'", "") default: - keywords = append(keywords, k) + keywords = append(keywords, strings.ReplaceAll(strings.TrimSpace(k), "'", "")) } } From ecf032512d5132d66117b4755590ca84be363dd0 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:04:21 +0200 Subject: [PATCH 05/14] nit --- modules/git/repo_commit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 14678aae8e7d..0dcf20f22e62 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -210,7 +210,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( stderr := strings.Builder{} err := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - "--end-of-options", revision, + "--end-of-options", "'"+revision+"'", "--", file). Run(&RunOpts{ Dir: repo.Path, From 68ea75660103043c15700d08a3d03712a17430df Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:33:12 +0200 Subject: [PATCH 06/14] cmd-args-respect-args --- modules/git/repo_commit.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 0dcf20f22e62..54c6ff955db5 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -103,23 +103,23 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add authors if present in search query if len(opts.Authors) > 0 { for _, v := range opts.Authors { - args = append(args, "--author='"+v+"'") + args = append(args, "--author="+v) } } // add committers if present in search query if len(opts.Committers) > 0 { for _, v := range opts.Committers { - args = append(args, "--committer='"+v+"'") + args = append(args, "--committer="+v) } } // add time constraints if present in search query if len(opts.After) > 0 { - args = append(args, "--after='"+opts.After+"'") + args = append(args, "--after="+opts.After) } if len(opts.Before) > 0 { - args = append(args, "--before='"+opts.Before+"'") + args = append(args, "--before="+opts.Before) } // pretend that all refs along with HEAD were listed on command line as @@ -133,7 +133,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // note this is done only for command created above if len(opts.Keywords) > 0 { for _, v := range opts.Keywords { - cmd.AddArguments("--grep='" + v + "'") + cmd.AddArguments("--grep=" + v) } } @@ -158,7 +158,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", "'"+v+"'") + hashCmd.AddArguments("--end-of-options", ""+v+"") // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) From 3df8cfe23d7ad7fa21c0515edadd612f87f904cc Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 02:34:47 +0200 Subject: [PATCH 07/14] Update modules/git/repo_commit.go --- modules/git/repo_commit.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 54c6ff955db5..55146a74efb6 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -158,7 +158,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", ""+v+"") + hashCmd.AddArguments("--end-of-options", v) // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) @@ -210,7 +210,7 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( stderr := strings.Builder{} err := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow", "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - "--end-of-options", "'"+revision+"'", + "--end-of-options", revision, "--", file). Run(&RunOpts{ Dir: repo.Path, From 003dde9a38e6b0fc3890015c26019dea4a7d3fae Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 04:33:22 +0200 Subject: [PATCH 08/14] rm --- modules/git/commit.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/git/commit.go b/modules/git/commit.go index 31f0a9ebd78d..82b5e0b25d08 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -250,15 +250,15 @@ func NewSearchCommitsOptions(searchString string, forAllRefs bool) SearchCommits for _, k := range fields { switch { case strings.HasPrefix(k, "author:"): - authors = append(authors, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "author:")), "'", "")) + authors = append(authors, strings.TrimPrefix(k, "author:")) case strings.HasPrefix(k, "committer:"): - committers = append(committers, strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "committer:")), "'", "")) + committers = append(committers, strings.TrimPrefix(k, "committer:")) case strings.HasPrefix(k, "after:"): - after = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "after:")), "'", "") + after = strings.TrimPrefix(k, "after:") case strings.HasPrefix(k, "before:"): - before = strings.ReplaceAll(strings.TrimSpace(strings.TrimPrefix(k, "before:")), "'", "") + before = strings.TrimPrefix(k, "before:") default: - keywords = append(keywords, strings.ReplaceAll(strings.TrimSpace(k), "'", "")) + keywords = append(keywords, k) } } From 48c89a2a928bc2da333383aaa80c44d08050ae44 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 04:43:00 +0200 Subject: [PATCH 09/14] bump git requirement to 2.24 --- modules/git/git.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/git/git.go b/modules/git/git.go index 606a73b23097..449a8938b581 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -25,7 +25,7 @@ import ( ) // GitVersionRequired is the minimum Git version required -const GitVersionRequired = "2.0.0" +const GitVersionRequired = "2.24.0" var ( // GitExecutable is the command name of git @@ -117,8 +117,7 @@ func VersionInfo() string { } format := "%s" args := []interface{}{gitVersion.Original()} - // Since git wire protocol has been released from git v2.18 - if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { + if setting.Git.EnableAutoGitWireProtocol { format += ", Wire Protocol %s Enabled" args = append(args, "Version 2") // for focus color } From af6933b122e0a4b5667cf941a4a2c045a1569b30 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Oct 2022 11:55:04 +0800 Subject: [PATCH 10/14] fix --- modules/git/command.go | 12 ++++++++++++ modules/git/command_test.go | 17 +++++++++++++++++ modules/git/commit.go | 2 +- modules/git/git.go | 5 +++-- modules/git/repo_commit.go | 20 ++++++++++---------- modules/git/repo_stats.go | 8 ++++---- modules/gitgraph/graph.go | 19 +++++++------------ 7 files changed, 54 insertions(+), 29 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index b24d32dbe874..8ebd8898fba5 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -95,6 +95,18 @@ func (c *Command) AddArguments(args ...string) *Command { return c } +// AddDynamicArguments adds new dynamic argument(s) to the command. +// If the argument is invalid (it shouldn't happen in real life), it panics to caller +func (c *Command) AddDynamicArguments(args ...string) *Command { + for _, arg := range args { + if arg != "" && arg[0] == '-' { + panic("invalid argument: " + arg) + } + } + c.args = append(c.args, args...) + return c +} + // RunOpts represents parameters to run the command. If UseContextTimeout is specified, then Timeout is ignored. type RunOpts struct { Env []string diff --git a/modules/git/command_test.go b/modules/git/command_test.go index 67d4ca388e2b..c965ea230d89 100644 --- a/modules/git/command_test.go +++ b/modules/git/command_test.go @@ -26,4 +26,21 @@ func TestRunWithContextStd(t *testing.T) { assert.Contains(t, err.Error(), "exit status 129 - unknown option:") assert.Empty(t, stdout) } + + assert.Panics(t, func() { + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("-test") + }) + + assert.Panics(t, func() { + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("--test") + }) + + subCmd := "version" + cmd = NewCommand(context.Background()).AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production + stdout, stderr, err = cmd.RunStdString(&RunOpts{}) + assert.NoError(t, err) + assert.Empty(t, stderr) + assert.Contains(t, stdout, "git version") } diff --git a/modules/git/commit.go b/modules/git/commit.go index 82b5e0b25d08..6a99d7b45862 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -163,7 +163,7 @@ func AllCommitsCount(ctx context.Context, repoPath string, hidePRRefs bool, file // CommitsCountFiles returns number of total commits of until given revision. func CommitsCountFiles(ctx context.Context, repoPath string, revision, relpath []string) (int64, error) { cmd := NewCommand(ctx, "rev-list", "--count") - cmd.AddArguments(revision...) + cmd.AddDynamicArguments(revision...) if len(relpath) > 0 { cmd.AddArguments("--") cmd.AddArguments(relpath...) diff --git a/modules/git/git.go b/modules/git/git.go index 449a8938b581..31bfd9602789 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -25,7 +25,7 @@ import ( ) // GitVersionRequired is the minimum Git version required -const GitVersionRequired = "2.24.0" +const GitVersionRequired = "2.20.0" var ( // GitExecutable is the command name of git @@ -117,7 +117,8 @@ func VersionInfo() string { } format := "%s" args := []interface{}{gitVersion.Original()} - if setting.Git.EnableAutoGitWireProtocol { + // Since git wire protocol has been released from git v2.18 + if setting.Git.EnableAutoGitWireProtocol && CheckGitVersionAtLeast("2.18") == nil { format += ", Wire Protocol %s Enabled" args = append(args, "Version 2") // for focus color } diff --git a/modules/git/repo_commit.go b/modules/git/repo_commit.go index 55146a74efb6..8d848076bcc3 100644 --- a/modules/git/repo_commit.go +++ b/modules/git/repo_commit.go @@ -158,7 +158,7 @@ func (repo *Repository) searchCommits(id SHA1, opts SearchCommitsOptions) ([]*Co // add previous arguments except for --grep and --all hashCmd.AddArguments(args...) // add keyword as - hashCmd.AddArguments("--end-of-options", v) + hashCmd.AddDynamicArguments(v) // search with given constraints for commit matching sha hash of v hashMatching, _, err := hashCmd.RunStdBytes(&RunOpts{Dir: repo.Path}) @@ -208,15 +208,15 @@ func (repo *Repository) CommitsByFileAndRange(revision, file string, page int) ( }() go func() { stderr := strings.Builder{} - err := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow", - "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page), - "--end-of-options", revision, - "--", file). - Run(&RunOpts{ - Dir: repo.Path, - Stdout: stdoutWriter, - Stderr: &stderr, - }) + gitCmd := NewCommand(repo.Ctx, "log", prettyLogFormat, "--follow", + "--max-count="+strconv.Itoa(setting.Git.CommitsRangeSize*page)) + gitCmd.AddDynamicArguments(revision) + gitCmd.AddArguments("--", file) + err := gitCmd.Run(&RunOpts{ + Dir: repo.Path, + Stdout: stdoutWriter, + Stderr: &stderr, + }) if err != nil { _ = stdoutWriter.CloseWithError(ConcatenateError(err, (&stderr).String())) } else { diff --git a/modules/git/repo_stats.go b/modules/git/repo_stats.go index fb294f5a29dd..164ae07322d3 100644 --- a/modules/git/repo_stats.go +++ b/modules/git/repo_stats.go @@ -59,15 +59,15 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string) _ = stdoutWriter.Close() }() - args := []string{"log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", fmt.Sprintf("--since='%s'", since)} + gitCmd := NewCommand(repo.Ctx, "log", "--numstat", "--no-merges", "--pretty=format:---%n%h%n%aN%n%aE%n", "--date=iso", fmt.Sprintf("--since='%s'", since)) if len(branch) == 0 { - args = append(args, "--branches=*") + gitCmd.AddArguments("--branches=*") } else { - args = append(args, "--first-parent", "--", branch) + gitCmd.AddArguments("--first-parent").AddDynamicArguments(branch) } stderr := new(strings.Builder) - err = NewCommand(repo.Ctx, args...).Run(&RunOpts{ + err = gitCmd.Run(&RunOpts{ Env: []string{}, Dir: repo.Path, Stdout: stdoutWriter, diff --git a/modules/gitgraph/graph.go b/modules/gitgraph/graph.go index 271382525a4a..0f3c02134482 100644 --- a/modules/gitgraph/graph.go +++ b/modules/gitgraph/graph.go @@ -24,19 +24,17 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo page = 1 } - args := make([]string, 0, 12+len(branches)+len(files)) - - args = append(args, "--graph", "--date-order", "--decorate=full") + graphCmd := git.NewCommand(r.Ctx, "log", "--graph", "--date-order", "--decorate=full") if hidePRRefs { - args = append(args, "--exclude="+git.PullPrefix+"*") + graphCmd.AddArguments("--exclude=" + git.PullPrefix + "*") } if len(branches) == 0 { - args = append(args, "--all") + graphCmd.AddArguments("--all") } - args = append(args, + graphCmd.AddArguments( "-C", "-M", fmt.Sprintf("-n %d", setting.UI.GraphMaxCommitNum*page), @@ -44,15 +42,12 @@ func GetCommitGraph(r *git.Repository, page, maxAllowedColors int, hidePRRefs bo fmt.Sprintf("--pretty=format:%s", format)) if len(branches) > 0 { - args = append(args, branches...) + graphCmd.AddDynamicArguments(branches...) } - args = append(args, "--") if len(files) > 0 { - args = append(args, files...) + graphCmd.AddArguments("--") + graphCmd.AddArguments(files...) } - - graphCmd := git.NewCommand(r.Ctx, "log") - graphCmd.AddArguments(args...) graph := NewGraph() stderr := new(strings.Builder) From 34a2e91b6c96e4e1f3176408e1decad9bb7a29f0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Oct 2022 18:58:50 +0800 Subject: [PATCH 11/14] fix2 --- modules/git/command.go | 14 +++++++++++--- modules/git/command_test.go | 16 +++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index 8ebd8898fba5..20c5a739fc38 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -40,6 +40,7 @@ type Command struct { parentContext context.Context desc string globalArgsLength int + broken bool } func (c *Command) String() string { @@ -89,18 +90,19 @@ func (c *Command) SetDescription(desc string) *Command { return c } -// AddArguments adds new argument(s) to the command. +// AddArguments adds new argument(s) to the command. Each argument must be safe trusted. func (c *Command) AddArguments(args ...string) *Command { c.args = append(c.args, args...) return c } // AddDynamicArguments adds new dynamic argument(s) to the command. -// If the argument is invalid (it shouldn't happen in real life), it panics to caller +// The arguments may come from user input and can not be trusted, so no leading '-' is allowed to avoid passing options func (c *Command) AddDynamicArguments(args ...string) *Command { for _, arg := range args { if arg != "" && arg[0] == '-' { - panic("invalid argument: " + arg) + c.broken = true + return c } } c.args = append(c.args, args...) @@ -150,8 +152,14 @@ func CommonCmdServEnvs() []string { return commonBaseEnvs() } +var ErrBrokenCommand = errors.New("git command is command") + // Run runs the command with the RunOpts func (c *Command) Run(opts *RunOpts) error { + if c.broken { + log.Error("git command is broken: %s", c.String()) + return ErrBrokenCommand + } if opts == nil { opts = &RunOpts{} } diff --git a/modules/git/command_test.go b/modules/git/command_test.go index c965ea230d89..52d25c9c7482 100644 --- a/modules/git/command_test.go +++ b/modules/git/command_test.go @@ -27,15 +27,13 @@ func TestRunWithContextStd(t *testing.T) { assert.Empty(t, stdout) } - assert.Panics(t, func() { - cmd = NewCommand(context.Background()) - cmd.AddDynamicArguments("-test") - }) - - assert.Panics(t, func() { - cmd = NewCommand(context.Background()) - cmd.AddDynamicArguments("--test") - }) + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("-test") + assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand) + + cmd = NewCommand(context.Background()) + cmd.AddDynamicArguments("--test") + assert.ErrorIs(t, cmd.Run(&RunOpts{}), ErrBrokenCommand) subCmd := "version" cmd = NewCommand(context.Background()).AddDynamicArguments(subCmd) // for test purpose only, the sub-command should never be dynamic for production From d0c58585f3715095c11a22b685f27d93baf36374 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Oct 2022 19:01:45 +0800 Subject: [PATCH 12/14] Update modules/git/command.go --- modules/git/command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/command.go b/modules/git/command.go index 20c5a739fc38..bc76d3368ee6 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -90,7 +90,7 @@ func (c *Command) SetDescription(desc string) *Command { return c } -// AddArguments adds new argument(s) to the command. Each argument must be safe trusted. +// AddArguments adds new argument(s) to the command. Each argument must be safe to be trusted. func (c *Command) AddArguments(args ...string) *Command { c.args = append(c.args, args...) return c From 225aa1dc48b88e78417757608935c985ad9b1cb4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 15 Oct 2022 13:07:52 +0200 Subject: [PATCH 13/14] Update modules/git/git.go --- modules/git/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/git/git.go b/modules/git/git.go index 31bfd9602789..606a73b23097 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -25,7 +25,7 @@ import ( ) // GitVersionRequired is the minimum Git version required -const GitVersionRequired = "2.20.0" +const GitVersionRequired = "2.0.0" var ( // GitExecutable is the command name of git From bb28c33a905b437c5607bb427bfad746c9c0b30e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Oct 2022 19:28:12 +0800 Subject: [PATCH 14/14] backport changes --- modules/git/command.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/modules/git/command.go b/modules/git/command.go index bc76d3368ee6..ed06dd9b08de 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -40,7 +40,7 @@ type Command struct { parentContext context.Context desc string globalArgsLength int - broken bool + brokenArgs []string } func (c *Command) String() string { @@ -51,6 +51,7 @@ func (c *Command) String() string { } // NewCommand creates and returns a new Git Command based on given command and arguments. +// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. func NewCommand(ctx context.Context, args ...string) *Command { // Make an explicit copy of globalCommandArgs, otherwise append might overwrite it cargs := make([]string, len(globalCommandArgs)) @@ -64,11 +65,13 @@ func NewCommand(ctx context.Context, args ...string) *Command { } // NewCommandNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args +// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. func NewCommandNoGlobals(args ...string) *Command { return NewCommandContextNoGlobals(DefaultContext, args...) } // NewCommandContextNoGlobals creates and returns a new Git Command based on given command and arguments only with the specify args and don't care global command args +// Each argument should be safe to be trusted. User-provided arguments should be passed to AddDynamicArguments instead. func NewCommandContextNoGlobals(ctx context.Context, args ...string) *Command { return &Command{ name: GitExecutable, @@ -91,6 +94,7 @@ func (c *Command) SetDescription(desc string) *Command { } // AddArguments adds new argument(s) to the command. Each argument must be safe to be trusted. +// User-provided arguments should be passed to AddDynamicArguments instead. func (c *Command) AddArguments(args ...string) *Command { c.args = append(c.args, args...) return c @@ -101,10 +105,12 @@ func (c *Command) AddArguments(args ...string) *Command { func (c *Command) AddDynamicArguments(args ...string) *Command { for _, arg := range args { if arg != "" && arg[0] == '-' { - c.broken = true - return c + c.brokenArgs = append(c.brokenArgs, arg) } } + if len(c.brokenArgs) != 0 { + return c + } c.args = append(c.args, args...) return c } @@ -152,12 +158,12 @@ func CommonCmdServEnvs() []string { return commonBaseEnvs() } -var ErrBrokenCommand = errors.New("git command is command") +var ErrBrokenCommand = errors.New("git command is broken") // Run runs the command with the RunOpts func (c *Command) Run(opts *RunOpts) error { - if c.broken { - log.Error("git command is broken: %s", c.String()) + if len(c.brokenArgs) != 0 { + log.Error("git command is broken: %s, broken args: %s", c.String(), strings.Join(c.brokenArgs, " ")) return ErrBrokenCommand } if opts == nil {