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().

This is the nonportable flavor targeting 1.2.12; unix 1.3.0 specific
commit to follow.

git-svn-id: http://svn.apache.org/repos/asf/apr/apr/trunk@580484 13f79535-47bb-0310-9956-ffa450edef68

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);