From fb649487465d8ed5504302649b69fc72c879acc1 Mon Sep 17 00:00:00 2001 From: Charlie McConnell Date: Thu, 1 Nov 2012 10:55:06 -0400 Subject: [PATCH 1/5] 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. This is a back-port of commit 1d85815 from the master branch. --- src/unix/process.c | 4 +++- test/run-tests.c | 13 +++++++++++++ test/test-list.h | 2 ++ test/test-spawn.c | 38 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) diff --git a/src/unix/process.c b/src/unix/process.c index 40330640..9b174795 100644 --- a/src/unix/process.c +++ b/src/unix/process.c @@ -260,7 +260,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); perror("execvp()"); diff --git a/test/run-tests.c b/test/run-tests.c index fb163037..d0e64f05 100644 --- a/test/run-tests.c +++ b/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); } diff --git a/test/test-list.h b/test/test-list.h index 19670e56..d3772588 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -133,6 +133,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) @@ -362,6 +363,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) diff --git a/test/test-spawn.c b/test/test-spawn.c index 51ed2e59..3fd1f266 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -373,6 +373,44 @@ 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); + + return 0; +} + + TEST_IMPL(spawn_detached) { int r; uv_err_t err; From 4c9e42d0e6462597aedd1cce885394cf0a2e4edf Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Mon, 5 Nov 2012 22:11:22 +0100 Subject: [PATCH 2/5] windows: un-break the build It was broken in fb64948. --- test/test-spawn.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/test-spawn.c b/test/test-spawn.c index 3fd1f266..f383d652 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -388,7 +388,13 @@ TEST_IMPL(spawn_preserve_env) { options.stdio[1].data.stream = (uv_stream_t*) &out; options.stdio_count = 2; - ASSERT(setenv("ENV_TEST", "testval", 1) == 0); +#ifdef _WIN32 + r = putenv("ENV_TEST=testval"); +#else + r = setenv("ENV_TEST", "testval", 1); +#endif + ASSERT(r == 0); + /* Explicitly set options.env to NULL to test for env clobbering. */ options.env = NULL; From 2affa60e19f6fa56166f63003285156a2ff0dfc0 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 6 Nov 2012 00:31:16 +0100 Subject: [PATCH 3/5] test: remove unnecessary #ifdef _WIN32 This is a back-port of commit 4d17337 from the master branch. --- test/test-spawn.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/test-spawn.c b/test/test-spawn.c index f383d652..43e80100 100644 --- a/test/test-spawn.c +++ b/test/test-spawn.c @@ -388,11 +388,7 @@ TEST_IMPL(spawn_preserve_env) { options.stdio[1].data.stream = (uv_stream_t*) &out; options.stdio_count = 2; -#ifdef _WIN32 r = putenv("ENV_TEST=testval"); -#else - r = setenv("ENV_TEST", "testval", 1); -#endif ASSERT(r == 0); /* Explicitly set options.env to NULL to test for env clobbering. */ From f372fd4b13cea4c40c82c0544560807ad15e8641 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Wed, 7 Nov 2012 11:10:43 +0100 Subject: [PATCH 4/5] windows: map ERROR_GEN_FAILURE to UV_EIO --- src/win/error.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/win/error.c b/src/win/error.c index 91f5111e..e5fb76a1 100644 --- a/src/win/error.c +++ b/src/win/error.c @@ -74,6 +74,7 @@ uv_err_code uv_translate_sys_error(int sys_errno) { case ERROR_DISK_CORRUPT: return UV_EIO; case ERROR_EOM_OVERFLOW: return UV_EIO; case ERROR_FILEMARK_DETECTED: return UV_EIO; + case ERROR_GEN_FAILURE: return UV_EIO; case ERROR_INVALID_BLOCK_LENGTH: return UV_EIO; case ERROR_IO_DEVICE: return UV_EIO; case ERROR_NO_DATA_DETECTED: return UV_EIO; From deeec421afed1fa44961204d2e0a17134f3ea02a Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 9 Nov 2012 20:20:50 +0100 Subject: [PATCH 5/5] build: use link_settings, fix typo * Use link_settings instead of direct_dependent_settings. * Fix typo: s/ldlags/ldflags/ Pointed out by a certain R.L. Dahl of San Francisco, CA. Fixes #618. --- uv.gyp | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/uv.gyp b/uv.gyp index 96c6b724..f288d92d 100644 --- a/uv.gyp +++ b/uv.gyp @@ -10,11 +10,9 @@ ], 'conditions': [ ['OS=="solaris"', { - 'cflags': ['-pthreads'], - 'ldlags': ['-pthreads'], + 'cflags': [ '-pthreads' ], }, { - 'cflags': ['-pthread'], - 'ldlags': ['-pthread'], + 'cflags': [ '-pthread' ], }], ], }], @@ -32,13 +30,7 @@ ], 'direct_dependent_settings': { 'include_dirs': [ 'include' ], - 'conditions': [ - ['OS=="linux"', { - 'libraries': [ '-ldl' ], - }], - ], }, - 'defines': [ 'HAVE_CONFIG_H' ], @@ -214,12 +206,21 @@ 'src/unix/uv-eio.h', ], 'include_dirs': [ 'src/unix/ev', ], - 'libraries': [ '-lm' ] + 'link_settings': { + 'libraries': [ '-lm' ], + 'conditions': [ + ['OS=="solaris"', { + 'ldflags': [ '-pthreads' ], + }, { + 'ldflags': [ '-pthread' ], + }], + ], + }, }], [ 'OS=="mac"', { 'include_dirs': [ 'src/ares/config_darwin' ], 'sources': [ 'src/unix/darwin.c' ], - 'direct_dependent_settings': { + 'link_settings': { 'libraries': [ '$(SDKROOT)/System/Library/Frameworks/CoreServices.framework', ], @@ -242,8 +243,8 @@ 'EV_CONFIG_H="config_linux.h"', 'EIO_CONFIG_H="config_linux.h"', ], - 'direct_dependent_settings': { - 'libraries': [ '-lrt' ], + 'link_settings': { + 'libraries': [ '-ldl', '-lrt' ], }, }], [ 'OS=="solaris"', { @@ -255,7 +256,7 @@ 'EV_CONFIG_H="config_sunos.h"', 'EIO_CONFIG_H="config_sunos.h"', ], - 'direct_dependent_settings': { + 'link_settings': { 'libraries': [ '-lkstat', '-lsocket', @@ -270,7 +271,7 @@ 'EV_CONFIG_H="config_freebsd.h"', 'EIO_CONFIG_H="config_freebsd.h"', ], - 'direct_dependent_settings': { + 'link_settings': { 'libraries': [ '-lkvm', ],