QProcess/Win: allow child processes to change modes of the stdin pipe

To be able to call SetNamedPipeHandleState on stdin in a child
process, we must create a read-end pipe handle with the
FILE_WRITE_ATTRIBUTES flag set.
This can't be done with CreateNamedPipe but only with CreateFile.
Therefore we're creating the handles for the child process always
with CreateFile now. Besides, it's conceptually cleaner to have the
server handle of the named pipe in the calling process.

[ChangeLog][QtCore][Windows] Fix regression from Qt4 in QProcess.
It wasn't possible anymore to alter pipe modes of stdin in child
processes.

Task-number: QTBUG-35357

Change-Id: I85f09753d0c924bdc8a6cef1ea5dbe6b2299c604
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@digia.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>

5 files changed, 113 insertions, 29 deletions

diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
index dba9f62b98..fc2adb783e 100644
--- a/src/corelib/io/qprocess_win.cpp
+++ b/src/corelib/io/qprocess_win.cpp
@@ -73,10 +73,11 @@ static void qt_create_pipe(Q_PIPE *pipe, bool isInputPipe)
     // Anomymous pipes do not support asynchronous I/O. Thus we
     // create named pipes for redirecting stdout, stderr and stdin.
 
+    // The write handle must be non-inheritable for input pipes.
+    // The read handle must be non-inheritable for output pipes.
     SECURITY_ATTRIBUTES secAtt = { sizeof(SECURITY_ATTRIBUTES), 0, false };
-    secAtt.bInheritHandle = isInputPipe;    // The read handle must be non-inheritable for output pipes.
 
-    HANDLE hRead;
+    HANDLE hServer;
     wchar_t pipeName[256];
     unsigned int attempts = 1000;
     forever {
@@ -85,19 +86,29 @@ static void qt_create_pipe(Q_PIPE *pipe, bool isInputPipe)
         _snwprintf(pipeName, sizeof(pipeName) / sizeof(pipeName[0]),
                 L"\\\\.\\pipe\\qt-%X", qrand());
 
+        DWORD dwOpenMode = FILE_FLAG_OVERLAPPED;
+        DWORD dwOutputBufferSize = 0;
+        DWORD dwInputBufferSize = 0;
+        const DWORD dwPipeBufferSize = 1024 * 1024;
+        if (isInputPipe) {
+            dwOpenMode |= PIPE_ACCESS_OUTBOUND;
+            dwOutputBufferSize = dwPipeBufferSize;
+        } else {
+            dwOpenMode |= PIPE_ACCESS_INBOUND;
+            dwInputBufferSize = dwPipeBufferSize;
+        }
         DWORD dwPipeFlags = PIPE_TYPE_BYTE | PIPE_WAIT;
         if (QSysInfo::windowsVersion() >= QSysInfo::WV_VISTA)
             dwPipeFlags |= PIPE_REJECT_REMOTE_CLIENTS;
-        const DWORD dwPipeBufferSize = 1024 * 1024;
-        hRead = CreateNamedPipe(pipeName,
-                                PIPE_ACCESS_INBOUND | FILE_FLAG_OVERLAPPED,
-                                dwPipeFlags,
-                                1,                      // only one pipe instance
-                                0,                      // output buffer size
-                                dwPipeBufferSize,       // input buffer size
-                                0,
-                                &secAtt);
-        if (hRead != INVALID_HANDLE_VALUE)
+        hServer = CreateNamedPipe(pipeName,
+                                  dwOpenMode,
+                                  dwPipeFlags,
+                                  1,                      // only one pipe instance
+                                  dwOutputBufferSize,
+                                  dwInputBufferSize,
+                                  0,
+                                  &secAtt);
+        if (hServer != INVALID_HANDLE_VALUE)
             break;
         DWORD dwError = GetLastError();
         if (dwError != ERROR_PIPE_BUSY || !--attempts) {
@@ -106,28 +117,31 @@ static void qt_create_pipe(Q_PIPE *pipe, bool isInputPipe)
         }
     }
 
-    // The write handle must be non-inheritable for input pipes.
-    secAtt.bInheritHandle = !isInputPipe;
-
-    HANDLE hWrite = INVALID_HANDLE_VALUE;
-    hWrite = CreateFile(pipeName,
-                        GENERIC_WRITE,
-                        0,
-                        &secAtt,
-                        OPEN_EXISTING,
-                        FILE_FLAG_OVERLAPPED,
-                        NULL);
-    if (hWrite == INVALID_HANDLE_VALUE) {
+    secAtt.bInheritHandle = TRUE;
+    const HANDLE hClient = CreateFile(pipeName,
+                                      (isInputPipe ? (GENERIC_READ | FILE_WRITE_ATTRIBUTES)
+                                                   : GENERIC_WRITE),
+                                      0,
+                                      &secAtt,
+                                      OPEN_EXISTING,
+                                      FILE_FLAG_OVERLAPPED,
+                                      NULL);
+    if (hClient == INVALID_HANDLE_VALUE) {
         qErrnoWarning("QProcess: CreateFile failed.");
-        CloseHandle(hRead);
+        CloseHandle(hServer);
         return;
     }
 
     // Wait until connection is in place.
-    ConnectNamedPipe(hRead, NULL);
+    ConnectNamedPipe(hServer, NULL);
 
-    pipe[0] = hRead;
-    pipe[1] = hWrite;
+    if (isInputPipe) {
+        pipe[0] = hClient;
+        pipe[1] = hServer;
+    } else {
+        pipe[0] = hServer;
+        pipe[1] = hClient;
+    }
 }
 
 static void duplicateStdWriteChannel(Q_PIPE *pipe, DWORD nStdHandle)
diff --git a/tests/auto/corelib/io/qprocess/qprocess.pro b/tests/auto/corelib/io/qprocess/qprocess.pro
index 4155e3f73d..6ba54b1e92 100644
--- a/tests/auto/corelib/io/qprocess/qprocess.pro
+++ b/tests/auto/corelib/io/qprocess/qprocess.pro
@@ -8,7 +8,11 @@ SUBDIRS += testProcessSpacesArgs/nospace.pro \
            testProcessSpacesArgs/twospaces.pro \
            testSpaceInName
 
-win32:!wince*:SUBDIRS+=testProcessEchoGui
+win32:!wince* {
+    SUBDIRS += \
+        testProcessEchoGui \
+        testSetNamedPipeHandleState
+}
 
 test.depends += $$SUBDIRS
 SUBDIRS += test
diff --git a/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/main.cpp b/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/main.cpp
new file mode 100644
index 0000000000..b2cc793b51
--- /dev/null
+++ b/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/main.cpp
@@ -0,0 +1,50 @@
+/****************************************************************************
+**
+** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
+** Contact: http://www.qt-project.org/legal
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and Digia.  For licensing terms and
+** conditions see http://qt.digia.com/licensing.  For further information
+** use the contact form at http://qt.digia.com/contact-us.
+**
+** GNU Lesser General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU Lesser
+** General Public License version 2.1 as published by the Free Software
+** Foundation and appearing in the file LICENSE.LGPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU Lesser General Public License version 2.1 requirements
+** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** In addition, as a special exception, Digia gives you certain additional
+** rights.  These rights are described in the Digia Qt LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3.0 as published by the Free Software
+** Foundation and appearing in the file LICENSE.GPL included in the
+** packaging of this file.  Please review the following information to
+** ensure the GNU General Public License version 3.0 requirements will be
+** met: http://www.gnu.org/copyleft/gpl.html.
+**
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <windows.h>
+
+int main()
+{
+    DWORD mode = PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT;
+    if (SetNamedPipeHandleState(GetStdHandle(STD_INPUT_HANDLE), &mode, NULL, NULL))
+        return 0;
+    return GetLastError();
+}
diff --git a/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/testSetNamedPipeHandleState.pro b/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/testSetNamedPipeHandleState.pro
new file mode 100644
index 0000000000..e236e05c7d
--- /dev/null
+++ b/tests/auto/corelib/io/qprocess/testSetNamedPipeHandleState/testSetNamedPipeHandleState.pro
@@ -0,0 +1,4 @@
+SOURCES = main.cpp
+CONFIG -= qt app_bundle
+CONFIG += console
+DESTDIR = ./
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index d141c11a90..37f224ff28 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -93,6 +93,7 @@ private slots:
     void echoTest2();
 #ifdef Q_OS_WIN
     void echoTestGui();
+    void testSetNamedPipeHandleState();
     void batFiles_data();
     void batFiles();
 #endif
@@ -555,6 +556,17 @@ void tst_QProcess::echoTestGui()
     QCOMPARE(process.readAllStandardOutput(), QByteArray("Hello"));
     QCOMPARE(process.readAllStandardError(), QByteArray("Hello"));
 }
+
+void tst_QProcess::testSetNamedPipeHandleState()
+{
+    QProcess process;
+    process.setProcessChannelMode(QProcess::SeparateChannels);
+    process.start("testSetNamedPipeHandleState/testSetNamedPipeHandleState");
+    QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString()));
+    QVERIFY(process.waitForFinished(5000));
+    QCOMPARE(process.exitCode(), 0);
+    QCOMPARE(process.exitStatus(), QProcess::NormalExit);
+}
 #endif // !Q_OS_WINCE && Q_OS_WIN
 
 //-----------------------------------------------------------------------------