From 9599589f181520279c28ccaadcf0be257ff728d4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 4 Oct 2023 16:57:30 +0200 Subject: [PATCH] Revert "fix(env): parsing --env incorrect in cli" This reverts commit 7ce654fea39195fae9f7f8d2cb1112b731e67fc3. see https://github.com/containers/podman/issues/19565 Signed-off-by: Paul Holzinger --- 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, 58 insertions(+), 107 deletions(-) diff --git a/pkg/env/env.go b/pkg/env/env.go index ad6b331c6d..80b13ed684 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); err != nil { + if err := parseEnv(env, text, false); err != nil { return nil, err } @@ -110,14 +110,20 @@ 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") => 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 { +// @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 { 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) @@ -189,47 +195,3 @@ 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 701663c486..1dc2b10eef 100644 --- a/pkg/env/env_test.go +++ b/pkg/env/env_test.go @@ -323,90 +323,79 @@ func Test_ParseFile(t *testing.T) { } } -func Test_parseEnvWithSlice(t *testing.T) { - type result struct { - key string - value string - hasErr bool - } +func Test_parseEnv(t *testing.T) { + good := make(map[string]string) - tests := []struct { - name string + type args struct { + env map[string]string line string - want result + } + tests := []struct { + name string + args args + wantErr bool }{ { name: "Good", - line: "apple=red", - want: result{ - key: "apple", - value: "red", + args: args{ + env: good, + line: "apple=red", }, + wantErr: false, }, { - name: "NoValue", - line: "google=", - want: result{ - key: "google", - value: "", + name: "GoodNoValue", + args: args{ + env: good, + line: "apple=", }, + wantErr: false, }, { - name: "OnlyKey", - line: "redhat", - want: result{ - key: "redhat", - value: "", + name: "GoodNoKeyNoValue", + args: args{ + env: good, + line: "=", }, + wantErr: true, }, { - name: "NoKey", - line: "=foobar", - want: result{ - hasErr: true, + name: "GoodOnlyKey", + args: args{ + env: good, + line: "apple", }, + wantErr: false, }, { - name: "OnlyDelim", - line: "=", - want: result{ - hasErr: true, + name: "BadNoKey", + args: args{ + env: good, + line: "=foobar", }, + wantErr: true, }, { - 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"`, + name: "BadOnlyDelim", + args: args{ + env: good, + line: "=", }, + wantErr: true, }, { name: "MultilineWithBackticksQuotes", - line: "github=`First line\nlast line`", - want: result{ - key: "github", - value: "`First line\nlast line`", + args: args{ + env: good, + line: "apple=`foo\nbar`", }, + wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - 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) + if err := parseEnv(tt.args.env, tt.args.line, true); (err != nil) != tt.wantErr { + t.Errorf("parseEnv() error = %v, wantErr %v", err, tt.wantErr) } }) } diff --git a/pkg/env/env_unix.go b/pkg/env/env_unix.go index b514917186..7fa3d59c91 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 := parseEnvWithSlice(env, e); err != nil { + if err := parseEnv(env, e, true); err != nil { return nil, err } } diff --git a/pkg/env/env_windows.go b/pkg/env/env_windows.go index f3eb4afce7..b9e4f4c589 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 := parseEnvWithSlice(env, e); err != nil { + if err := parseEnv(env, e, true); err != nil { return nil, err } }