Skip to content

Commit

Permalink
unix: fix swapping fds order when spawning processes
Browse files Browse the repository at this point in the history
Alternative implementation (and test) to
libuv#226

Fixes: joyent/libuv#1084
  • Loading branch information
saghul committed Apr 7, 2015
1 parent 3f3a790 commit a7bbcc3
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/unix/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,17 @@ static void uv__process_child_init(const uv_process_options_t* options,
if (options->flags & UV_PROCESS_DETACHED)
setsid();

/* First duplicate low numbered fds, since it's not safe to duplicate them,
* they could get replaced. Example: swapping stdout and stderr; without
* this fd 2 (stderr) would be duplicated into fd 1, thus making both
* stdout and stderr go to the same fd, which was not the intention. */
for (fd = 0; fd < stdio_count; fd++) {
use_fd = pipes[fd][1];
if (use_fd >= fd)
continue;
pipes[fd][1] = fcntl(use_fd, F_DUPFD, stdio_count);

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Apr 10, 2015

What happens when the fcntl call fails?

}

for (fd = 0; fd < stdio_count; fd++) {
close_fd = pipes[fd][0];
use_fd = pipes[fd][1];
Expand Down
2 changes: 2 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ TEST_DECLARE (spawn_stdout_to_file)
TEST_DECLARE (spawn_stdout_and_stderr_to_file)
#ifndef _WIN32
TEST_DECLARE (spawn_stdout_and_stderr_to_file2)
TEST_DECLARE (spawn_stdout_and_stderr_to_file_swap)
#endif
TEST_DECLARE (spawn_auto_unref)
TEST_DECLARE (spawn_closed_process_io)
Expand Down Expand Up @@ -578,6 +579,7 @@ TASK_LIST_START
TEST_ENTRY (spawn_stdout_and_stderr_to_file)
#ifndef _WIN32
TEST_ENTRY (spawn_stdout_and_stderr_to_file2)
TEST_ENTRY (spawn_stdout_and_stderr_to_file_swap)
#endif
TEST_ENTRY (spawn_auto_unref)
TEST_ENTRY (spawn_closed_process_io)
Expand Down
84 changes: 84 additions & 0 deletions test/test-spawn.c
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,90 @@ TEST_IMPL(spawn_stdout_and_stderr_to_file2) {
MAKE_VALGRIND_HAPPY();
return 0;
}


TEST_IMPL(spawn_stdout_and_stderr_to_file_swap) {
int r;
uv_file stdout_file;
uv_file stderr_file;
uv_fs_t fs_req;
uv_stdio_container_t stdio[3];
uv_buf_t buf;

/* Setup. */
unlink("stdout_file");
unlink("stderr_file");

init_process_options("spawn_helper6", exit_cb);

/* open 'stdout_file' and replace STDOUT_FILENO with it */
r = uv_fs_open(uv_default_loop(), &fs_req, "stdout_file", O_CREAT | O_RDWR,
S_IRUSR | S_IWUSR, NULL);
ASSERT(r != -1);
uv_fs_req_cleanup(&fs_req);
stdout_file = dup2(r, STDOUT_FILENO);
ASSERT(stdout_file != -1);

/* open 'stderr_file' and replace STDERR_FILENO with it */
r = uv_fs_open(uv_default_loop(), &fs_req, "stderr_file", O_CREAT | O_RDWR,
S_IRUSR | S_IWUSR, NULL);
ASSERT(r != -1);
uv_fs_req_cleanup(&fs_req);
stderr_file = dup2(r, STDERR_FILENO);
ASSERT(stderr_file != -1);

/* now we're going to swap them: the child process' stdout will be our
* stderr_file and vice versa */
options.stdio = stdio;
options.stdio[0].flags = UV_IGNORE;
options.stdio[1].flags = UV_INHERIT_FD;
options.stdio[1].data.fd = stderr_file;
options.stdio[2].flags = UV_INHERIT_FD;
options.stdio[2].data.fd = stdout_file;
options.stdio_count = 3;

r = uv_spawn(uv_default_loop(), &process, &options);
ASSERT(r == 0);

r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
ASSERT(r == 0);

ASSERT(exit_cb_called == 1);
ASSERT(close_cb_called == 1);

buf = uv_buf_init(output, sizeof(output));

/* check the content of stdout_file */
r = uv_fs_read(uv_default_loop(), &fs_req, stdout_file, &buf, 1, 0, NULL);
ASSERT(r >= 15);
uv_fs_req_cleanup(&fs_req);

r = uv_fs_close(uv_default_loop(), &fs_req, stdout_file, NULL);
ASSERT(r == 0);
uv_fs_req_cleanup(&fs_req);

printf("output is: %s", output);
ASSERT(strncmp("hello errworld\n", output, 15) == 0);

/* check the content of stderr_file */
r = uv_fs_read(uv_default_loop(), &fs_req, stderr_file, &buf, 1, 0, NULL);
ASSERT(r >= 12);
uv_fs_req_cleanup(&fs_req);

r = uv_fs_close(uv_default_loop(), &fs_req, stderr_file, NULL);
ASSERT(r == 0);
uv_fs_req_cleanup(&fs_req);

printf("output is: %s", output);
ASSERT(strncmp("hello world\n", output, 12) == 0);

/* Cleanup. */
unlink("stdout_file");
unlink("stderr_file");

MAKE_VALGRIND_HAPPY();
return 0;
}

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Apr 10, 2015

My comments from the previous commit apply here as well. :-)

#endif


Expand Down

0 comments on commit a7bbcc3

Please sign in to comment.