Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

If `options.env` is not provided, do not replace the child's environ #612

Closed
wants to merge 2 commits into from

3 participants

@AvianFlu

Currently, setting options.env to NULL for uv_spawn results in a child process for whom environ == NULL. This can cause a wide variety of unexpected issues - this patch is a simple fix, and a test for the issue.

AvianFlu added some commits
@AvianFlu AvianFlu unix: do not set environ unless one is provided
Currently, `uv_spawn` will set `environ` to the value of `options.env`, even if
`options.env` is `NULL`.  This results in child processes for whom `environ ==
NULL`, which can cause a variety of unexpected issues.
5b9b3a7
@AvianFlu AvianFlu unix: test: Add test for child process env vars
Test if the env is preserved when `options.env` is explicitly set to NULL.
7ac4649
@bnoordhuis

Thanks Charlie, landed in 1d85815.

@bnoordhuis bnoordhuis closed this
@bnoordhuis

And in fb64948 in v0.8.

@piscisaureus
Collaborator

You broke the windows build guys. Well done!

@bnoordhuis

Why? setenv?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 1, 2012
  1. @AvianFlu

    unix: do not set environ unless one is provided

    AvianFlu authored
    Currently, `uv_spawn` will set `environ` to the value of `options.env`, even if
    `options.env` is `NULL`.  This results in child processes for whom `environ ==
    NULL`, which can cause a variety of unexpected issues.
Commits on Nov 2, 2012
  1. @AvianFlu

    unix: test: Add test for child process env vars

    AvianFlu authored
    Test if the env is preserved when `options.env` is explicitly set to NULL.
This page is out of date. Refresh to see the latest.
View
4 src/unix/process.c
@@ -340,7 +340,9 @@ static void uv__process_child_init(uv_process_options_t options,
_exit(127);
}
- environ = options.env;
+ if (options.env) {
+ environ = options.env;
+ }
execvp(options.file, options.args);
uv__write_int(error_fd, errno);
View
13 test/run-tests.c
@@ -134,5 +134,18 @@ static int maybe_run_test(int argc, char **argv) {
return 1;
}
+ if (strcmp(argv[1], "spawn_helper7") == 0) {
+ int r;
+ char *test;
+ /* Test if the test value from the parent is still set */
+ test = getenv("ENV_TEST");
+ ASSERT(test != NULL);
+
+ r = fprintf(stdout, "%s", test);
+ ASSERT(r > 0);
+
+ return 1;
+ }
+
return run_test(argv[1], TEST_TIMEOUT, 0);
}
View
2  test/test-list.h
@@ -146,6 +146,7 @@ TEST_DECLARE (spawn_and_kill)
TEST_DECLARE (spawn_detached)
TEST_DECLARE (spawn_and_kill_with_std)
TEST_DECLARE (spawn_and_ping)
+TEST_DECLARE (spawn_preserve_env)
TEST_DECLARE (spawn_setuid_fails)
TEST_DECLARE (spawn_setgid_fails)
TEST_DECLARE (spawn_stdout_to_file)
@@ -395,6 +396,7 @@ TASK_LIST_START
TEST_ENTRY (spawn_detached)
TEST_ENTRY (spawn_and_kill_with_std)
TEST_ENTRY (spawn_and_ping)
+ TEST_ENTRY (spawn_preserve_env)
TEST_ENTRY (spawn_setuid_fails)
TEST_ENTRY (spawn_setgid_fails)
TEST_ENTRY (spawn_stdout_to_file)
View
37 test/test-spawn.c
@@ -387,6 +387,43 @@ TEST_IMPL(spawn_and_kill) {
return 0;
}
+TEST_IMPL(spawn_preserve_env) {
+ int r;
+ uv_pipe_t out;
+ uv_stdio_container_t stdio[2];
+
+ init_process_options("spawn_helper7", exit_cb);
+
+ uv_pipe_init(uv_default_loop(), &out, 0);
+ options.stdio = stdio;
+ options.stdio[0].flags = UV_IGNORE;
+ options.stdio[1].flags = UV_CREATE_PIPE | UV_WRITABLE_PIPE;
+ options.stdio[1].data.stream = (uv_stream_t*)&out;
+ options.stdio_count = 2;
+
+ ASSERT(setenv("ENV_TEST", "testval", 1) == 0);
+ /* Explicitly set options.env to NULL to test for env clobbering. */
+ options.env = NULL;
+
+ r = uv_spawn(uv_default_loop(), &process, options);
+ ASSERT(r == 0);
+
+ r = uv_read_start((uv_stream_t*) &out, on_alloc, on_read);
+ ASSERT(r == 0);
+
+ r = uv_run(uv_default_loop());
+ ASSERT(r == 0);
+
+ ASSERT(exit_cb_called == 1);
+ ASSERT(close_cb_called == 2);
+
+ printf("output is: %s", output);
+ ASSERT(strcmp("testval", output) == 0);
+
+ MAKE_VALGRIND_HAPPY();
+ return 0;
+}
+
TEST_IMPL(spawn_detached) {
int r;
uv_err_t err;
Something went wrong with that request. Please try again.