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

[shved]

The case becomes more annoing if you use env variables from .env and run the service with docker-compose which according to docker docs does not respect any kind of qotation for values

[delwaterman]

Man is this still a thing

[intel352]

Wow, this has been ignored for a looooong time. And still in effect.

[richardwooding]

This is still an issue, sigh!

[zablvit]

2024 😢

[sagikazarmark]

Unfortunately, it's challenging to change a default behavior without someone calling it a regression.

Chances are, someone relies on the current behavior.

Here is a workaround that you could use (based on the example of the reporter):

package main

import (
	"fmt"
	"os"

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

var RootCmd = &cobra.Command{
	Run: func(cmd *cobra.Command, args []string) {
+		var h []string
+		viper.UnmarshalKey("hosts", &h)
-		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)
	}
}

I know a lot of people like using the Get* functions, but they are quite hard to maintain and are misused a lot, so I discourage people from using it.

I also opened a more generic issue to track this: #1744

I hope the above helps.

[dima-dmytruk23]

+1