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.

Sign up for GitHub

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

Allow podman to run in an environment with keys containing spaces #15434

Merged
merged 1 commit into from
Aug 24, 2022
Merged

Allow podman to run in an environment with keys containing spaces #15434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 16 additions & 4 deletions pkg/env/env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ func Slice(m map[string]string) []string {
return env
}

// Map transforms the specified slice of environment variables into a
// map.
func Map(slice []string) map[string]string {
envmap := make(map[string]string, len(slice))
for _, val := range slice {
data := strings.SplitN(val, "=", 2)

if len(data) > 1 {
envmap[data[0]] = data[1]
} else {
envmap[data[0]] = ""
}
}
return envmap
}

// Join joins the two environment maps with override overriding base.
func Join(base map[string]string, override map[string]string) map[string]string {
if len(base) == 0 {
Expand Down Expand Up @@ -87,10 +103,6 @@ func parseEnv(env map[string]string, line string) error {
}
// trim the front of a variable, but nothing else
name := strings.TrimLeft(data[0], whiteSpaces)
if strings.ContainsAny(name, whiteSpaces) {
return fmt.Errorf("name %q has white spaces, poorly formatted name", name)
}

if len(data) > 1 {
env[name] = data[1]
} else {
Expand Down
6 changes: 2 additions & 4 deletions pkg/specgen/generate/container.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,8 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
}
// First transform the os env into a map. We need it for the labels later in
// any case.
osEnv, err := envLib.ParseSlice(os.Environ())
if err != nil {
return nil, fmt.Errorf("error parsing host environment variables: %w", err)
}
osEnv := envLib.Map(os.Environ())

// Caller Specified defaults
if s.EnvHost {
defaultEnvs = envLib.Join(defaultEnvs, osEnv)
Expand Down
5 changes: 1 addition & 4 deletions pkg/specgenutil/specgen.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,10 +362,7 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions

// First transform the os env into a map. We need it for the labels later in
// any case.
osEnv, err := envLib.ParseSlice(os.Environ())
if err != nil {
return fmt.Errorf("error parsing host environment variables: %w", err)
}
osEnv := envLib.Map(os.Environ())

if !s.EnvHost {
s.EnvHost = c.EnvHost
Expand Down
7 changes: 7 additions & 0 deletions test/e2e/run_env_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ var _ = Describe("Podman run", func() {
Expect(session).Should(Exit(0))
Expect(session.OutputToString()).To(ContainSubstring("/bin"))

// Verify environ keys with spaces do not blow up podman command
os.Setenv("FOO BAR", "BAZ")
session = podmanTest.Podman([]string{"run", "--rm", ALPINE, "true"})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not go for a full-on all-the-way test?

    ...{"run", "--rm", "-e", "FOO BAR", ALPINE, "printenv", "FOO BAR"})
   ...output-should-equal("BAZ")

(for extra credit, s/BAZ/BAZ OOKA or something with spaces/)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do that but that would be a separate test. What I was looking at was a failure to run if any environment variable contained a space, even if the variable was not being passed to the container.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also did not see an easy way to do this in a system test.

env "foo bar"="foo baz" run_podman ...

Blew up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using --env-host work?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a sane way to test this in system tests either. Bash won't allow it. It would be possible to build a test binary that uses prctl() to inject FOO BAR into the parent process's environ, but let's just not go there.

session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
os.Unsetenv("FOO BAR")

os.Setenv("FOO", "BAR")
session = podmanTest.Podman([]string{"run", "--rm", "--env", "FOO", ALPINE, "printenv", "FOO"})
session.WaitWithDefaultTimeout()
Expand Down