[PATCH v7 0/4] MR10960: scrrun/dictionary: Match native semantics for Empty and Boolean keys.
An uninitialized VBScript variable is Empty, so scripts can end up using Empty as a dictionary key alongside a numeric 0 or "". Native treats those as the same key; Wine kept Empty distinct from every other type. Match an Empty key against any numeric zero or the empty string, while keeping it distinct from Null and from non-zero/non-empty keys. A Boolean key was rejected outright (E_NOTIMPL), breaking Exists/Remove when a script uses a comparison result as a key; hash it by its numeric value like the other numeric types. -- v7: scrrun/dictionary: Match native key semantics for Boolean keys. scrrun/tests: Add tests for Boolean dictionary keys. scrrun/dictionary: Match Empty keys against zero-valued and empty-string keys. scrrun/tests: Add tests for Empty dictionary keys. https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> An Empty key matches the zero/default value of the other key's type: any numeric zero or the empty string, but not a non-zero number, a non-empty string, or Null. --- dlls/scrrun/tests/dictionary.c | 107 +++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 3c052caed0e..f6b2f967196 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -1269,6 +1269,112 @@ static void test_put_Key(void) IDictionary_Release(dict); } +/* Add two keys to a fresh dictionary; returns TRUE if the second key is + * treated as a duplicate of the first. */ +static BOOL keys_match(VARIANT *key1, VARIANT *key2) +{ + IDictionary *dict; + VARIANT item; + HRESULT hr; + + if (FAILED(CoCreateInstance(&CLSID_Dictionary, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, + &IID_IDictionary, (void**)&dict))) + return FALSE; + + V_VT(&item) = VT_I2; + V_I2(&item) = 1; + if (FAILED(IDictionary_Add(dict, key1, &item))) + { + IDictionary_Release(dict); + return FALSE; + } + + hr = IDictionary_Add(dict, key2, &item); + IDictionary_Release(dict); + return hr == CTL_E_KEY_ALREADY_EXISTS; +} + +static void test_empty_key(void) +{ + static const VARTYPE zero_keys[] = { + VT_I2, VT_I4, VT_UI1, VT_R4, VT_R8, VT_DATE, + }; + VARIANT_BOOL exists; + IDictionary *dict; + VARIANT a, b, item; + HRESULT hr; + unsigned i; + + /* Empty matches the zero/default value of any numeric type. */ + for (i = 0; i < ARRAY_SIZE(zero_keys); i++) + { + V_VT(&a) = VT_EMPTY; + V_VT(&b) = zero_keys[i]; + if (zero_keys[i] == VT_R8 || zero_keys[i] == VT_DATE) + V_R8(&b) = 0.0; + else if (zero_keys[i] == VT_R4) + V_R4(&b) = 0.0f; + else + V_I4(&b) = 0; + todo_wine ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); + } + + /* Empty matches the empty string, but not a non-empty one. */ + V_VT(&a) = VT_EMPTY; + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L""); + todo_wine ok(keys_match(&a, &b), "Empty should match empty string\n"); + VariantClear(&b); + + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L"0"); + ok(!keys_match(&a, &b), "Empty should not match \"0\"\n"); + VariantClear(&b); + + /* Empty does not match a non-zero number or Null. */ + V_VT(&b) = VT_I2; + V_I2(&b) = 5; + ok(!keys_match(&a, &b), "Empty should not match I2 5\n"); + + V_VT(&b) = VT_NULL; + ok(!keys_match(&a, &b), "Empty should not match Null\n"); + + /* A numeric zero does not match the empty string (only Empty bridges). */ + V_VT(&a) = VT_I2; + V_I2(&a) = 0; + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L""); + ok(!keys_match(&a, &b), "I2 0 should not match empty string\n"); + VariantClear(&b); + + /* Null matches only Null. */ + V_VT(&a) = VT_NULL; + V_VT(&b) = VT_NULL; + ok(keys_match(&a, &b), "Null should match Null\n"); + + /* Exists and Remove accept an Empty key, matching a zero-valued entry. */ + hr = CoCreateInstance(&CLSID_Dictionary, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, + &IID_IDictionary, (void**)&dict); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + V_VT(&a) = VT_I2; + V_I2(&a) = 0; + VariantInit(&item); + hr = IDictionary_Add(dict, &a, &item); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + V_VT(&b) = VT_EMPTY; + exists = VARIANT_FALSE; + hr = IDictionary_Exists(dict, &b, &exists); + ok(hr == S_OK, "Exists with Empty key: %#lx.\n", hr); + todo_wine ok(exists == VARIANT_TRUE, "Empty should find I2 0, got %x\n", exists); + + hr = IDictionary_Remove(dict, &b); + todo_wine ok(hr == S_OK, "Remove with Empty key: %#lx.\n", hr); + + IDictionary_Release(dict); +} + static void test_IEnumVARIANT(void) { IUnknown *enum1, *enum2; @@ -1451,6 +1557,7 @@ START_TEST(dictionary) test_Add(); test_object_key_hashfail(); test_put_Key(); + test_empty_key(); test_IEnumVARIANT(); test_putref_Item(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> An uninitialized VBScript variable is Empty, so scripts can end up using Empty as a dictionary key alongside a numeric 0 or "". Native treats those as the same key; Wine kept Empty distinct from every other type. Match an Empty key against any numeric zero or the empty string, while keeping it distinct from Null and from non-zero/non-empty keys. --- dlls/scrrun/dictionary.c | 35 +++++++++++++++++++++++++++++++--- dlls/scrrun/tests/dictionary.c | 8 ++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c index f1bd646e29e..b4fa54ddec4 100644 --- a/dlls/scrrun/dictionary.c +++ b/dlls/scrrun/dictionary.c @@ -162,9 +162,38 @@ static inline BOOL numeric_key_eq(const VARIANT *key1, const VARIANT *key2) return V_R4(&v1) == V_R4(&v2); } +/* Empty matches the zero/default value of the other key's type: any numeric + * zero, the empty string, or another Empty. */ +static BOOL empty_matches_key(const VARIANT *key) +{ + VARIANT v; + + switch (V_VT(key)) + { + case VT_EMPTY: + return TRUE; + case VT_BSTR: + { + const WCHAR *str = get_key_strptr(key); + return !str || !*str; + } + default: + if (!is_numeric_key(key)) + return FALSE; + VariantInit(&v); + if (FAILED(VariantChangeType(&v, key, 0, VT_R4))) + return FALSE; + return V_R4(&v) == 0.0f; + } +} + static BOOL is_matching_key(const struct dictionary *dict, const struct keyitem_pair *pair, const VARIANT *key, DWORD hash) { - if (is_string_key(key) != is_string_key(&pair->key)) + if (V_VT(key) == VT_EMPTY || V_VT(&pair->key) == VT_EMPTY) + { + return empty_matches_key(V_VT(key) == VT_EMPTY ? &pair->key : key); + } + else if (is_string_key(key) != is_string_key(&pair->key)) { return FALSE; } @@ -188,9 +217,9 @@ static BOOL is_matching_key(const struct dictionary *dict, const struct keyitem_ { return hash == pair->hash && numeric_key_eq(key, &pair->key); } - else if (V_VT(&pair->key) == VT_EMPTY || V_VT(&pair->key) == VT_NULL) + else if (V_VT(&pair->key) == VT_NULL) { - return V_VT(&pair->key) == V_VT(key); + return V_VT(key) == VT_NULL; } else { diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index f6b2f967196..0d5f86d6dd9 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -1316,14 +1316,14 @@ static void test_empty_key(void) V_R4(&b) = 0.0f; else V_I4(&b) = 0; - todo_wine ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); + ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); } /* Empty matches the empty string, but not a non-empty one. */ V_VT(&a) = VT_EMPTY; V_VT(&b) = VT_BSTR; V_BSTR(&b) = SysAllocString(L""); - todo_wine ok(keys_match(&a, &b), "Empty should match empty string\n"); + ok(keys_match(&a, &b), "Empty should match empty string\n"); VariantClear(&b); V_VT(&b) = VT_BSTR; @@ -1367,10 +1367,10 @@ static void test_empty_key(void) exists = VARIANT_FALSE; hr = IDictionary_Exists(dict, &b, &exists); ok(hr == S_OK, "Exists with Empty key: %#lx.\n", hr); - todo_wine ok(exists == VARIANT_TRUE, "Empty should find I2 0, got %x\n", exists); + ok(exists == VARIANT_TRUE, "Empty should find I2 0, got %x\n", exists); hr = IDictionary_Remove(dict, &b); - todo_wine ok(hr == S_OK, "Remove with Empty key: %#lx.\n", hr); + ok(hr == S_OK, "Remove with Empty key: %#lx.\n", hr); IDictionary_Release(dict); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> A Boolean key hashes by its numeric value and matches the equal numeric value across types (True is -1, False is 0); False is the zero/default value, so it matches an Empty key. A script can use a comparison result (a Boolean) as the key to Exists and Remove, which must not fail. --- dlls/scrrun/tests/dictionary.c | 76 ++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 0d5f86d6dd9..2da3ae7bb89 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -828,6 +828,22 @@ if (0) { /* crashes on native */ ok(V_VT(&hash) == VT_I4, "got %d\n", V_VT(&hash)); ok(V_I4(&hash) == expected, "got hash %#lx, expected %#lx\n", V_I4(&hash), expected); + V_VT(&key) = VT_BOOL; + V_BOOL(&key) = VARIANT_FALSE; + V_I4(&hash) = 5678; + hr = IDictionary_get_HashVal(dict, &key, &hash); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(V_VT(&hash) == VT_I4, "Unexpected hash type %d.\n", V_VT(&hash)); + todo_wine ok(V_I4(&hash) == get_num_hash(0.0f), "Unexpected hash value %#lx.\n", V_I4(&hash)); + + V_VT(&key) = VT_BOOL; + V_BOOL(&key) = VARIANT_TRUE; + V_I4(&hash) = 5678; + hr = IDictionary_get_HashVal(dict, &key, &hash); + todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(V_VT(&hash) == VT_I4, "Unexpected hash type %d.\n", V_VT(&hash)); + todo_wine ok(V_I4(&hash) == get_num_hash((FLOAT)VARIANT_TRUE), "Unexpected hash value %#lx.\n", V_I4(&hash)); + V_VT(&key) = VT_EMPTY; V_I4(&key) = 1234; V_I4(&hash) = 5678; @@ -1375,6 +1391,65 @@ static void test_empty_key(void) IDictionary_Release(dict); } +static void test_bool_key(void) +{ + VARIANT_BOOL exists; + IDictionary *dict; + VARIANT a, b, item; + HRESULT hr; + + /* Boolean keys match the equal numeric value across types: True is -1, + * False is 0. */ + V_VT(&a) = VT_BOOL; + V_BOOL(&a) = VARIANT_TRUE; + V_VT(&b) = VT_I2; + V_I2(&b) = -1; + todo_wine ok(keys_match(&a, &b), "True should match I2 -1\n"); + + V_VT(&a) = VT_BOOL; + V_BOOL(&a) = VARIANT_FALSE; + V_VT(&b) = VT_I2; + V_I2(&b) = 0; + todo_wine ok(keys_match(&a, &b), "False should match I2 0\n"); + + /* False is the zero/default value, so it matches an Empty key. */ + V_VT(&a) = VT_BOOL; + V_BOOL(&a) = VARIANT_FALSE; + V_VT(&b) = VT_EMPTY; + todo_wine ok(keys_match(&a, &b), "False should match Empty\n"); + + /* True and False are distinct keys. */ + V_VT(&a) = VT_BOOL; + V_BOOL(&a) = VARIANT_TRUE; + V_VT(&b) = VT_BOOL; + V_BOOL(&b) = VARIANT_FALSE; + ok(!keys_match(&a, &b), "True should not match False\n"); + + /* A script can use a comparison result (a Boolean) as the key to Exists + * and Remove. */ + hr = CoCreateInstance(&CLSID_Dictionary, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, + &IID_IDictionary, (void**)&dict); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + V_VT(&a) = VT_I2; + V_I2(&a) = -1; + VariantInit(&item); + hr = IDictionary_Add(dict, &a, &item); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + V_VT(&b) = VT_BOOL; + V_BOOL(&b) = VARIANT_TRUE; + exists = VARIANT_FALSE; + hr = IDictionary_Exists(dict, &b, &exists); + todo_wine ok(hr == S_OK, "Exists with Boolean key: %#lx.\n", hr); + todo_wine ok(exists == VARIANT_TRUE, "True should find I2 -1, got %x\n", exists); + + hr = IDictionary_Remove(dict, &b); + todo_wine ok(hr == S_OK, "Remove with Boolean key: %#lx.\n", hr); + + IDictionary_Release(dict); +} + static void test_IEnumVARIANT(void) { IUnknown *enum1, *enum2; @@ -1558,6 +1633,7 @@ START_TEST(dictionary) test_object_key_hashfail(); test_put_Key(); test_empty_key(); + test_bool_key(); test_IEnumVARIANT(); test_putref_Item(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> Boolean keys were rejected outright by the hash function, returning E_NOTIMPL, so Exists and Remove failed for them. An uninitialized or comparison-result VBScript value can be a Boolean, so a script can end up using one as a dictionary key. Hash it by its numeric value like the other numeric types, so True and False match the equal numeric value across types (and False matches an Empty key). --- dlls/scrrun/dictionary.c | 5 +++++ dlls/scrrun/tests/dictionary.c | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c index b4fa54ddec4..c35ca6a44b8 100644 --- a/dlls/scrrun/dictionary.c +++ b/dlls/scrrun/dictionary.c @@ -119,6 +119,7 @@ static inline BOOL is_numeric_key(const VARIANT *key) case VT_DATE: case VT_R4: case VT_R8: + case VT_BOOL: return TRUE; default: return FALSE; @@ -995,6 +996,10 @@ static HRESULT WINAPI dictionary_get_HashVal(IDictionary *iface, VARIANT *key, V case VT_R8|VT_BYREF: case VT_R8: return get_flt_hash(V_VT(key) & VT_BYREF ? *V_R8REF(key) : V_R8(key), &V_I4(hash)); + case VT_BOOL|VT_BYREF: + case VT_BOOL: + V_I4(hash) = get_num_hash(V_VT(key) & VT_BYREF ? *V_BOOLREF(key) : V_BOOL(key)); + break; case VT_EMPTY: case VT_NULL: V_I4(hash) = 0; diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 2da3ae7bb89..ea0b2dd3622 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -832,17 +832,17 @@ if (0) { /* crashes on native */ V_BOOL(&key) = VARIANT_FALSE; V_I4(&hash) = 5678; hr = IDictionary_get_HashVal(dict, &key, &hash); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(V_VT(&hash) == VT_I4, "Unexpected hash type %d.\n", V_VT(&hash)); - todo_wine ok(V_I4(&hash) == get_num_hash(0.0f), "Unexpected hash value %#lx.\n", V_I4(&hash)); + ok(V_I4(&hash) == get_num_hash(0.0f), "Unexpected hash value %#lx.\n", V_I4(&hash)); V_VT(&key) = VT_BOOL; V_BOOL(&key) = VARIANT_TRUE; V_I4(&hash) = 5678; hr = IDictionary_get_HashVal(dict, &key, &hash); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); ok(V_VT(&hash) == VT_I4, "Unexpected hash type %d.\n", V_VT(&hash)); - todo_wine ok(V_I4(&hash) == get_num_hash((FLOAT)VARIANT_TRUE), "Unexpected hash value %#lx.\n", V_I4(&hash)); + ok(V_I4(&hash) == get_num_hash((FLOAT)VARIANT_TRUE), "Unexpected hash value %#lx.\n", V_I4(&hash)); V_VT(&key) = VT_EMPTY; V_I4(&key) = 1234; @@ -1404,19 +1404,19 @@ static void test_bool_key(void) V_BOOL(&a) = VARIANT_TRUE; V_VT(&b) = VT_I2; V_I2(&b) = -1; - todo_wine ok(keys_match(&a, &b), "True should match I2 -1\n"); + ok(keys_match(&a, &b), "True should match I2 -1\n"); V_VT(&a) = VT_BOOL; V_BOOL(&a) = VARIANT_FALSE; V_VT(&b) = VT_I2; V_I2(&b) = 0; - todo_wine ok(keys_match(&a, &b), "False should match I2 0\n"); + ok(keys_match(&a, &b), "False should match I2 0\n"); /* False is the zero/default value, so it matches an Empty key. */ V_VT(&a) = VT_BOOL; V_BOOL(&a) = VARIANT_FALSE; V_VT(&b) = VT_EMPTY; - todo_wine ok(keys_match(&a, &b), "False should match Empty\n"); + ok(keys_match(&a, &b), "False should match Empty\n"); /* True and False are distinct keys. */ V_VT(&a) = VT_BOOL; @@ -1441,11 +1441,11 @@ static void test_bool_key(void) V_BOOL(&b) = VARIANT_TRUE; exists = VARIANT_FALSE; hr = IDictionary_Exists(dict, &b, &exists); - todo_wine ok(hr == S_OK, "Exists with Boolean key: %#lx.\n", hr); - todo_wine ok(exists == VARIANT_TRUE, "True should find I2 -1, got %x\n", exists); + ok(hr == S_OK, "Exists with Boolean key: %#lx.\n", hr); + ok(exists == VARIANT_TRUE, "True should find I2 -1, got %x\n", exists); hr = IDictionary_Remove(dict, &b); - todo_wine ok(hr == S_OK, "Remove with Boolean key: %#lx.\n", hr); + ok(hr == S_OK, "Remove with Boolean key: %#lx.\n", hr); IDictionary_Release(dict); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
rebased and conflicts resolved -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10960#note_141965
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)