summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGES7
-rw-r--r--file_io/win32/filedup.c110
-rw-r--r--file_io/win32/open.c39
-rw-r--r--include/apr_thread_proc.h9
-rw-r--r--include/arch/win32/apr_arch_file_io.h6
-rw-r--r--threadproc/win32/proc.c66
6 files changed, 159 insertions, 78 deletions
diff --git a/CHANGES b/CHANGES
index b0bac1cec..02e9b359b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
- -*- coding: utf-8 -*-
+ -*- coding: utf-8 -*-
Changes for APR 1.3.0
+ *) Introduce APR_NO_FILE as an option for any of the three stdio streams
+ to cause the specified streams to be closed to the child process,
+ when the caller has chosen that flag via apr_procattr_io_set().
+ [William Rowe]
+
*) Solve winNT inherited pipe leaks by mutexing apr_proc_create calls,
on WinNT (not WinCE, nor 9x) so that we toggle the inherited state
of the stdin/out/err pipes. All other file handles are treated as
diff --git a/file_io/win32/filedup.c b/file_io/win32/filedup.c
index 964ec680c..70ed4379d 100644
--- a/file_io/win32/filedup.c
+++ b/file_io/win32/filedup.c
@@ -20,6 +20,8 @@
#include "apr_strings.h"
#include <string.h>
#include "apr_arch_inherit.h"
+#include <io.h> /* for [_open/_get]_osfhandle */
+
APR_DECLARE(apr_status_t) apr_file_dup(apr_file_t **new_file,
apr_file_t *old_file, apr_pool_t *p)
@@ -63,10 +65,6 @@ APR_DECLARE(apr_status_t) apr_file_dup(apr_file_t **new_file,
#endif /* !defined(_WIN32_WCE) */
}
-#define stdin_handle 0x01
-#define stdout_handle 0x02
-#define stderr_handle 0x04
-
APR_DECLARE(apr_status_t) apr_file_dup2(apr_file_t *new_file,
apr_file_t *old_file, apr_pool_t *p)
{
@@ -77,32 +75,88 @@ APR_DECLARE(apr_status_t) apr_file_dup2(apr_file_t *new_file,
HANDLE hproc = GetCurrentProcess();
HANDLE newhand = NULL;
apr_int32_t newflags;
+ int fd;
+
+ if (new_file->flags & APR_STD_FLAGS) {
+ /* if (!DuplicateHandle(hproc, old_file->filehand,
+ * hproc, &newhand, 0,
+ * TRUE, DUPLICATE_SAME_ACCESS)) {
+ * return apr_get_os_error();
+ * }
+ * if (((stdhandle & stderr_handle) && !SetStdHandle(STD_ERROR_HANDLE, newhand)) ||
+ * ((stdhandle & stdout_handle) && !SetStdHandle(STD_OUTPUT_HANDLE, newhand)) ||
+ * ((stdhandle & stdin_handle) && !SetStdHandle(STD_INPUT_HANDLE, newhand))) {
+ * return apr_get_os_error();
+ * }
+ * newflags = old_file->flags | APR_INHERIT;
+ */
+
+ if ((new_file->flags & APR_STD_FLAGS) == APR_STDERR_FLAG) {
+ /* Flush stderr and unset its buffer, then commit the fd-based buffer.
+ * This is typically a noop for Win2K/XP since services with NULL std
+ * handles [but valid FILE *'s, oddly enough], but is required
+ * for NT 4.0 and to use this code outside of services.
+ */
+ fflush(stderr);
+ setvbuf(stderr, NULL, _IONBF, 0);
+ _commit(2 /* stderr */);
+
+ /* Clone a handle can _close() without harming the source handle,
+ * open an MSVCRT-based pseudo-fd for the file handle, then dup2
+ * and close our temporary pseudo-fd once it's been duplicated.
+ * This will incidently keep the FILE-based stderr in sync.
+ * Note the apparently redundant _O_BINARY coersions are required.
+ */
+ if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+ 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ return apr_get_os_error();
+ }
+ fd = _open_osfhandle((INT_PTR)newhand, _O_WRONLY | _O_BINARY);
+ _dup2(fd, 2);
+ _close(fd);
+ _setmode(2, _O_BINARY);
+
+ /* hPipeWrite was _close()'ed above, and _dup2()'ed
+ * to fd 2 creating a new, inherited Win32 handle.
+ * Recover that real handle from fd 2. Note that
+ * SetStdHandle(STD_ERROR_HANDLE, _get_osfhandle(2))
+ * is implicit in the dup2() call above
+ */
+ newhand = (HANDLE)_get_osfhandle(2);
+ }
- /* dup2 is not supported literaly with native Windows handles.
- * We can, however, emulate dup2 for the standard i/o handles,
- * and close and replace other handles with duped handles.
- * The os_handle will change, however.
- */
- if (new_file->filehand == GetStdHandle(STD_ERROR_HANDLE)) {
- stdhandle |= stderr_handle;
- }
- if (new_file->filehand == GetStdHandle(STD_OUTPUT_HANDLE)) {
- stdhandle |= stdout_handle;
- }
- if (new_file->filehand == GetStdHandle(STD_INPUT_HANDLE)) {
- stdhandle |= stdin_handle;
- }
-
- if (stdhandle) {
- if (!DuplicateHandle(hproc, old_file->filehand,
- hproc, &newhand, 0,
- TRUE, DUPLICATE_SAME_ACCESS)) {
- return apr_get_os_error();
+ if ((new_file->flags & APR_STD_FLAGS) == APR_STDOUT_FLAG) {
+ /* For the process flow see the stderr case above */
+ fflush(stdout);
+ setvbuf(stdout, NULL, _IONBF, 0);
+ _commit(1 /* stdout */);
+
+ if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+ 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ return apr_get_os_error();
+ }
+ fd = _open_osfhandle((INT_PTR)newhand, _O_WRONLY | _O_BINARY);
+ _dup2(fd, 1);
+ _close(fd);
+ _setmode(1, _O_BINARY);
+ newhand = (HANDLE)_get_osfhandle(1);
}
- if (((stdhandle & stderr_handle) && !SetStdHandle(STD_ERROR_HANDLE, newhand)) ||
- ((stdhandle & stdout_handle) && !SetStdHandle(STD_OUTPUT_HANDLE, newhand)) ||
- ((stdhandle & stdin_handle) && !SetStdHandle(STD_INPUT_HANDLE, newhand))) {
- return apr_get_os_error();
+
+ if ((new_file->flags & APR_STD_FLAGS) == APR_STDIN_FLAG) {
+ /* For the process flow see the stderr case above */
+ fflush(stdin);
+ setvbuf(stdin, NULL, _IONBF, 0);
+ _commit(0 /* stdin */);
+
+ if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+ 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+ return apr_get_os_error();
+ }
+ fd = _open_osfhandle((INT_PTR)newhand, _O_RDONLY | _O_BINARY);
+ _dup2(fd, 0);
+ _close(fd);
+ _setmode(0, _O_BINARY);
+ newhand = (HANDLE)_get_osfhandle(0);
}
newflags = old_file->flags | APR_INHERIT;
}
diff --git a/file_io/win32/open.c b/file_io/win32/open.c
index f81c9c80e..0d8f88d1b 100644
--- a/file_io/win32/open.c
+++ b/file_io/win32/open.c
@@ -550,8 +550,7 @@ APR_DECLARE(apr_status_t) apr_os_file_put(apr_file_t **file,
/* ### check return codes */
(void) apr_pollset_create(&(*file)->pollset, 1, pool, 0);
- /* XXX... we pcalloc above so all others are zeroed.
- * Should we be testing if thefile is a handle to
+ /* Should we be testing if thefile is a handle to
* a PIPE and set up the mechanics appropriately?
*
* (*file)->pipe;
@@ -578,15 +577,11 @@ APR_DECLARE(apr_status_t) apr_file_open_flags_stderr(apr_file_t **thefile,
apr_set_os_error(APR_SUCCESS);
file_handle = GetStdHandle(STD_ERROR_HANDLE);
- if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
- apr_status_t rv = apr_get_os_error();
- if (rv == APR_SUCCESS) {
- return APR_EINVAL;
- }
- return rv;
- }
+ if (!file_handle)
+ file_handle = INVALID_HANDLE_VALUE;
- return apr_os_file_put(thefile, &file_handle, flags | APR_WRITE, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_WRITE | APR_STDERR_FLAG, pool);
#endif
}
@@ -601,15 +596,11 @@ APR_DECLARE(apr_status_t) apr_file_open_flags_stdout(apr_file_t **thefile,
apr_set_os_error(APR_SUCCESS);
file_handle = GetStdHandle(STD_OUTPUT_HANDLE);
- if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
- apr_status_t rv = apr_get_os_error();
- if (rv == APR_SUCCESS) {
- return APR_EINVAL;
- }
- return rv;
- }
+ if (!file_handle)
+ file_handle = INVALID_HANDLE_VALUE;
- return apr_os_file_put(thefile, &file_handle, flags | APR_WRITE, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_WRITE | APR_STDOUT_FLAG, pool);
#endif
}
@@ -624,15 +615,11 @@ APR_DECLARE(apr_status_t) apr_file_open_flags_stdin(apr_file_t **thefile,
apr_set_os_error(APR_SUCCESS);
file_handle = GetStdHandle(STD_INPUT_HANDLE);
- if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
- apr_status_t rv = apr_get_os_error();
- if (rv == APR_SUCCESS) {
- return APR_EINVAL;
- }
- return rv;
- }
+ if (!file_handle)
+ file_handle = INVALID_HANDLE_VALUE;
- return apr_os_file_put(thefile, &file_handle, flags | APR_READ, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_READ | APR_STDIN_FLAG, pool);
#endif
}
diff --git a/include/apr_thread_proc.h b/include/apr_thread_proc.h
index c7930901b..5cdb59d42 100644
--- a/include/apr_thread_proc.h
+++ b/include/apr_thread_proc.h
@@ -87,6 +87,15 @@ typedef enum {
/** @see apr_procattr_io_set */
#define APR_CHILD_BLOCK 4
+/** @see apr_procattr_io_set
+ * @note introduced strictly for Win32 to apr revision 1.2.12 (to restore
+ * the non-portable default behavior of 1.2.9 and prior versions on Win32).
+ * This becomes portable to all platforms effective revision 1.3.0, ensuring
+ * the standard files specified in the call to apr_procattr_io_set are not
+ * open in the created process (on Win32 as INVALID_HANDLE_VALUEs).
+ */
+#define APR_NO_FILE 8
+
/** @see apr_procattr_limit_set */
#define APR_LIMIT_CPU 0
/** @see apr_procattr_limit_set */
diff --git a/include/arch/win32/apr_arch_file_io.h b/include/arch/win32/apr_arch_file_io.h
index a0b4391f8..6807546bf 100644
--- a/include/arch/win32/apr_arch_file_io.h
+++ b/include/arch/win32/apr_arch_file_io.h
@@ -99,8 +99,12 @@ void *res_name_from_filename(const char *file, int global, apr_pool_t *pool);
#define APR_OPENINFO 0x00100000 /* Open without READ or WRITE access */
#define APR_OPENLINK 0x00200000 /* Open a link itself, if supported */
#define APR_READCONTROL 0x00400000 /* Read the file's owner/perms */
-#define APR_WRITECONTROL 0x00800000 /* Modifythe file's owner/perms */
+#define APR_WRITECONTROL 0x00800000 /* Modify the file's owner/perms */
#define APR_WRITEATTRS 0x01000000 /* Modify the file's attributes */
+#define APR_STDIN_FLAG 0x02000000 /* Obtained via apr_file_open_stdin() */
+#define APR_STDOUT_FLAG 0x04000000 /* Obtained via apr_file_open_stdout() */
+#define APR_STDERR_FLAG 0x06000000 /* Obtained via apr_file_open_stderr() */
+#define APR_STD_FLAGS (APR_STDIN_FLAG | APR_STDOUT_FLAG | APR_STDERR_FLAG)
/* Entries missing from the MSVC 5.0 Win32 SDK:
*/
diff --git a/threadproc/win32/proc.c b/threadproc/win32/proc.c
index d9c318ef1..c7f2408e9 100644
--- a/threadproc/win32/proc.c
+++ b/threadproc/win32/proc.c
@@ -32,6 +32,15 @@
#include <process.h>
#endif
+/* Heavy on no'ops, here's what we want to pass if there is APR_NO_FILE
+ * requested for a specific child handle;
+ */
+static apr_file_t no_file = { NULL, INVALID_HANDLE_VALUE, };
+
+/* We have very carefully excluded volumes of definitions from the
+ * Microsoft Platform SDK, which kill the build time performance.
+ * These the sole constants we borrow from WinBase.h and WinUser.h
+ */
#ifndef LOGON32_LOGON_NETWORK
#define LOGON32_LOGON_NETWORK 3
#endif
@@ -50,12 +59,12 @@
#define SW_HIDE 0
#endif
#endif
+
/*
* some of the ideas expressed herein are based off of Microsoft
* Knowledge Base article: Q190351
*
*/
-
APR_DECLARE(apr_status_t) apr_procattr_create(apr_procattr_t **new,
apr_pool_t *pool)
{
@@ -83,20 +92,30 @@ APR_DECLARE(apr_status_t) apr_procattr_io_set(apr_procattr_t *attr,
in = APR_READ_BLOCK;
else if (in == APR_PARENT_BLOCK)
in = APR_WRITE_BLOCK;
- stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in, in,
- attr->pool);
+
+ if (in == APR_NO_FILE)
+ attr->child_in = &no_file;
+ else
+ stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in,
+ in, attr->pool);
if (stat == APR_SUCCESS)
stat = apr_file_inherit_unset(attr->parent_in);
}
if (out && stat == APR_SUCCESS) {
- stat = apr_create_nt_pipe(&attr->parent_out, &attr->child_out, out,
- attr->pool);
+ if (out == APR_NO_FILE)
+ attr->child_out = &no_file;
+ else
+ stat = apr_create_nt_pipe(&attr->parent_out, &attr->child_out,
+ out, attr->pool);
if (stat == APR_SUCCESS)
stat = apr_file_inherit_unset(attr->parent_out);
}
if (err && stat == APR_SUCCESS) {
- stat = apr_create_nt_pipe(&attr->parent_err, &attr->child_err, err,
- attr->pool);
+ if (err == APR_NO_FILE)
+ attr->child_err = &no_file;
+ else
+ stat = apr_create_nt_pipe(&attr->parent_err, &attr->child_err,
+ err, attr->pool);
if (stat == APR_SUCCESS)
stat = apr_file_inherit_unset(attr->parent_err);
}
@@ -110,7 +129,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_in_set(apr_procattr_t *attr,
apr_status_t rv = APR_SUCCESS;
if (child_in) {
- if (attr->child_in == NULL)
+ if ((attr->child_in == NULL) || (attr->child_in == &no_file))
rv = apr_file_dup(&attr->child_in, child_in, attr->pool);
else
rv = apr_file_dup2(attr->child_in, child_in, attr->pool);
@@ -136,7 +155,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_out_set(apr_procattr_t *attr,
apr_status_t rv = APR_SUCCESS;
if (child_out) {
- if (attr->child_out == NULL)
+ if ((attr->child_out == NULL) || (attr->child_out == &no_file))
rv = apr_file_dup(&attr->child_out, child_out, attr->pool);
else
rv = apr_file_dup2(attr->child_out, child_out, attr->pool);
@@ -162,7 +181,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_err_set(apr_procattr_t *attr,
apr_status_t rv = APR_SUCCESS;
if (child_err) {
- if (attr->child_err == NULL)
+ if ((attr->child_err == NULL) || (attr->child_err == &no_file))
rv = apr_file_dup(&attr->child_err, child_err, attr->pool);
else
rv = apr_file_dup2(attr->child_err, child_err, attr->pool);
@@ -784,9 +803,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
SetHandleInformation(si.hStdInput,
HANDLE_FLAG_INHERIT, 0);
- si.hStdInput = attr->child_in->filehand;
- SetHandleInformation(si.hStdInput, HANDLE_FLAG_INHERIT,
- HANDLE_FLAG_INHERIT);
+ if ( (si.hStdInput = attr->child_in->filehand)
+ != INVALID_HANDLE_VALUE )
+ SetHandleInformation(si.hStdInput, HANDLE_FLAG_INHERIT,
+ HANDLE_FLAG_INHERIT);
}
si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
@@ -798,9 +818,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
SetHandleInformation(si.hStdOutput,
HANDLE_FLAG_INHERIT, 0);
- si.hStdOutput = attr->child_out->filehand;
- SetHandleInformation(si.hStdOutput, HANDLE_FLAG_INHERIT,
- HANDLE_FLAG_INHERIT);
+ if ( (si.hStdOutput = attr->child_out->filehand)
+ != INVALID_HANDLE_VALUE )
+ SetHandleInformation(si.hStdOutput, HANDLE_FLAG_INHERIT,
+ HANDLE_FLAG_INHERIT);
}
si.hStdError = GetStdHandle(STD_ERROR_HANDLE);
@@ -812,9 +833,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
SetHandleInformation(si.hStdError,
HANDLE_FLAG_INHERIT, 0);
- si.hStdError = attr->child_err->filehand;
- SetHandleInformation(si.hStdError, HANDLE_FLAG_INHERIT,
- HANDLE_FLAG_INHERIT);
+ if ( (si.hStdError = attr->child_err->filehand)
+ != INVALID_HANDLE_VALUE )
+ SetHandleInformation(si.hStdError, HANDLE_FLAG_INHERIT,
+ HANDLE_FLAG_INHERIT);
}
}
if (attr->user_token) {
@@ -937,13 +959,13 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
new->hproc = pi.hProcess;
new->pid = pi.dwProcessId;
- if (attr->child_in) {
+ if ((attr->child_in) && (attr->child_in != &no_file)) {
apr_file_close(attr->child_in);
}
- if (attr->child_out) {
+ if ((attr->child_out) && (attr->child_out != &no_file)) {
apr_file_close(attr->child_out);
}
- if (attr->child_err) {
+ if ((attr->child_err) && (attr->child_err != &no_file)) {
apr_file_close(attr->child_err);
}
CloseHandle(pi.hThread);