Skip to content

Commit

Permalink
[TOOL-1955] Fix infinite loop (#717)
Browse files Browse the repository at this point in the history
* added test to detect infinite loops

* another loop test

* added test

* more tests, test fixes, new expand behaviour that mimics the way shell expands

* simplified logic

* require new plugin, version bump

* dep update
  • Loading branch information
Szabados Áron authored Apr 17, 2020
1 parent 6414764 commit dcef36f
Show file tree
Hide file tree
Showing 37 changed files with 211 additions and 41 deletions.
8 changes: 4 additions & 4 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions _tests/integration/version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func Test_VersionOutput(t *testing.T) {
{
out, err := command.RunCommandAndReturnCombinedStdoutAndStderr(binPath(), "version")
require.NoError(t, err)
require.Equal(t, "1.41.0", out)
require.Equal(t, "1.41.1", out)
}

t.Log("Version --full")
Expand All @@ -23,7 +23,7 @@ func Test_VersionOutput(t *testing.T) {
require.NoError(t, err)

expectedOSVersion := fmt.Sprintf("%s (%s)", runtime.GOOS, runtime.GOARCH)
expectedVersionOut := fmt.Sprintf(`version: 1.41.0
expectedVersionOut := fmt.Sprintf(`version: 1.41.1
format version: 10
os: %s
go: %s
Expand Down
2 changes: 1 addition & 1 deletion bitrise/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var PluginDependencyMap = map[string]PluginDependency{
},
"analytics": PluginDependency{
Source: "https://github.com/bitrise-io/bitrise-plugins-analytics.git",
MinVersion: "0.12.0",
MinVersion: "0.12.1",
},
}

Expand Down
44 changes: 21 additions & 23 deletions cli/run_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,26 @@ func expandStepInputs(
) map[string]string {
stepInputs := make(map[string]string)

var mappingFuncFactory func([]envmanModels.EnvironmentItemModel) func(string) string
mappingFuncFactory = func(environments []envmanModels.EnvironmentItemModel) func(key string) string {
return func(key string) string {
for inputName, inputValue := range stepInputs {
if inputName == key {
return os.Expand(inputValue, mappingFuncFactory(environments))
}
}

for index, environmentItem := range environments {
envValue := environmentItem[key]
if envValue != nil {
return os.Expand(envValue.(string), mappingFuncFactory(environments[:index]))
}
}

return os.Getenv(key)
}
}

This comment has been minimized.

Copy link
@jhass

jhass Apr 20, 2020

This panics somewhere if bitrise.yml contains something along the lines of

env: 
  FOO: 123

So defining an environment variable with an integer value instead of a string value.

panic: interface conversion: interface {} is int, not string
goroutine 1 [running]:
github.com/bitrise-io/bitrise/cli.expandStepInputs.func1.1(0xc00018c611, 0x17, 0xc00018c611, 0x17)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:601 +0x2f3
os.Expand(0xc00018c000, 0x96d, 0xc001551890, 0xc001551890, 0x96d)
	/usr/local/Cellar/go/1.13.6/libexec/src/os/env.go:35 +0x254
github.com/bitrise-io/bitrise/cli.expandStepInputs(0xc001214460, 0x2, 0x2, 0xc0000cf600, 0xa8, 0x120, 0x436ebb)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:619 +0x3e3
github.com/bitrise-io/bitrise/cli.activateAndRunSteps.func1(0xc001214aa0, 0xc001214070, 0xc0012140a0, 0xc0012140e0, 0xc001214130, 0xc001214170, 0xc0001e55c0, 0xc0001e5620, 0x0, 0xc0001e5660, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:670 +0x225
github.com/bitrise-io/bitrise/cli.activateAndRunSteps(0xc00013a9d9, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1008 +0x1490
github.com/bitrise-io/bitrise/cli.runWorkflow(0xc00013a9d9, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1026 +0x206
github.com/bitrise-io/bitrise/cli.activateAndRunWorkflow(0xc00013a9d9, 0x5, 0xc00013a9d9, 0x5, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1056 +0x3cb
github.com/bitrise-io/bitrise/cli.activateAndRunWorkflow(0xc0001c2170, 0xc, 0xc0001c2170, 0xc, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1071 +0x637
github.com/bitrise-io/bitrise/cli.activateAndRunWorkflow(0xc000106340, 0x11, 0xc000106340, 0x11, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1071 +0x637
github.com/bitrise-io/bitrise/cli.runWorkflowWithConfiguration(0xbf9f7f2e0b9338f6, 0x1a41841, 0xce32c0, 0xc000106340, 0x11, 0xc9e835, 0x1, 0xc000017cc0, 0x31, 0xc00013a550, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run_util.go:1168 +0x9e6
github.com/bitrise-io/bitrise/cli.runAndExit(0xc9e835, 0x1, 0xc000017cc0, 0x31, 0xc00013a550, 0x7, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run.go:117 +0x165
github.com/bitrise-io/bitrise/cli.run(0xc0000a09a0, 0x0, 0xc000096ab0)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/run.go:273 +0xbe4
github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli.HandleAction(0x877460, 0x9321c8, 0xc0000a09a0, 0xc0000a09a0, 0x0)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli/app.go:514 +0xbe
github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli.Command.Run(0x90a9ad, 0x3, 0x0, 0x0, 0xccea40, 0x1, 0x1, 0x9146cb, 0x1a, 0x0, ...)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli/command.go:171 +0x4cd
github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli.(*App).Run(0xc0000dc380, 0xc000010080, 0x8, 0x8, 0x0, 0x0)
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/vendor/github.com/urfave/cli/app.go:265 +0x72f
github.com/bitrise-io/bitrise/cli.Run()
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/cli/cli.go:160 +0x25c
main.main()
	/Users/vagrant/go/src/github.com/bitrise-io/bitrise/main.go:6 +0x20
// Retrieve all non-sensitive input values
for _, input := range inputs {
if err := input.FillMissingDefaults(); err != nil {
Expand All @@ -596,7 +616,7 @@ func expandStepInputs(
options, err := input.GetOptions()
if err == nil && *options.IsSensitive == false {
if inputName, inputValue, err := input.GetKeyValuePair(); err == nil {
stepInputs[inputName] = inputValue
stepInputs[inputName] = os.Expand(inputValue, mappingFuncFactory(environments))
} else {
log.Warnf("Failed to get input value for '%s', skipping input: %s", inputName, err)
}
Expand All @@ -605,28 +625,6 @@ func expandStepInputs(
}
}

var mappingFunc func(string) string
mappingFunc = func(key string) string {
for inputName, inputValue := range stepInputs {
if inputName == key {
return os.Expand(inputValue, mappingFunc)
}
}

for _, environmentItem := range environments {
envValue := environmentItem[key]
if envValue != nil {
return os.Expand(envValue.(string), mappingFunc)
}
}

return os.Getenv(key)
}

for inputName, inputValue := range stepInputs {
stepInputs[inputName] = os.Expand(inputValue, mappingFunc)
}

return stepInputs
}

Expand Down
85 changes: 84 additions & 1 deletion cli/run_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,9 +805,9 @@ func TestExpandStepInputsNeedExpansionWithinExpansion(t *testing.T) {
}

testEnvironment := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"SIMULATOR_OS_VERSION": "$SIMULATOR_OS_MAJOR_VERSION.$SIMULATOR_OS_MINOR_VERSION", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"SIMULATOR_OS_MAJOR_VERSION": "13", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"SIMULATOR_OS_MINOR_VERSION": "3", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"SIMULATOR_OS_VERSION": "$SIMULATOR_OS_MAJOR_VERSION.$SIMULATOR_OS_MINOR_VERSION", "opts": map[string]interface{}{"is_sensitive": false}},
}

// Act
Expand Down Expand Up @@ -838,3 +838,86 @@ func TestExpandStepInputsNeedCrossInputExpansion(t *testing.T) {
require.Empty(t, expandedInputs["secret_input"])
require.Equal(t, "iPhone 8 (13.3)", expandedInputs["simulator_device"])
}

func TestExpandSimpleLoopSkipped(t *testing.T) {
// Arrange
testInputs := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"loop": "$loop", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"env_loop": "$ENV_LOOP", "opts": map[string]interface{}{"is_sensitive": false}},
}

testEnvironment := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"ENV_LOOP": "$ENV_LOOP", "opts": map[string]interface{}{"is_sensitive": false}},
}

// Act
expandedInputs := expandStepInputs(testInputs, testEnvironment)

// Assert
require.NotNil(t, expandedInputs)
require.Equal(t, "", expandedInputs["loop"])
require.Equal(t, "", expandedInputs["env_loop"])
}

func TestExpandLoopWithLengthOneSkipped(t *testing.T) {
// Arrange
testInputs := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"loop": "Something: $loop", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"env_loop": "$ENV_LOOP", "opts": map[string]interface{}{"is_sensitive": false}},
}

testEnvironment := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"ENV_LOOP": "Env Something: $ENV_LOOP", "opts": map[string]interface{}{"is_sensitive": false}},
}

// Act
expandedInputs := expandStepInputs(testInputs, testEnvironment)

// Assert
require.NotNil(t, expandedInputs)
require.Equal(t, "Something: ", expandedInputs["loop"])
require.Equal(t, "Env Something: ", expandedInputs["env_loop"])
}

func TestExpandPrefixNotSkipped(t *testing.T) {
// Arrange
testInputs := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"similar2": "anything", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"similar": "$similar2", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"env": "Something: $similar", "opts": map[string]interface{}{"is_sensitive": false}},
}

testEnvironment := []envmanModels.EnvironmentItemModel{}

// Act
expandedInputs := expandStepInputs(testInputs, testEnvironment)

// Assert
require.NotNil(t, expandedInputs)
require.Equal(t, "Something: anything", expandedInputs["env"])
}

func TestExpandMultiLengthLoopsSkipped(t *testing.T) {
// Arrange
testInputs := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"a": "$b", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"b": "$c", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"c": "$a", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"env": "$A", "opts": map[string]interface{}{"is_sensitive": false}},
}

testEnvironment := []envmanModels.EnvironmentItemModel{
envmanModels.EnvironmentItemModel{"B": "$A", "opts": map[string]interface{}{"is_sensitive": false}},
envmanModels.EnvironmentItemModel{"A": "$B", "opts": map[string]interface{}{"is_sensitive": false}},
}

// Act
expandedInputs := expandStepInputs(testInputs, testEnvironment)

// Assert
require.NotNil(t, expandedInputs)
require.Equal(t, "", expandedInputs["a"])
require.Equal(t, "", expandedInputs["b"])
require.Equal(t, "", expandedInputs["c"])
require.Equal(t, "", expandedInputs["env"])
}
8 changes: 8 additions & 0 deletions vendor/golang.org/x/crypto/ssh/terminal/terminal.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions vendor/golang.org/x/sys/unix/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions vendor/golang.org/x/sys/unix/mkerrors.sh

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 16 additions & 5 deletions vendor/golang.org/x/sys/unix/zerrors_linux.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions vendor/golang.org/x/sys/unix/zsysnum_linux_386.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions vendor/golang.org/x/sys/unix/zsysnum_linux_amd64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit dcef36f

Please sign in to comment.