Eric Pouech : cmd: Fix a couple of issues with redirections.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 957995c1c6e608b449a94c7c0bee7485a7eb174d
URL:    https://gitlab.winehq.org/wine/wine/-/commit/957995c1c6e608b449a94c7c0bee748...

Author: Eric Pouech <epouech(a)codeweavers.com>
Date:   Thu Jul 11 09:46:16 2024 +0200

cmd: Fix a couple of issues with redirections.

Signed-off-by: Eric Pouech <epouech(a)codeweavers.com>

---

 programs/cmd/tests/test_builtins.cmd     | 14 ++++++++++++++
 programs/cmd/tests/test_builtins.cmd.exp |  6 ++++--
 programs/cmd/wcmdmain.c                  | 30 +++++++++++++++++++-----------
 3 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 66d5e894662..c67fc84988a 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -217,6 +217,20 @@ type C
 (if 1==0 (echo A) else echo B) > C
 type C
 (if 1==0 (echo A > B) else echo C)
+echo --- multiredirections
+erase /q a b & (echo >a >b)
+if exist a echo a shouldn't exist
+if not exist b echo b should exist
+erase /q a b & (echo >a >>b)
+if exist a echo a shouldn't exist
+if not exist b echo b should exist
+erase /q a b & (echo >a | (echo b > b))
+if not exist a echo a should exist
+if not exist b echo b should exist
+erase /q a b & (echo cc1 2>a 1>&2 2>b)
+if exist a echo a shouldn't exist
+if not exist b (echo b should exist) else (echo cc2 & type b)
+
 cd .. & rd /s/q foobar
 
 echo ------------ Testing circumflex escape character ------------
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index a96c7dde10b..84bcdcdf581 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -202,8 +202,8 @@ fooc(a)space@
 food1
 food2
 food21
-(a)todo_wine@foo7(a)space@@space@@or_broken(a)not supported(a)space@
-(a)todo_wine@foo(a)or_broken@not supported
+foo7(a)space@@space@@or_broken(a)not supported(a)space@
+foo(a)or_broken@not supported
 --- redirect at beginning of line
 foo
 foo1
@@ -229,6 +229,8 @@ foo
 A
 B
 C
+--- multiredirections
+cc2(a)space@
 ------------ Testing circumflex escape character ------------
 hello, world
 hello, world
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index eee9270582c..5f55e4c8d9e 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -1649,12 +1649,6 @@ RETURN_CODE WCMD_run_program(WCHAR *command, BOOL called)
   return ERROR_INVALID_FUNCTION;
 }
 
-/* this is obviously wrong... will require more work to be fixed */
-static inline unsigned clamp_fd(unsigned fd)
-{
-    return fd <= 2 ? fd : 1;
-}
-
 static BOOL set_std_redirections(CMD_REDIRECTION *redir)
 {
     static DWORD std_index[3] = {STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE};
@@ -1664,6 +1658,12 @@ static BOOL set_std_redirections(CMD_REDIRECTION *redir)
 
     for (; redir; redir = redir->next)
     {
+        CMD_REDIRECTION *next;
+
+        /* if we have several elements changing same std stream, only use last one */
+        for (next = redir->next; next; next = next->next)
+            if (redir->fd == next->fd) break;
+        if (next) continue;
         switch (redir->kind)
         {
         case REDIR_READ_FROM:
@@ -1697,17 +1697,26 @@ static BOOL set_std_redirections(CMD_REDIRECTION *redir)
             }
             break;
         case REDIR_WRITE_CLONE:
+            if (redir->clone > 2 || redir->clone == redir->fd)
+            {
+                WARN("Can't duplicate %d from %d\n", redir->fd, redir->clone);
+                return FALSE;
+            }
             if (!DuplicateHandle(GetCurrentProcess(),
-                                 GetStdHandle(std_index[clamp_fd(redir->clone)]),
+                                 GetStdHandle(std_index[redir->clone]),
                                  GetCurrentProcess(),
                                  &h,
                                  0, TRUE, DUPLICATE_SAME_ACCESS))
             {
                 WARN("Duplicating handle failed with gle %ld\n", GetLastError());
+                return FALSE;
             }
             break;
         }
-        SetStdHandle(std_index[clamp_fd(redir->fd)], h);
+        if (redir->fd > 2)
+            CloseHandle(h);
+        else
+            SetStdHandle(std_index[redir->fd], h);
     }
     return TRUE;
 }
@@ -3654,10 +3663,9 @@ RETURN_CODE node_execute(CMD_NODE *node)
     if (!set_std_redirections(node->redirects))
     {
         WCMD_print_error();
-        /* FIXME potentially leaking here (if first redir created ok, and second failed */
-        return ERROR_INVALID_FUNCTION;
+        return_code = ERROR_INVALID_FUNCTION;
     }
-    switch (node->op)
+    else switch (node->op)
     {
     case CMD_SINGLE:
         if (node->command[0] != ':')