[PATCH v3 0/1] MR7049: vbscript: fix Mid() empty and null handling

[Francis De Brabandere (＠francisdb)]

Fixes https://bugs.winehq.org/show_bug.cgi?id=57563

--
  v3: vbscript: Mid() empty and null handling
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7049

[PATCH v3 1/1] vbscript: Mid() empty and null handling

[Francis De Brabandere]

From: Francis De Brabandere <francisdb(a)gmail.com>

---
 dlls/vbscript/global.c      |  9 ++++++---
 dlls/vbscript/tests/api.vbs | 17 +++++++++++++++--
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
index 058ddabcde1..109a20a3ac3 100644
--- a/dlls/vbscript/global.c
+++ b/dlls/vbscript/global.c
@@ -1404,9 +1404,6 @@ static HRESULT Global_Mid(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, V
 
     assert(args_cnt == 2 || args_cnt == 3);
 
-    if(V_VT(args) == VT_EMPTY)
-        return MAKE_VBSERROR(VBSE_ILLEGAL_FUNC_CALL);
-
     if(V_VT(args+1) == VT_NULL || (args_cnt == 3 && V_VT(args+2) == VT_NULL))
         return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE);
 
@@ -1432,6 +1429,12 @@ static HRESULT Global_Mid(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, V
             return MAKE_VBSERROR(VBSE_ILLEGAL_FUNC_CALL);
     }
 
+    if(V_VT(args) == VT_EMPTY)
+        return return_string(res, L"");
+
+    if(V_VT(args) == VT_NULL)
+        return return_null(res);
+
     if(V_VT(args) == VT_BSTR) {
         str = V_BSTR(args);
     }else {
diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs
index 7b8744f4c74..893c3569e43 100644
--- a/dlls/vbscript/tests/api.vbs
+++ b/dlls/vbscript/tests/api.vbs
@@ -632,6 +632,16 @@ Sub TestMid2(str, start, ex)
     Call ok(x = ex, "Mid(" & str & ", " & start & ") = " & x & " expected " & ex)
 End Sub
 
+Sub TestMidNull(str, start, len)
+    x = Mid(str, start, len)
+    Call ok(IsNull(x), "Mid(" & str & ", " & start & ", " & len & ") = " & x & " expected Null")
+End Sub
+
+Sub TestMidNull2(str, start)
+    x = Mid(str, start)
+    Call ok(IsNull(x), "Mid(" & str & ", " & start & ") = " & x & " expected Null")
+End Sub
+
 TestMid "test", 2, 2, "es"
 TestMid "test", 2, 4, "est"
 TestMid "test", 1, 2, "te"
@@ -640,11 +650,15 @@ TestMid "test", 1, 0, ""
 TestMid "test", 5, 2, ""
 TestMid 1234, 1, 2, "12"
 TestMid 1234, 5, 2, ""
+TestMid empty, 5, 2, ""
 TestMid2 "test", 1, "test"
 TestMid2 "test", 2, "est"
 TestMid2 "test", 4, "t"
 TestMid2 "test", 5, ""
 TestMid2 1234, 5, ""
+TestMid2 empty, 5, ""
+TestMidNull null, 5, 2
+TestMidNull2 null, 5
 
 sub TestMidError()
     on error resume next
@@ -657,6 +671,7 @@ sub TestMidError()
     call Err.clear()
     call Mid("test", "a", empty)
     call ok(Err.number = 13, "Err.number = " & Err.number)
+    call Err.clear()
     call Mid("test", 0, -1)
     call ok(Err.number = 5, "Err.number = " & Err.number)
     call Err.clear()
@@ -687,8 +702,6 @@ sub TestMidError()
     call Mid("test", 0, empty)
     call ok(Err.number = 5, "Err.number = " & Err.number)
     call Err.clear()
-    call Mid(empty, 0, 0)
-    call ok(Err.number = 5, "Err.number = " & Err.number)
 end sub
 call TestMidError()
 
-- 
GitLab

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

[PATCH v3 1/1] vbscript: Mid() empty and null handling

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=150454

Your paranoid android.


=== debian11b (64 bit WoW report) ===

user32:
msg.c:12676: Test failed: Got expected 0.
msg.c:12691: Test failed: Got expected 0.

[Francis De Brabandere (＠francisdb)]

I don't think any of these remaining test failures are related to this merge request.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7049#note_90918

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/vbscript/tests/api.vbs:
> call Mid("test", 0, empty)
>      call ok(Err.number = 5, "Err.number = " & Err.number)
>      call Err.clear()
> -    call Mid(empty, 0, 0)
> -    call ok(Err.number = 5, "Err.number = " & Err.number)

Why did you remove those tests? They pass on Windows, so if they fail on Wine, the implementation should be fixed, not the tests removed.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7049#note_90933

[Francis De Brabandere (＠francisdb)]

On Thu Dec 19 21:07:03 2024 +0000, Jacek Caban wrote:
> Why did you remove those tests? They pass on Windows, so if they fail on
> Wine, the implementation should be fixed, not the tests removed.
> Interesting, so these tests are run on windows with the real cscript?
> Will fix this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7049#note_90950

[Jacek Caban (＠jacek)]

On Thu Dec 19 22:33:30 2024 +0000, Francis De Brabandere wrote:
> Interesting, so these tests are run on windows with the real cscript?
> Will fix this.
> Yes, it's ran on Windows in addition to Wine. `dlls/vbscript/tests/run.c` implements its own script host that provides additional functionality like `ok()` function, so it uses the script engine directly, without cscript involvement. If you'd have mingw installed, you'd have `vbscript_test.exe` in your build that you can just copy to Windows and run. CI also runs it on Windows, see https://gitlab.winehq.org/francisdb/wine/-/jobs/124440.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7049#note_90952