[PATCH v12 0/1] MR10314: vbscript: Convert string to number for comparison operators.
When one operand was a string and the other numeric, VBScript should have converted the string to a double before comparing. VarCmp did not do this automatically, resulting in incorrect string comparison. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56281 -- v12: vbscript: Convert string to number for comparison operators. https://gitlab.winehq.org/wine/wine/-/merge_requests/10314
From: Francis De Brabandere <francisdb@gmail.com> When one operand was a string and the other numeric, VBScript should have converted the string to a double before comparing. VarCmp did not do this automatically, resulting in incorrect string comparison. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56281 --- dlls/vbscript/global.c | 2 +- dlls/vbscript/interp.c | 34 +++++++++++++++++++++++++++++++--- dlls/vbscript/tests/lang.vbs | 35 +++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 1 + 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c index f1afcfade89..d1326ac0245 100644 --- a/dlls/vbscript/global.c +++ b/dlls/vbscript/global.c @@ -425,7 +425,7 @@ HRESULT to_int(VARIANT *v, int *ret) return S_OK; } -static HRESULT to_double(VARIANT *v, double *ret) +HRESULT to_double(VARIANT *v, double *ret) { VARIANT dst; HRESULT hres; diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index a71901a1d83..58afe4c6b41 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -2107,14 +2107,37 @@ static HRESULT interp_imp(exec_ctx_t *ctx) return stack_push(ctx, &v); } +static inline BOOL is_numeric_vt(VARTYPE vt) +{ + return vt == VT_I2 || vt == VT_I4 || vt == VT_R4 || vt == VT_R8 + || vt == VT_CY || vt == VT_DATE || vt == VT_UI1; +} + static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) { TRACE("%s %s\n", debugstr_variant(l), debugstr_variant(r)); - /* FIXME: Fix comparing string to number */ + /* VarCmp would use string comparison; VBScript converts the string to a number. */ + if((V_VT(l) == VT_BSTR && is_numeric_vt(V_VT(r))) || + (V_VT(r) == VT_BSTR && is_numeric_vt(V_VT(l)))) { + double dl, dr; + HRESULT hres; + + hres = to_double(l, &dl); + if(FAILED(hres)) + return hres; + hres = to_double(r, &dr); + if(FAILED(hres)) + return hres; + if(dl < dr) + return VARCMP_LT; + if(dl > dr) + return VARCMP_GT; + return VARCMP_EQ; + } return VarCmp(l, r, ctx->script->lcid, 0); - } +} static HRESULT cmp_oper(exec_ctx_t *ctx) { @@ -2257,8 +2280,13 @@ static HRESULT interp_case(exec_ctx_t *ctx) hres = var_cmp(ctx, stack_top(ctx, 0), v.v); release_val(&v); - if(FAILED(hres)) + if(FAILED(hres)) { + if(hres == DISP_E_TYPEMISMATCH) { + ctx->instr++; + return S_OK; + } return hres; + } if(hres == VARCMP_EQ) { stack_popn(ctx, 1); diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index d139a2bf282..01485f913f2 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -117,6 +117,41 @@ Call ok(1 <> Not 0 = 0, "1 <> Not 0 = 0 should be true") Call ok(0 = Not 1 = 1, "0 = Not 1 = 1 should be true") Call ok(1 > Not 5 > 3, "1 > Not 5 > 3 should be true") Call ok(Not false And false = false, "Not false And false should be false") +x = "0" +Call ok(not (x > 30), """0"" > 30 should be false") +Call ok(x < 30, """0"" < 30 should be true") +Call ok(not ("0" > 30), """0"" > 30 literal should be false") +Call ok("0" < 30, """0"" < 30 literal should be true") +Call ok("10" > 9, """10"" > 9 should be true") +Call ok(not ("10" > 100), """10"" > 100 should be false") +Call ok("10" < 100, """10"" < 100 should be true") +Call ok("42" = 42, """42"" = 42 should be true") +Call ok(not ("42" = 43), """42"" = 43 should be false") +Call ok(doubleAsString(0.5) > 0, """0.5"" > 0 should be true") +Call ok(doubleAsString(0.5) < 1, """0.5"" < 1 should be true") +Call ok(doubleAsString(3.5) = 3.5, """3.5"" = 3.5 should be true") +Call ok(not (doubleAsString(1.5) > 2), """1.5"" > 2 should be false") +Call ok(doubleAsString(2.5) > 2, """2.5"" > 2 should be true") +Call ok(30 > "0", "30 > ""0"" should be true") +Call ok(9 < "10", "9 < ""10"" should be true") +Call ok(42 = "42", "42 = ""42"" should be true") +Call ok(not ("10" > "9"), """10"" > ""9"" should be false (string comparison)") +' String vs Boolean uses string comparison, not numeric conversion +Call ok(not ("1" = True), """1"" = True should be false") +Call ok(not ("-1" = True), """-1"" = True should be false") +Call ok(not ("0" = False), """0"" = False should be false") +' Non-numeric string compared to number should raise type mismatch +on error resume next +err.clear +x = ("abc" > 5) +Call ok(err.number = 13, """abc"" > 5 err.number = " & err.number) +err.clear +x = ("" > 0) +Call ok(err.number = 13, """"" > 0 err.number = " & err.number) +err.clear +x = (5 > "abc") +Call ok(err.number = 13, "5 > ""abc"" err.number = " & err.number) +on error goto 0 Call ok(getVT(false) = "VT_BOOL", "getVT(false) is not VT_BOOL") Call ok(getVT(true) = "VT_BOOL", "getVT(true) is not VT_BOOL") diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index e9eae6b5819..6ff6e8ff8eb 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -179,6 +179,7 @@ HRESULT create_func_ref(script_ctx_t*,function_t*,IDispatch**); function_t *script_disp_find_func(ScriptDisp*,const WCHAR*); HRESULT to_int(VARIANT*,int*); +HRESULT to_double(VARIANT*,double*); static inline unsigned arg_cnt(const DISPPARAMS *dp) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10314
On Fri Apr 24 15:49:40 2026 +0000, Jacek Caban wrote:
We don't really need `VarCmp` for comparing numbers, I would suggest something like (with proper error handling): ``` if(is_numeric_vt(V_VT(l)) || is_numeric_vt(V_VT(r)) { to_double(l, &dl); to_double(r, &dr); if(dl < dr) return VARCMP_LT; if(dl > dr) return VARCMP_GT; return VARCMP_EQ; } ``` Implemented
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10314#note_137613
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10314
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)