Revert environment regexp from 13694

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
This commit is contained in:
Vincent Demeester 2015-09-28 20:26:20 +02:00
parent 863be38db7
commit aca36e11f2
4 changed files with 43 additions and 45 deletions

View File

@ -4,18 +4,22 @@ import (
"bufio" "bufio"
"fmt" "fmt"
"os" "os"
"regexp"
"strings" "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 // 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) { func ParseEnvFile(filename string) ([]string, error) {
fh, err := os.Open(filename) fh, err := os.Open(filename)
if err != nil { if err != nil {
@ -31,11 +35,13 @@ func ParseEnvFile(filename string) ([]string, error) {
// line is not empty, and not starting with '#' // line is not empty, and not starting with '#'
if len(line) > 0 && !strings.HasPrefix(line, "#") { if len(line) > 0 && !strings.HasPrefix(line, "#") {
data := strings.SplitN(line, "=", 2) data := strings.SplitN(line, "=", 2)
variable := data[0]
if !EnvironmentVariableRegexp.MatchString(variable) { // trim the front of a variable, but nothing else
return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' is not a valid environment variable", variable)} 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 { if len(data) > 1 {
// pass the value through, no trimming // pass the value through, no trimming

View File

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

View File

@ -256,16 +256,16 @@ func validatePath(val string, validator func(string) bool) (string, error) {
} }
// ValidateEnv validates an environment variable and returns it. // 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. // 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) { func ValidateEnv(val string) (string, error) {
arr := strings.Split(val, "=") arr := strings.Split(val, "=")
if len(arr) > 1 { if len(arr) > 1 {
return val, nil 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) { if !doesEnvExist(val) {
return val, nil return val, nil
} }

View File

@ -377,35 +377,23 @@ func TestValidateDevice(t *testing.T) {
} }
func TestValidateEnv(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{ valids := map[string]string{
"a": "a", "a": "a",
"something": "something", "something": "something",
"_=a": "_=a", "_=a": "_=a",
"env1=value1": "env1=value1", "env1=value1": "env1=value1",
"_env1=value1": "_env1=value1", "_env1=value1": "_env1=value1",
"env2=value2=value3": "env2=value2=value3", "env2=value2=value3": "env2=value2=value3",
"env3=abc!qwe": "env3=abc!qwe", "env3=abc!qwe": "env3=abc!qwe",
"env_4=value 4": "env_4=value 4", "env_4=value 4": "env_4=value 4",
"PATH": fmt.Sprintf("PATH=%v", os.Getenv("PATH")), "PATH": fmt.Sprintf("PATH=%v", os.Getenv("PATH")),
"PATH=something": "PATH=something", "PATH=something": "PATH=something",
} "asd!qwe": "asd!qwe",
for value, expectedError := range invalids { "1asd": "1asd",
_, err := ValidateEnv(value) "123": "123",
if err == nil { "some space": "some space",
t.Fatalf("Expected ErrBadEnvVariable, got nothing") " some space before": " some space before",
} "some space after ": "some space after ",
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())
}
} }
for value, expected := range valids { for value, expected := range valids {
actual, err := ValidateEnv(value) actual, err := ValidateEnv(value)