Skip to content

Commit

Permalink
fix(env): parsing --env incorrect in cli
Browse files Browse the repository at this point in the history
Signed-off-by: Black-Hole1 <bh@bugs.cc>
  • Loading branch information
BlackHole1 committed Aug 9, 2023
1 parent 6f92c59 commit 7ce654f
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 58 deletions.
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",
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

0 comments on commit 7ce654f

Please sign in to comment.