diff options
author | Paul Okstad <pokstad@gitlab.com> | 2020-05-22 12:48:00 +0300 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2020-05-22 12:48:00 +0300 |
commit | 472a07e34ca5d2124ed8173f65d3bb62dc6be6ee (patch) | |
tree | dde7f2c49d12d6f735a119a5c4b29e2e94a7a5ed | |
parent | 57f4779249b0be37659b9bd67f410a6b478ef3b7 (diff) |
Add end-of-options to supported commands
-rw-r--r-- | .gitlab-ci.yml | 8 | ||||
-rw-r--r-- | changelogs/unreleased/po-end-of-options.yml | 5 | ||||
-rw-r--r-- | internal/git/safecmd.go | 12 | ||||
-rw-r--r-- | internal/git/safecmd_test.go | 14 | ||||
-rw-r--r-- | internal/service/smarthttp/upload_pack_test.go | 9 |
5 files changed, 27 insertions, 21 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 08d05b215..9243413ef 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -124,19 +124,11 @@ test:go1.14-git-2.24-ruby-2.6: image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.14-git-2.24 <<: *test_definition -test:go1.14-git-2.22-ruby-2.6: - image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.14-git-2.26 - <<: *test_definition - test:go1.13-git-2.26-ruby-2.6: image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.26 <<: *test_definition test:go1.13-git-2.24-ruby-2.6: - image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.22 - <<: *test_definition - -test:go1.13-git-2.22-ruby-2.6: image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6-golang-1.13-git-2.24 <<: *test_definition diff --git a/changelogs/unreleased/po-end-of-options.yml b/changelogs/unreleased/po-end-of-options.yml new file mode 100644 index 000000000..1b2d85e0a --- /dev/null +++ b/changelogs/unreleased/po-end-of-options.yml @@ -0,0 +1,5 @@ +--- +title: Add end-of-options to supported commands +merge_request: 2192 +author: +type: added diff --git a/internal/git/safecmd.go b/internal/git/safecmd.go index 6906c9b55..d60fb076b 100644 --- a/internal/git/safecmd.go +++ b/internal/git/safecmd.go @@ -59,6 +59,14 @@ var subCmdNameRegex = regexp.MustCompile(`^[[:alnum:]]+(-[[:alnum:]]+)*$`) // IsCmd allows SubCmd to satisfy the Cmd interface func (sc SubCmd) IsCmd() {} +func (sc SubCmd) supportsEndOfOptions() bool { + switch sc.Name { + case "linguist", "for-each-ref", "archive", "upload-archive", "grep", "clone", "config", "rev-parse", "remote", "blame", "ls-tree": + return false + } + return true +} + // ValidateArgs checks all arguments in the sub command and validates them func (sc SubCmd) ValidateArgs() ([]string, error) { var safeArgs []string @@ -83,6 +91,10 @@ func (sc SubCmd) ValidateArgs() ([]string, error) { safeArgs = append(safeArgs, a) } + if sc.supportsEndOfOptions() { + safeArgs = append(safeArgs, "--end-of-options") + } + if len(sc.PostSepArgs) > 0 { safeArgs = append(safeArgs, "--") } diff --git a/internal/git/safecmd_test.go b/internal/git/safecmd_test.go index 257a5646d..07f742bcc 100644 --- a/internal/git/safecmd_test.go +++ b/internal/git/safecmd_test.go @@ -125,6 +125,8 @@ func TestSafeCmdValid(t *testing.T) { reenableGitCmd := disableGitCmd() defer reenableGitCmd() + endOfOptions := "--end-of-options" + for _, tt := range []struct { globals []git.Option subCmd git.SubCmd @@ -132,14 +134,14 @@ func TestSafeCmdValid(t *testing.T) { }{ { subCmd: git.SubCmd{Name: "meow"}, - expectArgs: []string{"meow"}, + expectArgs: []string{"meow", endOfOptions}, }, { globals: []git.Option{ git.Flag{"--aaaa-bbbb"}, }, subCmd: git.SubCmd{Name: "cccc"}, - expectArgs: []string{"--aaaa-bbbb", "cccc"}, + expectArgs: []string{"--aaaa-bbbb", "cccc", endOfOptions}, }, { subCmd: git.SubCmd{ @@ -147,7 +149,7 @@ func TestSafeCmdValid(t *testing.T) { Args: []string{""}, PostSepArgs: []string{"-woof", ""}, }, - expectArgs: []string{"meow", "", "--", "-woof", ""}, + expectArgs: []string{"meow", "", endOfOptions, "--", "-woof", ""}, }, { globals: []git.Option{ @@ -164,7 +166,7 @@ func TestSafeCmdValid(t *testing.T) { Args: []string{"1", "2"}, PostSepArgs: []string{"3", "4", "5"}, }, - expectArgs: []string{"-a", "-b", "c", "d", "-e", "-f", "g", "-h=i", "1", "2", "--", "3", "4", "5"}, + expectArgs: []string{"-a", "-b", "c", "d", "-e", "-f", "g", "-h=i", "1", "2", endOfOptions, "--", "3", "4", "5"}, }, { subCmd: git.SubCmd{ @@ -175,7 +177,7 @@ func TestSafeCmdValid(t *testing.T) { git.Flag{"--adjective"}, }, }, - expectArgs: []string{"noun", "verb", "-", "--adjective"}, + expectArgs: []string{"noun", "verb", "-", "--adjective", endOfOptions}, }, { globals: []git.Option{ @@ -190,7 +192,7 @@ func TestSafeCmdValid(t *testing.T) { git.ValueFlag{"--why", "looking-for-first-contribution"}, }, }, - expectArgs: []string{"--contributing", "--author", "a-gopher", "accept", "--is-important", "--why", "looking-for-first-contribution", "mr"}, + expectArgs: []string{"--contributing", "--author", "a-gopher", "accept", "--is-important", "--why", "looking-for-first-contribution", "mr", endOfOptions}, }, } { cmd, err := git.SafeCmd(ctx, testRepo, tt.globals, tt.subCmd) diff --git a/internal/service/smarthttp/upload_pack_test.go b/internal/service/smarthttp/upload_pack_test.go index 26d078846..780039a0e 100644 --- a/internal/service/smarthttp/upload_pack_test.go +++ b/internal/service/smarthttp/upload_pack_test.go @@ -164,13 +164,8 @@ func TestUploadPackRequestWithGitConfigOptions(t *testing.T) { response, err = makePostUploadPackRequest(ctx, t, serverSocketPath, rpcRequest, requestBodyCopy) testhelper.RequireGrpcError(t, err, codes.Unavailable) - // Remove the if clause if support is dropped for Git versions before 2.22 - if git.NoMissingWantErrMessage() { - assert.Equal(t, "", response.String(), "Ref is hidden so no response should be received") - } else { - expected := fmt.Sprintf("0049ERR upload-pack: not our ref %v", want) - assert.Equal(t, expected, response.String(), "Ref is hidden, expected error message did not appear") - } + expected := fmt.Sprintf("0049ERR upload-pack: not our ref %v", want) + assert.Equal(t, expected, response.String(), "Ref is hidden, expected error message did not appear") } func TestUploadPackRequestWithGitProtocol(t *testing.T) { |