From 7335544fd062de71454a9cd45ff664e2407206e6 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Mon, 28 Sep 2015 20:26:20 +0200 Subject: [PATCH] Revert environment regexp from 13694 Signed-off-by: Vincent Demeester --- opts/envfile.go | 28 +++++++++++++++++----------- opts/envfile_test.go | 8 ++++++-- opts/opts.go | 8 ++++---- opts/opts_test.go | 44 ++++++++++++++++---------------------------- 4 files changed, 43 insertions(+), 45 deletions(-) diff --git a/opts/envfile.go b/opts/envfile.go index a1cfbae5e4..ba8b4f2016 100644 --- a/opts/envfile.go +++ b/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 diff --git a/opts/envfile_test.go b/opts/envfile_test.go index b82435155a..a172267b5b 100644 --- a/opts/envfile_test.go +++ b/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()) } diff --git a/opts/opts.go b/opts/opts.go index 330e6b250d..6b44e8d801 100644 --- a/opts/opts.go +++ b/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 } diff --git a/opts/opts_test.go b/opts/opts_test.go index 48b21f3442..237b470b95 100644 --- a/opts/opts_test.go +++ b/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)