Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(env): parsing --env incorrect in cli #19560

Merged
merged 1 commit into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 50 additions & 12 deletions pkg/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,28 +102,22 @@ func ParseFile(path string) (_ map[string]string, err error) {

// replace all \r\n and \r with \n
text := strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(string(content))
if err := parseEnv(env, text, false); err != nil {
if err := parseEnv(env, text); err != nil {
return nil, err
}

return env, nil
}

// parseEnv parse the given content into env format
// @param enforceMatch bool "it throws an error if there is no match"
//
// @example: parseEnv(env, "#comment", true) => error("invalid variable: #comment")
// @example: parseEnv(env, "#comment", false) => nil
// @example: parseEnv(env, "", false) => nil
// @example: parseEnv(env, "KEY=FOO", true) => nil
// @example: parseEnv(env, "KEY", true) => nil
func parseEnv(env map[string]string, content string, enforceMatch bool) error {
// @example: parseEnv(env, "#comment") => nil
// @example: parseEnv(env, "") => nil
// @example: parseEnv(env, "KEY=FOO") => nil
// @example: parseEnv(env, "KEY") => nil
func parseEnv(env map[string]string, content string) error {
m := envMatch(content)

if len(m) == 0 && enforceMatch {
return fmt.Errorf("invalid variable: %q", content)
}

for _, match := range m {
key := match[1]
separator := strings.Trim(match[2], whiteSpaces)
Expand Down Expand Up @@ -195,3 +189,47 @@ func envMatch(content string) [][]string {

return m
}

// parseEnvWithSlice parsing a set of Env variables from a slice of strings
// because the majority of shell interpreters discard double quotes and single quotes,
// for example: podman run -e K='V', when passed into a program, it will become: K=V.
// This can lead to unexpected issues, as discussed in this link: https://github.com/containers/podman/pull/19096#issuecomment-1670164724.
//
// parseEnv method will discard all comments (#) that are not wrapped in quotation marks,
// so it cannot be used to parse env variables obtained from the command line.
//
// @example: parseEnvWithSlice(env, "KEY=FOO") => KEY: FOO
// @example: parseEnvWithSlice(env, "KEY") => KEY: ""
// @example: parseEnvWithSlice(env, "KEY=") => KEY: ""
// @example: parseEnvWithSlice(env, "KEY=FOO=BAR") => KEY: FOO=BAR
// @example: parseEnvWithSlice(env, "KEY=FOO#BAR") => KEY: FOO#BAR
func parseEnvWithSlice(env map[string]string, content string) error {
data := strings.SplitN(content, "=", 2)

// catch invalid variables such as "=" or "=A"
if data[0] == "" {
return fmt.Errorf("invalid variable: %q", content)
}
// trim the front of a variable, but nothing else
name := strings.TrimLeft(data[0], whiteSpaces)
if len(data) > 1 {
env[name] = data[1]
} else {
if strings.HasSuffix(name, "*") {
name = strings.TrimSuffix(name, "*")
for _, e := range os.Environ() {
part := strings.SplitN(e, "=", 2)
if len(part) < 2 {
continue
}
if strings.HasPrefix(part[0], name) {
env[part[0]] = part[1]
}
}
} else if val, ok := os.LookupEnv(name); ok {
// if only a pass-through variable is given, clean it up.
env[name] = val
}
}
return nil
}
99 changes: 55 additions & 44 deletions pkg/env/env_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,79 +323,90 @@ func Test_ParseFile(t *testing.T) {
}
}

func Test_parseEnv(t *testing.T) {
good := make(map[string]string)

type args struct {
env map[string]string
line string
func Test_parseEnvWithSlice(t *testing.T) {
type result struct {
key string
value string
hasErr bool
}

tests := []struct {
name string
args args
wantErr bool
name string
line string
want result
}{
{
name: "Good",
args: args{
env: good,
line: "apple=red",
line: "apple=red",
want: result{
key: "apple",
value: "red",
},
wantErr: false,
},
{
name: "GoodNoValue",
args: args{
env: good,
line: "apple=",
name: "NoValue",
line: "google=",
want: result{
key: "google",
value: "",
},
wantErr: false,
},
{
name: "GoodNoKeyNoValue",
args: args{
env: good,
line: "=",
name: "OnlyKey",
line: "redhat",
want: result{
key: "redhat",
value: "",
},
wantErr: true,
},
{
name: "GoodOnlyKey",
args: args{
env: good,
line: "apple",
name: "NoKey",
line: "=foobar",
want: result{
hasErr: true,
},
wantErr: false,
},
{
name: "BadNoKey",
args: args{
env: good,
line: "=foobar",
name: "OnlyDelim",
line: "=",
want: result{
hasErr: true,
},
wantErr: true,
},
{
name: "BadOnlyDelim",
args: args{
env: good,
line: "=",
name: "Has#",
line: "facebook=red#blue",
want: result{
key: "facebook",
value: "red#blue",
},
},
{
name: "Has\\n",
// twitter="foo
// bar"
line: "twitter=\"foo\nbar\"",
want: result{
key: "twitter",
value: `"foo` + "\n" + `bar"`,
},
wantErr: true,
},
{
name: "MultilineWithBackticksQuotes",
BlackHole1 marked this conversation as resolved.
Show resolved Hide resolved
args: args{
env: good,
line: "apple=`foo\nbar`",
line: "github=`First line\nlast line`",
want: result{
key: "github",
value: "`First line\nlast line`",
},
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := parseEnv(tt.args.env, tt.args.line, true); (err != nil) != tt.wantErr {
t.Errorf("parseEnv() error = %v, wantErr %v", err, tt.wantErr)
envs := make(map[string]string)
if err := parseEnvWithSlice(envs, tt.line); (err != nil) != tt.want.hasErr {
t.Errorf("parseEnv() error = %v, want has Err %v", err, tt.want.hasErr)
} else if envs[tt.want.key] != tt.want.value {
t.Errorf("parseEnv() result = map[%v:%v], but got %v", tt.want.key, tt.want.value, envs)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/env/env_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ package env
func ParseSlice(s []string) (map[string]string, error) {
env := make(map[string]string, len(s))
for _, e := range s {
if err := parseEnv(env, e, true); err != nil {
if err := parseEnvWithSlice(env, e); err != nil {
return nil, err
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/env/env_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func ParseSlice(s []string) (map[string]string, error) {
continue
}

if err := parseEnv(env, e, true); err != nil {
if err := parseEnvWithSlice(env, e); err != nil {
return nil, err
}
}
Expand Down