Merge branch 'fix-multistringflag-split' into 'master'

Return error if setting an empty value to a MultiStringFlag

See merge request gitlab-org/gitlab-pages!218

3 files changed, 46 insertions, 1 deletions

diff --git a/README.md b/README.md
index f81d582f..0c26c835 100644
--- a/README.md
+++ b/README.md
@@ -76,7 +76,14 @@ run in SSL-offloading mode, custom TLS certificates will not work.
 Example:
 ```
 $ make
-$ ./gitlab-pages -listen-https "" -listen-http ":8090" -pages-root path/to/gitlab/shared/pages -pages-domain example.com
+$ ./gitlab-pages -listen-http ":8090" -pages-root path/to/gitlab/shared/pages -pages-domain example.com
+```
+
+To run on HTTPS ensure you have a root certificate key pair available
+
+```
+$ make
+$ ./gitlab-pages -listen-https ":9090" -root-cert=path/to/example.com.crt -root-key=path/to/example.com.key -pages-root path/to/gitlab/shared/pages -pages-domain example.com
 ```
 
 ### Run daemon **in secure mode**
diff --git a/multi_string_flag.go b/multi_string_flag.go
index 25e9b9fe..699529a0 100644
--- a/multi_string_flag.go
+++ b/multi_string_flag.go
@@ -1,9 +1,12 @@
 package main
 
 import (
+	"errors"
 	"strings"
 )
 
+var errMultiStringSetEmptyValue = errors.New("value cannot be empty")
+
 // MultiStringFlag implements the flag.Value interface and allows a string flag
 // to be specified multiple times on the command line.
 //
@@ -17,6 +20,9 @@ func (s *MultiStringFlag) String() string {
 
 // Set appends the value to the list of parameters
 func (s *MultiStringFlag) Set(value string) error {
+	if value == "" {
+		return errMultiStringSetEmptyValue
+	}
 	*s = append(*s, value)
 	return nil
 }
diff --git a/multi_string_flag_test.go b/multi_string_flag_test.go
index 20cac831..5cddac98 100644
--- a/multi_string_flag_test.go
+++ b/multi_string_flag_test.go
@@ -16,5 +16,37 @@ func TestMultiStringFlagAppendsOnSet(t *testing.T) {
 	require.NoError(t, iface.Set("foo"))
 	require.NoError(t, iface.Set("bar"))
 
+	require.EqualError(t, iface.Set(""), "value cannot be empty")
+
 	require.Equal(t, MultiStringFlag{"foo", "bar"}, concrete)
 }
+
+func TestMultiStringFlag_Split(t *testing.T) {
+	tests := []struct {
+		name       string
+		s          *MultiStringFlag
+		wantResult []string
+	}{
+		{
+			name:       "empty_string",
+			s:          &MultiStringFlag{}, // -flag ""
+			wantResult: []string{},
+		},
+		{
+			name:       "one_value",
+			s:          &MultiStringFlag{"value1"}, // -flag "value1"
+			wantResult: []string{"value1"},
+		},
+		{
+			name:       "multiple_values",
+			s:          &MultiStringFlag{"value1", "", "value3"}, // -flag "value1,,value3"
+			wantResult: []string{"value1", "", "value3"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotResult := tt.s.Split()
+			require.ElementsMatch(t, tt.wantResult, gotResult)
+		})
+	}
+}