diff --git a/.golangci.yml b/.golangci.yml index b5da28b098965ab0052d3462ea8de5544bbebacb..a0f86ad2eef0f0588491bdbb0cf8495d41d92360 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -51,9 +51,6 @@ issues: - path: helpers/shell_escape.go linters: - gocyclo - - path: helpers/shell/legacy_expand.go - linters: - - gocyclo - path: executors/kubernetes/kubernetes_test.go linters: - gocyclo @@ -85,4 +82,3 @@ issues: linters: - deadcode - unused - diff --git a/common/variables.go b/common/variables.go index 639435345bcb861bd2e4364c94f080eb1edc648a..28bfd202c5c18def484494739e2575e4d9b83101 100644 --- a/common/variables.go +++ b/common/variables.go @@ -3,9 +3,8 @@ package common import ( "errors" "fmt" + "os" "strings" - - "gitlab.com/gitlab-org/gitlab-runner/helpers/shell" ) type JobVariable struct { @@ -56,7 +55,7 @@ func (b JobVariables) Get(key string) string { } func (b JobVariables) ExpandValue(value string) string { - return shell.LegacyExpand(value, b.Get) + return os.Expand(value, b.Get) } func (b JobVariables) Expand() JobVariables { diff --git a/common/variables_test.go b/common/variables_test.go index 89204e66c18df99387e8c9c8a7a4f51d36151f40..6735c19218a008de70951059525fb5e1de088744 100644 --- a/common/variables_test.go +++ b/common/variables_test.go @@ -102,7 +102,7 @@ func TestSpecialVariablesExpansion(t *testing.T) { expanded := all.Expand() assert.Len(t, expanded, 4) assert.Equal(t, "$", expanded.Get("key")) - assert.Equal(t, "/dsa", expanded.Get("key2")) + assert.Equal(t, "$/dsa", expanded.Get("key2")) assert.Equal(t, "aabb", expanded.Get("key3")) assert.Equal(t, "aabb", expanded.Get("key4")) } diff --git a/helpers/shell/legacy_expand.go b/helpers/shell/legacy_expand.go deleted file mode 100644 index 657984ccf8354c53b7f376e7d70b52b34d397769..0000000000000000000000000000000000000000 --- a/helpers/shell/legacy_expand.go +++ /dev/null @@ -1,79 +0,0 @@ -// TODO: Remove in 13.0 - -// Backported from Go v1.10.8: -// https://github.com/golang/go/blob/8623503fe54642a21854c551129d550139f3bbac/src/os/env.go - -// Go v1.11 changed the behavior of Os.Expand() to gobble '$' only if it -// looks like it belongs to a valid shell variable. For example, -// $VARIABLE and ${VARIABLE} would expand to VARIABLE, but $\VARIABLE -// would retain its '$'. This might break CI variables that depend on -// this behavior. - -// Copyright 2010 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// General environment variables. - -package shell - -// Expand replaces ${var} or $var in the string based on the mapping function. -func LegacyExpand(s string, mapping func(string) string) string { - buf := make([]byte, 0, 2*len(s)) - // ${} is all ASCII, so bytes are fine for this operation. - i := 0 - for j := 0; j < len(s); j++ { - if s[j] == '$' && j+1 < len(s) { - buf = append(buf, s[i:j]...) - name, w := getShellName(s[j+1:]) - buf = append(buf, mapping(name)...) - j += w - i = j + 1 - } - } - return string(buf) + s[i:] -} - -// isShellSpecialVar reports whether the character identifies a special -// shell variable such as $*. -func isShellSpecialVar(c uint8) bool { - switch c { - case '*', '#', '$', '@', '!', '?', '-', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9': - return true - } - return false -} - -// isAlphaNum reports whether the byte is an ASCII letter, number, or underscore -func isAlphaNum(c uint8) bool { - return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' -} - -// getShellName returns the name that begins the string and the number of bytes -// consumed to extract it. If the name is enclosed in {}, it's part of a ${} -// expansion and two more bytes are needed than the length of the name. -func getShellName(s string) (string, int) { - switch { - case s[0] == '{': - if len(s) > 2 && isShellSpecialVar(s[1]) && s[2] == '}' { - return s[1:2], 3 - } - // Scan to closing brace - for i := 1; i < len(s); i++ { - if s[i] == '}' { - if i == 1 { - return "", 2 // Bad syntax; eat "${}" - } - return s[1:i], i + 1 - } - } - return "", 1 // Bad syntax; eat "${" - case isShellSpecialVar(s[0]): - return s[0:1], 1 - } - // Scan alphanumerics. - var i int - for i = 0; i < len(s) && isAlphaNum(s[i]); i++ { - } - return s[:i], i -} diff --git a/helpers/shell/legacy_expand_test.go b/helpers/shell/legacy_expand_test.go deleted file mode 100644 index 4ba9146c2b79e6123b10b18c8a711ae030be2e29..0000000000000000000000000000000000000000 --- a/helpers/shell/legacy_expand_test.go +++ /dev/null @@ -1,69 +0,0 @@ -// TODO: Remove in 13.0 - -// Backported from Go v1.10.8: -// https://github.com/golang/go/blob/8623503fe54642a21854c551129d550139f3bbac/src/os/env_test.go - -// Go v1.11 changed the behavior of Os.Expand() to gobble '$' only if it -// looks like it belongs to a valid shell variable. For example, -// $VARIABLE and ${VARIABLE} would expand to VARIABLE, but $\VARIABLE -// would retain its '$'. This might break CI variables that depend on -// this behavior. - -// Copyright 2010 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package shell - -import ( - "testing" -) - -// testGetenv gives us a controlled set of variables for testing Expand. -func testGetenv(s string) string { - switch s { - case "*": - return "all the args" - case "#": - return "NARGS" - case "$": - return "PID" - case "1": - return "ARGUMENT1" - case "HOME": - return "/usr/gopher" - case "H": - return "(Value of H)" - case "home_1": - return "/usr/foo" - case "_": - return "underscore" - } - return "" -} - -var expandTests = []struct { - in, out string -}{ - {"", ""}, - {"$*", "all the args"}, - {"$$", "PID"}, - {"${*}", "all the args"}, - {"$1", "ARGUMENT1"}, - {"${1}", "ARGUMENT1"}, - {"now is the time", "now is the time"}, - {"$HOME", "/usr/gopher"}, - {"$home_1", "/usr/foo"}, - {"${HOME}", "/usr/gopher"}, - {"${H}OME", "(Value of H)OME"}, - {"A$$$#$1$H$home_1*B", "APIDNARGSARGUMENT1(Value of H)/usr/foo*B"}, -} - -func TestLegacyExpand(t *testing.T) { - for _, test := range expandTests { - result := LegacyExpand(test.in, testGetenv) - if result != test.out { - t.Errorf("Expand(%q)=%q; expected %q", test.in, result, test.out) - } - } -}