From 7ce654fea39195fae9f7f8d2cb1112b731e67fc3 Mon Sep 17 00:00:00 2001 From: Black-Hole1 Date: Wed, 9 Aug 2023 13:04:41 +0800 Subject: [PATCH] fix(env): parsing --env incorrect in cli Signed-off-by: Black-Hole1 --- pkg/env/env.go | 62 +++++++++++++++++++++----- pkg/env/env_test.go | 99 +++++++++++++++++++++++------------------- pkg/env/env_unix.go | 2 +- pkg/env/env_windows.go | 2 +- 4 files changed, 107 insertions(+), 58 deletions(-) diff --git a/pkg/env/env.go b/pkg/env/env.go index 80b13ed684..ad6b331c6d 100644 --- a/pkg/env/env.go +++ b/pkg/env/env.go @@ -102,7 +102,7 @@ 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 } @@ -110,20 +110,14 @@ func ParseFile(path string) (_ map[string]string, err error) { } // 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) @@ -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 +} diff --git a/pkg/env/env_test.go b/pkg/env/env_test.go index 1dc2b10eef..701663c486 100644 --- a/pkg/env/env_test.go +++ b/pkg/env/env_test.go @@ -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) } }) } diff --git a/pkg/env/env_unix.go b/pkg/env/env_unix.go index 7fa3d59c91..b514917186 100644 --- a/pkg/env/env_unix.go +++ b/pkg/env/env_unix.go @@ -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 } } diff --git a/pkg/env/env_windows.go b/pkg/env/env_windows.go index b9e4f4c589..f3eb4afce7 100644 --- a/pkg/env/env_windows.go +++ b/pkg/env/env_windows.go @@ -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 } }