MYST-546/Refactor spaces in option values handling

openvpn/config.go

@@ -18,7 +18,6 @@
 package openvpn
 
 import (
-	"fmt"
 	"path/filepath"
 	"strconv"
 )
@@ -46,9 +45,9 @@ func (c *Config) AddOptions(options ...configOption) {
 	c.options = append(c.options, options...)
 }
 
-func (c *Config) setParam(name, value string) {
+func (c *Config) setParam(name string, values ...string) {
 	c.AddOptions(
-		OptionParam(name, value),
+		OptionParam(name, values...),
 	)
 }
 
@@ -60,9 +59,7 @@ func (c *Config) setFlag(name string) {
 
 // SetManagementAddress creates TCP socket option for communication with openvpn process
 func (c *Config) SetManagementAddress(ip string, port int) {
-	address := fmt.Sprintf("%s %d", ip, port)
-
-	c.setParam("management", address)
+	c.setParam("management", ip, strconv.Itoa(port))
 	c.setFlag("management-client")
 }
 
@@ -102,7 +99,7 @@ func (c *Config) RestrictReconnects() {
 
 // SetKeepAlive setups keepalive interval and timeout values
 func (c *Config) SetKeepAlive(interval, timeout int) {
-	c.setParam("keepalive", strconv.Itoa(interval)+" "+strconv.Itoa(timeout))
+	c.setParam("keepalive", strconv.Itoa(interval), strconv.Itoa(timeout))
 }
 
 // SetPingTimerRemote sets "ping from remote required" option

openvpn/factory.go

@@ -18,6 +18,7 @@
 package openvpn
 
 import (
+	"fmt"
 	"github.com/mysterium/node/openvpn/tls"
 	"io/ioutil"
 	"path/filepath"
@@ -73,9 +74,9 @@ func newClientConfig(configDir string) *ClientConfig {
 
 	config.setParam("reneg-sec", "60")
 	config.setParam("resolv-retry", "infinite")
-	config.setParam("redirect-gateway", "def1 bypass-dhcp")
-	config.setParam("dhcp-option", "DNS 208.67.222.222")
-	config.setParam("dhcp-option", "DNS 208.67.220.220")
+	config.setParam("redirect-gateway", "def1", "bypass-dhcp")
+	config.setParam("dhcp-option", "DNS", "208.67.222.222")
+	config.setParam("dhcp-option", "DNS", "208.67.220.220")
 
 	return &config
 }
@@ -106,8 +107,15 @@ func NewClientConfigFromSession(vpnConfig *VPNConfig, configDir string, configFi
 	config := ClientConfig{NewConfig(configDir)}
 	config.AddOptions(OptionFile("config", configAsString, configFile))
 
-	config.setParam("up", filepath.Join(configDir, "update-resolv-conf"))
-	config.setParam("down", filepath.Join(configDir, "update-resolv-conf"))
+	//because of special case how openvpn handles executable/scripts paths, we need to surround values with double quotes
+	updateResolvConfScriptPath := wrapWithDoubleQuotes(filepath.Join(configDir, "update-resolv-conf"))
+
+	config.setParam("up", updateResolvConfScriptPath)
+	config.setParam("down", updateResolvConfScriptPath)
 
 	return &config, nil
 }
+
+func wrapWithDoubleQuotes(val string) string {
+	return fmt.Sprintf(`"%s"`, val)
+}

openvpn/option_file.go

@@ -39,12 +39,12 @@ func (option optionFile) getName() string {
 	return option.name
 }
 
-func (option optionFile) toCli() (string, error) {
+func (option optionFile) toCli() ([]string, error) {
 	err := ioutil.WriteFile(option.filePath, []byte(option.content), 0600)
 	if err != nil {
-		return "", err
+		return nil, err
 	}
-	return "--" + option.name + " " + option.filePath, nil
+	return []string{"--" + option.name, option.filePath}, nil
 }
 
 func (option optionFile) toFile() (string, error) {

openvpn/option_file_test.go

@@ -44,7 +44,7 @@ func TestFile_ToCli(t *testing.T) {
 
 	optionValue, err := option.toCli()
 	assert.NoError(t, err)
-	assert.Equal(t, "--special-file "+filename, optionValue)
+	assert.Equal(t, []string{"--special-file", filename}, optionValue)
 	readedContent, err := ioutil.ReadFile(filename)
 	assert.NoError(t, err)
 	assert.Equal(t, fileContent, string(readedContent))

openvpn/option_flag.go

@@ -29,8 +29,8 @@ func (option optionFlag) getName() string {
 	return option.name
 }
 
-func (option optionFlag) toCli() (string, error) {
-	return "--" + option.name, nil
+func (option optionFlag) toCli() ([]string, error) {
+	return []string{"--" + option.name}, nil
 }
 
 func (option optionFlag) toFile() (string, error) {

openvpn/option_flag_test.go

@@ -37,7 +37,7 @@ func TestFlag_ToCli(t *testing.T) {
 
 	optionValue, err := option.toCli()
 	assert.NoError(t, err)
-	assert.Equal(t, "--enable-something", optionValue)
+	assert.Equal(t, []string{"--enable-something"}, optionValue)
 }
 
 func TestFlag_ToFile(t *testing.T) {

openvpn/option_param.go

@@ -17,23 +17,25 @@
 
 package openvpn
 
-func OptionParam(name, value string) optionParam {
-	return optionParam{name, value}
+import "strings"
+
+func OptionParam(name string, values ...string) optionParam {
+	return optionParam{name, values}
 }
 
 type optionParam struct {
-	name  string
-	value string
+	name   string
+	values []string
 }
 
 func (option optionParam) getName() string {
 	return option.name
 }
 
-func (option optionParam) toCli() (string, error) {
-	return "--" + option.name + " " + option.value, nil
+func (option optionParam) toCli() ([]string, error) {
+	return append([]string{"--" + option.name}, option.values...), nil
 }
 
 func (option optionParam) toFile() (string, error) {
-	return option.name + " " + option.value, nil
+	return option.name + " " + strings.Join(option.values, " "), nil
 }

openvpn/option_param_test.go

@@ -37,7 +37,7 @@ func TestParam_ToCli(t *testing.T) {
 
 	optionValue, err := option.toCli()
 	assert.NoError(t, err)
-	assert.Equal(t, "--very-value 1234", optionValue)
+	assert.Equal(t, []string{"--very-value", "1234"}, optionValue)
 }
 
 func TestParam_ToFile(t *testing.T) {

openvpn/serializer_cli.go

@@ -19,7 +19,6 @@ package openvpn
 
 import (
 	"fmt"
-	"strings"
 )
 
 func (config *Config) ConfigToArguments() ([]string, error) {
@@ -31,18 +30,17 @@ func (config *Config) ConfigToArguments() ([]string, error) {
 			return nil, fmt.Errorf("Unserializable option '%s': %#v", item.getName(), item)
 		}
 
-		optionValue, err := option.toCli()
+		optionValues, err := option.toCli()
 		if err != nil {
 			return nil, err
 		}
 
-		optionArguments := strings.Split(optionValue, " ")
-		arguments = append(arguments, optionArguments...)
+		arguments = append(arguments, optionValues...)
 	}
 
 	return arguments, nil
 }
 
 type optionCliSerializable interface {
-	toCli() (string, error)
+	toCli() ([]string, error)
 }

openvpn/serializer_cli_test.go

@@ -19,6 +19,7 @@ package openvpn
 
 import (
 	"github.com/stretchr/testify/assert"
+	"path/filepath"
 	"testing"
 )
 
@@ -28,20 +29,38 @@ func TestConfigToArguments(t *testing.T) {
 		OptionFlag("flag"),
 		OptionFlag("spacy flag"),
 		OptionParam("value", "1234"),
-		OptionParam("very-value", "1234 5678"),
-		OptionParam("spacy value", "1234 5678"),
+		OptionParam("very-value", "1234", "5678"),
+		OptionParam("spacy value", "1234", "5678"),
 	)
 
 	arguments, err := config.ConfigToArguments()
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.Equal(t,
 		[]string{
 			"--flag",
-			"--spacy", "flag",
+			"--spacy flag",
 			"--value", "1234",
 			"--very-value", "1234", "5678",
-			"--spacy", "value", "1234", "5678",
+			"--spacy value", "1234", "5678",
 		},
 		arguments,
 	)
 }
+
+func TestSpacedValuesArePassedAsSingleArg(t *testing.T) {
+	config := Config{}
+	config.AddOptions(
+		OptionParam("value1", "with spaces"),
+		OptionFile("value2", "file content", filepath.Join("testdataoutput", "name with spaces.txt")),
+	)
+	args, err := config.ConfigToArguments()
+	assert.NoError(t, err)
+	assert.Equal(
+		t,
+		[]string{
+			"--value1", "with spaces",
+			"--value2", "testdataoutput/name with spaces.txt",
+		},
+		args,
+	)
+}

openvpn/server_config.go

@@ -23,7 +23,7 @@ type ServerConfig struct {
 
 func (c *ServerConfig) SetServerMode(port int, network, netmask string) {
 	c.SetPort(port)
-	c.setParam("server", network+" "+netmask)
+	c.setParam("server", network, netmask)
 	c.setParam("topology", "subnet")
 }