Merge pull request #19560 from BlackHole1/fix

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

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
+}

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)
 			}
 		})
 	}

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
 		}
 	}

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
 		}
 	}