Viper: StringSlice behavior different for command line flag and environment variable

[jgoodall]

The behavior is different for StringSlice if an argument is on the command line flag compared to an environment variable. The flag is parsed correctly, but the environment variable is not.

Here is an example to see the behavior:

package main

import (
    "fmt"
    "os"

    "github.com/spf13/cobra"
    "github.com/spf13/viper"
)

var RootCmd = &cobra.Command{
    Run: func(cmd *cobra.Command, args []string) {
        h := viper.GetStringSlice("hosts")
        fmt.Printf("hosts: %+v (type: %T, length: %d)\n", h, h, len(h))
    },
}

func init() {

    viper.SetEnvPrefix("test")
    viper.AutomaticEnv()

    RootCmd.PersistentFlags().StringSlice("hosts", []string{}, "list of hosts")
    viper.BindPFlag("hosts", RootCmd.PersistentFlags().Lookup("hosts"))
    viper.SetDefault("hosts", []string{})

}

func Execute() {
    RootCmd.Execute()
}

func main() {
    if err := RootCmd.Execute(); err != nil {
        fmt.Println(err)
        os.Exit(1)
    }
}

Here is the output:

# Incorrect length of 1
$ TEST_HOSTS=1.2.3.4:9000,5.6.7.8:9000 go run main.go
hosts: [1.2.3.4:9000,5.6.7.8:9000] (type: []string, length: 1)
# Correctly set length of 2
$ go run main.go --hosts=1.2.3.4:9000,5.6.7.8:9000
hosts: [1.2.3.4:9000 5.6.7.8:9000] (type: []string, length: 2)

[mindscratch]

Another example, which shows environment variables are handled differently. Notice, the last examlpe shows if the environment variable values are separated by spaces then a slice is correctly populated. Here's the code:

package main

import (
    "fmt"
    "github.com/spf13/pflag"
    "github.com/spf13/viper"
    "strings"
)

func main() {
    viper.SetEnvPrefix("LR")
    viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
    viper.AutomaticEnv()

    pflag.StringSlice("foo-bar", []string{"a", "b"}, "stuff")

    pflag.Parse()
    viper.BindPFlags(pflag.CommandLine)

    result := viper.GetStringSlice("foo-bar")
    fmt.Printf("RESULT: %v (type=%T, len=%d)\n", result, result, len(result))

    for i, val := range result {
        fmt.Printf("%d\t%s\n", i, val)
    }
}

If you build it (name the binary app) and run it:

> ./app
RESULT: [a b] (type=[]string, len=2)
0   a
1   b

> LR_FOO_BAR="a,b,c" ./app
RESULT: [a,b,c] (type=[]string, len=1)
0   a,b,c

> LR_FOO_BAR="a b c" ./app
RESULT: [a b c] (type=[]string, len=3)
0   a
1   b
2   c

[flowchartsman]

This looks like it could use some movement; is anyone working on it?

[hhubris]

I ran into the same issue and demonstrated it with this test case.

func TestAutoEnvStringSlice(t *testing.T) {
        Reset()

        AutomaticEnv()

        // this passes
        os.Setenv("BAR", "a b c d")
        assert.Equal(t, []string{"a","b","c","d"}, GetStringSlice("bar"))

        // this should pass, but fails
        os.Setenv("BAR", "a,b,c,d")
        assert.Equal(t, []string{"a","b","c","d"}, GetStringSlice("bar"))
}

➜  viper git:(master) ✗ go test
--- FAIL: TestAutoEnvStringSlice (0.00s)
    viper_test.go:468:
            Error Trace:    viper_test.go:468
            Error:          Not equal:
                            expected: []string{"a", "b", "c", "d"}
                            actual  : []string{"a,b,c,d"}

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1,6 +1,3 @@
                            -([]string) (len=4) {
                            - (string) (len=1) "a",
                            - (string) (len=1) "b",
                            - (string) (len=1) "c",
                            - (string) (len=1) "d"
                            +([]string) (len=1) {
                            + (string) (len=7) "a,b,c,d"
                             }
            Test:           TestAutoEnvStringSlice
FAIL
exit status 1
FAIL    github.com/spf13/viper  3.505s

Further research, the code in pflag string_slice.go that parses StringArray is:

func readAsCSV(val string) ([]string, error) {
    if val == "" {
        return []string{}, nil
    }
    stringReader := strings.NewReader(val)
    csvReader := csv.NewReader(stringReader)
    return csvReader.Read()
}

Viper uses the cast library, which only splits on runes that return true for IsSpace()

I think this hasn't been addressed because viper uses the cast library in many places, and the correct fix is probably to extract a common lib that both viper and pflag use to ensure the same behavior. It seems to be more than a 5 minute fix.

[shousper]

Any updates on this issue?

[paulbdavis]

The behavior seems inconsistent between the flags and the env, as evidenced by the following:

package main

import (
    "log"

    "github.com/spf13/pflag"
    "github.com/spf13/viper"
)

func main() {
    viper.SetEnvPrefix("foo")
    viper.AutomaticEnv()

    pflag.StringSlice("strarr", []string{}, "")
    viper.BindPFlags(pflag.CommandLine)

    pflag.Parse()

    strs := viper.GetStringSlice("strarr")
    log.Println(len(strs), strs)
}

Then running these yields the following:

% ./vipertest --strarr foo --strarr bar
2019/12/03 10:37:13 2 [foo bar] # expected result

% ./vipertest --strarr "foo bar"       
2019/12/03 10:39:41 1 [foo bar] # expected result

% ./vipertest --strarr foo,bar  
2019/12/03 10:37:05 2 [foo bar] # parses , as a delimiter

% FOO_STRARR=foo,bar ./vipertest  
2019/12/03 10:37:01 1 [foo,bar] # doesn't parse , as a delimiter

% FOO_STRARR="foo bar" ./vipertest
2019/12/03 10:39:17 2 [foo bar] # expected result