Merge pull request #16608 from vdemeester/16585-revert-env-regexp

Revert environment regexp in opts

opts/envfile.go

@@ -4,18 +4,22 @@ import (
 	"bufio"
 	"fmt"
 	"os"
-	"regexp"
 	"strings"
 )
 
-var (
-	// EnvironmentVariableRegexp is a regexp to validate correct environment variables
-	// Environment variables set by the user must have a name consisting solely of
-	// alphabetics, numerics, and underscores - the first of which must not be numeric.
-	EnvironmentVariableRegexp = regexp.MustCompile("^[[:alpha:]_][[:alpha:][:digit:]_]*$")
-)
-
 // ParseEnvFile reads a file with environment variables enumerated by lines
+//
+// ``Environment variable names used by the utilities in the Shell and
+// Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase
+// letters, digits, and the '_' (underscore) from the characters defined in
+// Portable Character Set and do not begin with a digit. *But*, other
+// characters may be permitted by an implementation; applications shall
+// tolerate the presence of such names.''
+// -- http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html
+//
+// As of #16585, it's up to application inside docker to validate or not
+// environment variables, that's why we just strip leading whitespace and
+// nothing more.
 func ParseEnvFile(filename string) ([]string, error) {
 	fh, err := os.Open(filename)
 	if err != nil {
@@ -31,11 +35,13 @@ func ParseEnvFile(filename string) ([]string, error) {
 		// line is not empty, and not starting with '#'
 		if len(line) > 0 && !strings.HasPrefix(line, "#") {
 			data := strings.SplitN(line, "=", 2)
-			variable := data[0]
 
-			if !EnvironmentVariableRegexp.MatchString(variable) {
-				return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' is not a valid environment variable", variable)}
+			// trim the front of a variable, but nothing else
+			variable := strings.TrimLeft(data[0], whiteSpaces)
+			if strings.ContainsAny(variable, whiteSpaces) {
+				return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' has white spaces", variable)}
 			}
+
 			if len(data) > 1 {
 
 				// pass the value through, no trimming

opts/envfile_test.go

@@ -28,6 +28,8 @@ func TestParseEnvFileGoodFile(t *testing.T) {
 # comment
 
 _foobar=foobaz
+with.dots=working
+and_underscore=working too
 `
 	// Adding a newline + a line with pure whitespace.
 	// This is being done like this instead of the block above
@@ -47,6 +49,8 @@ _foobar=foobaz
 		"foo=bar",
 		"baz=quux",
 		"_foobar=foobaz",
+		"with.dots=working",
+		"and_underscore=working too",
 	}
 
 	if !reflect.DeepEqual(lines, expectedLines) {
@@ -96,7 +100,7 @@ func TestParseEnvFileBadlyFormattedFile(t *testing.T) {
 	if _, ok := err.(ErrBadEnvVariable); !ok {
 		t.Fatalf("Expected a ErrBadEnvVariable, got [%v]", err)
 	}
-	expectedMessage := "poorly formatted environment: variable 'f   ' is not a valid environment variable"
+	expectedMessage := "poorly formatted environment: variable 'f   ' has white spaces"
 	if err.Error() != expectedMessage {
 		t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error())
 	}
@@ -131,7 +135,7 @@ another invalid line`
 	if _, ok := err.(ErrBadEnvVariable); !ok {
 		t.Fatalf("Expected a ErrBadEnvvariable, got [%v]", err)
 	}
-	expectedMessage := "poorly formatted environment: variable 'first line' is not a valid environment variable"
+	expectedMessage := "poorly formatted environment: variable 'first line' has white spaces"
 	if err.Error() != expectedMessage {
 		t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error())
 	}

opts/opts.go

@@ -256,16 +256,16 @@ func validatePath(val string, validator func(string) bool) (string, error) {
 }
 
 // ValidateEnv validates an environment variable and returns it.
-// It uses EnvironmentVariableRegexp to ensure the name of the environment variable is valid.
 // If no value is specified, it returns the current value using os.Getenv.
+//
+// As on ParseEnvFile and related to #16585, environment variable names
+// are not validate what so ever, it's up to application inside docker
+// to validate them or not.
 func ValidateEnv(val string) (string, error) {
 	arr := strings.Split(val, "=")
 	if len(arr) > 1 {
 		return val, nil
 	}
-	if !EnvironmentVariableRegexp.MatchString(arr[0]) {
-		return val, ErrBadEnvVariable{fmt.Sprintf("variable '%s' is not a valid environment variable", val)}
-	}
 	if !doesEnvExist(val) {
 		return val, nil
 	}

opts/opts_test.go

@@ -377,35 +377,23 @@ func TestValidateDevice(t *testing.T) {
 }
 
 func TestValidateEnv(t *testing.T) {
-	invalids := map[string]string{
-		"some spaces": "poorly formatted environment: variable 'some spaces' is not a valid environment variable",
-		"asd!qwe":     "poorly formatted environment: variable 'asd!qwe' is not a valid environment variable",
-		"1asd":        "poorly formatted environment: variable '1asd' is not a valid environment variable",
-		"123":         "poorly formatted environment: variable '123' is not a valid environment variable",
-	}
 	valids := map[string]string{
-		"a":                  "a",
-		"something":          "something",
-		"_=a":                "_=a",
-		"env1=value1":        "env1=value1",
-		"_env1=value1":       "_env1=value1",
-		"env2=value2=value3": "env2=value2=value3",
-		"env3=abc!qwe":       "env3=abc!qwe",
-		"env_4=value 4":      "env_4=value 4",
-		"PATH":               fmt.Sprintf("PATH=%v", os.Getenv("PATH")),
-		"PATH=something":     "PATH=something",
-	}
-	for value, expectedError := range invalids {
-		_, err := ValidateEnv(value)
-		if err == nil {
-			t.Fatalf("Expected ErrBadEnvVariable, got nothing")
-		}
-		if _, ok := err.(ErrBadEnvVariable); !ok {
-			t.Fatalf("Expected ErrBadEnvVariable, got [%s]", err)
-		}
-		if err.Error() != expectedError {
-			t.Fatalf("Expected ErrBadEnvVariable with message [%s], got [%s]", expectedError, err.Error())
-		}
+		"a":                   "a",
+		"something":           "something",
+		"_=a":                 "_=a",
+		"env1=value1":         "env1=value1",
+		"_env1=value1":        "_env1=value1",
+		"env2=value2=value3":  "env2=value2=value3",
+		"env3=abc!qwe":        "env3=abc!qwe",
+		"env_4=value 4":       "env_4=value 4",
+		"PATH":                fmt.Sprintf("PATH=%v", os.Getenv("PATH")),
+		"PATH=something":      "PATH=something",
+		"asd!qwe":             "asd!qwe",
+		"1asd":                "1asd",
+		"123":                 "123",
+		"some space":          "some space",
+		"  some space before": "  some space before",
+		"some space after  ":  "some space after  ",
 	}
 	for value, expected := range valids {
 		actual, err := ValidateEnv(value)