[PATCH 0/2] MR5428: Fix substring operation in 'magic' variable expansion.

[eric pouech (＠epo)]

In substring expansion of variables, the length computation wasn't the same between:
- magic variables: length doesn't include terminating NUL 
- regular variables: length does include terminating NUL
This led to incorrect substring operation for magic variables.
Always set length to not include terminating NUL. Took the opportunity to not
recompute length (as it's already computed by called functions).

[PATCH 1/2] programs/cmd: Add test for substring handling in 'magic' variable expansion.

[Eric Pouech]

From: Eric Pouech

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd     | 6 ++++++
 programs/cmd/tests/test_builtins.cmd.exp | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/programs/cmd/tests/test_builtins.cmd b/programs/cmd/tests/test_builtins.cmd
index 1ab2935aec4..7517cca343f 100644
--- a/programs/cmd/tests/test_builtins.cmd
+++ b/programs/cmd/tests/test_builtins.cmd
@@ -622,7 +622,13 @@ echo %WINE_VAR:~2,-1%
 echo %WINE_VAR:~2,-3%
 echo '%WINE_VAR:~-2,-4%'
 echo %WINE_VAR:~-3,-2%
+echo %WINE_VAR:~4,4%
 set WINE_VAR=
+mkdir dummydir
+cd dummydir
+echo %CD:~-6,6%
+cd ..
+rmdir dummydir

echo ------------ Testing variable substitution ------------
 echo --- in FOR variables
diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 0fb4a597ac5..7030748d549 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -532,6 +532,8 @@ ert@or_broken@qwerty
 e@or_broken@qwerty
 ''@or_broken@'qwerty'
 r@or_broken@qwerty
+ty
+@todo_wine@mmydir
 ------------ Testing variable substitution ------------
 --- in FOR variables
 "A B"

[PATCH 2/2] programs/cmd: Fix substring expansion for 'magic' variables.

[Eric Pouech]

From: Eric Pouech

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

Signed-off-by: Eric Pouech 
---
 programs/cmd/tests/test_builtins.cmd.exp |  2 +-
 programs/cmd/wcmdmain.c                  | 32 ++++++++++--------------
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp
index 7030748d549..1365a5c3474 100644
--- a/programs/cmd/tests/test_builtins.cmd.exp
+++ b/programs/cmd/tests/test_builtins.cmd.exp
@@ -533,7 +533,7 @@ e@or_broken@qwerty
 ''@or_broken@'qwerty'
 r@or_broken@qwerty
 ty
-@todo_wine@mmydir
+mmydir
 ------------ Testing variable substitution ------------
 --- in FOR variables
 "A B"
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index 1867c5afb92..eb66b29836e 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -614,25 +614,19 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
     /* Handle DATE, TIME, ERRORLEVEL and CD replacements allowing */
     /* override if existing env var called that name              */
     if (WCMD_is_magic_envvar(thisVar, L"ERRORLEVEL")) {
-      wsprintfW(thisVarContents, L"%d", errorlevel);
-      len = lstrlenW(thisVarContents);
+      len = wsprintfW(thisVarContents, L"%d", errorlevel);
     } else if (WCMD_is_magic_envvar(thisVar, L"DATE")) {
-      GetDateFormatW(LOCALE_USER_DEFAULT, DATE_SHORTDATE, NULL,
-                    NULL, thisVarContents, MAXSTRING);
-      len = lstrlenW(thisVarContents);
+      len = GetDateFormatW(LOCALE_USER_DEFAULT, DATE_SHORTDATE, NULL,
+                           NULL, thisVarContents, ARRAY_SIZE(thisVarContents));
     } else if (WCMD_is_magic_envvar(thisVar, L"TIME")) {
-      GetTimeFormatW(LOCALE_USER_DEFAULT, TIME_NOSECONDS, NULL,
-                        NULL, thisVarContents, MAXSTRING);
-      len = lstrlenW(thisVarContents);
+      len = GetTimeFormatW(LOCALE_USER_DEFAULT, TIME_NOSECONDS, NULL,
+                           NULL, thisVarContents, ARRAY_SIZE(thisVarContents));
     } else if (WCMD_is_magic_envvar(thisVar, L"CD")) {
-      GetCurrentDirectoryW(MAXSTRING, thisVarContents);
-      len = lstrlenW(thisVarContents);
+      len = GetCurrentDirectoryW(ARRAY_SIZE(thisVarContents), thisVarContents);
     } else if (WCMD_is_magic_envvar(thisVar, L"RANDOM")) {
-      wsprintfW(thisVarContents, L"%d", rand() % 32768);
-      len = lstrlenW(thisVarContents);
+      len = wsprintfW(thisVarContents, L"%d", rand() % 32768);
     } else {
-
-      len = ExpandEnvironmentStringsW(thisVar, thisVarContents, ARRAY_SIZE(thisVarContents));
+      if ((len = ExpandEnvironmentStringsW(thisVar, thisVarContents, ARRAY_SIZE(thisVarContents)))) len--;
     }

if (len == 0)
@@ -704,9 +698,9 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)

/* Check bounds */
       if (substrposition >= 0) {
-        startCopy = &thisVarContents[min(substrposition, len)];
+        startCopy = &thisVarContents[min(substrposition, len - 1)];
       } else {
-        startCopy = &thisVarContents[max(0, len+substrposition-1)];
+        startCopy = &thisVarContents[max(0, len + substrposition)];
       }

if (commapos == NULL) {
@@ -714,12 +708,12 @@ static WCHAR *WCMD_expand_envvar(WCHAR *start, WCHAR startchar)
         WCMD_strsubstW(start, endOfVar + 1, startCopy, -1);
       } else if (substrlength < 0) {

-        int copybytes = (len+substrlength-1)-(startCopy-thisVarContents);
-        if (copybytes > len) copybytes = len;
+        int copybytes = len + substrlength - (startCopy - thisVarContents);
+        if (copybytes >= len) copybytes = len - 1;
         else if (copybytes < 0) copybytes = 0;
         WCMD_strsubstW(start, endOfVar + 1, startCopy, copybytes);
       } else {
-        substrlength = min(substrlength, len - (startCopy- thisVarContents + 1));
+        substrlength = min(substrlength, len - (startCopy - thisVarContents));
         WCMD_strsubstW(start, endOfVar + 1, startCopy, substrlength);
       }