[PATCH v10 0/1] MR5968: cmd: Do not set enviroment variable when no input is provided by set /p command.

[Vijay Kiran Kamuju (＠infyquest)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55662

Tests are not provided as its interactive.

--
  v10: cmd: Do not set enviroment variable when no input is provided by set /p command.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5968

[PATCH v10 1/1] cmd: Do not set enviroment variable when no input is provided by set /p command.

[Vijay Kiran Kamuju]

From: Vijay Kiran Kamuju <infyquest(a)gmail.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55662
---
 programs/cmd/builtins.c                  | 9 ++++++---
 programs/cmd/tests/test_builtins.cmd     | 5 +++++
 programs/cmd/tests/test_builtins.cmd.exp | 4 ++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index 17671c32eba..5bb77408aac 100644
--- a/programs/cmd/builtins.c
+++ b/programs/cmd/builtins.c
@@ -3188,7 +3188,7 @@ RETURN_CODE WCMD_setshow_env(WCHAR *s)
     }
 
     /* If no parameter, or no '=' sign, return an error */
-    if (!(*s) || ((p = wcschr (s, '=')) == NULL )) {
+    if (!(*s) || ((p = wcschr(s, '=')) == NULL )) {
       WCMD_output_stderr(WCMD_LoadMessage(WCMD_NOARG));
       return_code = ERROR_INVALID_FUNCTION;
     }
@@ -3196,7 +3196,10 @@ RETURN_CODE WCMD_setshow_env(WCHAR *s)
     {
       /* Output the prompt */
       *p++ = '\0';
-      if (*p) WCMD_output_asis(p);
+      if (*p) {
+        if (wcschr(p, '\"')) WCMD_strip_quotes(p);
+        WCMD_output_asis(p);
+      }
 
       /* Read the reply */
       if (WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), string, ARRAY_SIZE(string), &count) && count > 1) {
@@ -3204,7 +3207,7 @@ RETURN_CODE WCMD_setshow_env(WCHAR *s)
         if (string[count-2] == '\r') string[count-2] = '\0'; /* Under Windoze we get CRLF! */
         TRACE("set /p: Setting var '%s' to '%s'\n", wine_dbgstr_w(s),
               wine_dbgstr_w(string));
-        SetEnvironmentVariableW(s, string);
+        if (*string) SetEnvironmentVariableW(s, string);
       }
     }
 
diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 66d5e894662..f8e11595d4c 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -821,6 +821,11 @@ del folder\sub1.bat
 rmdir "fol;der"
 rmdir folder
 PATH=%PATH_BACKUP%
+set WINE_FOO=foo
+echo bar| cmd /v:on /c "set /p WINE_FOO=prompt & echo X!WINE_FOO!X"
+echo:| cmd /v:on /c "set /p WINE_FOO=prompt & echo Y!WINE_FOO!Y"
+echo:| cmd /v:on /c "set /p WINE_FOO='prompt' & echo Y!WINE_FOO!Y"
+echo:| cmd /v:on /c "set /p WINE_FOO="prompt" & echo Y!WINE_FOO!Y"
 
 echo ------------ Testing 'choice' ------------
 
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index a96c7dde10b..ebcaabab2fa 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -687,6 +687,10 @@ foo
 I'm here!@space@
 I'm here!@space@
 I'm here!@space@
+prompt XbarX
+prompt YfooY
+'prompt' YfooY
+(a)todo_wine@promptYfooY
 ------------ Testing 'choice' ------------
 @todo_wine(a)Example message [A,B,C]?A(a)or_broken@choice unavailable
 1(a)or_broken@9009
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5968

[eric pouech (＠epo)]

eric pouech (@epo) commented about programs/cmd/builtins.c:
> {
>        /* Output the prompt */
>        *p++ = '\0';
> -      if (*p) WCMD_output_asis(p);
> +      if (*p) {
> +        if (wcschr(p, '\"')) WCMD_strip_quotes(p);

not fully compatible with additonal tests:
```
> set /p A=ab"c"
> ab"c"
> set /p A=  ab"c"
> ab"c"
> set /p A=  "ab"c
> ab
so likely closer to:

p = WCMD_skip_leading_spaces(p);
if (*p)
{
if (*p == L'"') { WCHAR* last = wcsrchr(p + 1, L'"'); p++; if (last) *last = L'\0';}
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5968#note_75884

[Vijay Kiran Kamuju (＠infyquest)]

On Thu Jul 11 08:28:13 2024 +0000, eric pouech wrote:
> not fully compatible with additonal tests:
> ```
> > set /p A=ab"c"
> > ab"c"
> > set /p A=  ab"c"
> > ab"c"
> > set /p A=  "ab"c
> > ab
> so likely closer to:
>
> p = WCMD_skip_leading_spaces(p);
> if (*p)
> {
> if (*p == L'"') { WCHAR* last = wcsrchr(p + 1, L'"'); p++; if (last)
> *last = L'\0';}
> ```
> I have not tested this case, will add the code as you suggested. I will also test other cases which use \"

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5968#note_75887