[PATCH v3 0/2] MR10960: scrrun/dictionary: Match Empty keys against zero-valued and empty-string keys.

[Francis De Brabandere (＠francisdb)]

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.

--
  v3: 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

[PATCH v3 1/2] scrrun/tests: Add tests for Empty dictionary keys.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

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 | 82 ++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c
index 8fc369e7062..3c6c44017c7 100644
--- a/dlls/scrrun/tests/dictionary.c
+++ b/dlls/scrrun/tests/dictionary.c
@@ -1134,6 +1134,87 @@ static void test_Add(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 a, b;
+    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");
+}
+
 static void test_IEnumVARIANT(void)
 {
     IUnknown *enum1, *enum2;
@@ -1315,6 +1396,7 @@ START_TEST(dictionary)
     test_Item();
     test_Add();
     test_object_key_hashfail();
+    test_empty_key();
     test_IEnumVARIANT();
     test_putref_Item();
 
-- 
GitLab


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

[PATCH v3 2/2] scrrun/dictionary: Match Empty keys against zero-valued and empty-string keys.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

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 |  4 ++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c
index 1b618865360..8137855b592 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 3c6c44017c7..7531ade6802 100644
--- a/dlls/scrrun/tests/dictionary.c
+++ b/dlls/scrrun/tests/dictionary.c
@@ -1178,14 +1178,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;
-- 
GitLab

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