[PATCH 0/2] MR6429: msi: Allow package template summary info strings without semicolon separator.

[Andrew Nguyen (＠arethusa)]

The OpenKiosk MSI installer specifies "x64" for PID_TEMPLATE in its summary information stream, which is rejected by Wine at package open time. While the examples of valid template strings in MSDN suggest that a semicolon is mandatory in a template string, tests of template string validation show that a platform may be specified in a template without the semicolon delimiter.

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

[PATCH 1/2] msi/tests: Add additional test cases for validating package template summary info strings.

[Andrew Nguyen]

From: Andrew Nguyen

---
 dlls/msi/tests/install.c | 138 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 138 insertions(+)

diff --git a/dlls/msi/tests/install.c b/dlls/msi/tests/install.c
index c3d0ed2d16b..d29eedf78a3 100644
--- a/dlls/msi/tests/install.c
+++ b/dlls/msi/tests/install.c
@@ -5547,6 +5547,73 @@ static void test_package_validation(void)
     ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
     ok(delete_pf("msitest", FALSE), "directory does not exist\n");

+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "");
+
+    r = MsiInstallProductA(msifile, NULL);
+    todo_wine
+    {
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+    }
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, ";");
+
+    r = MsiInstallProductA(msifile, NULL);
+    ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+    ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+    ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Intel");
+
+    r = MsiInstallProductA(msifile, NULL);
+    todo_wine
+    {
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+    }
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Intel;");
+
+    r = MsiInstallProductA(msifile, NULL);
+    ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+    ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+    ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Alpha,,Intel;");
+
+    r = MsiInstallProductA(msifile, NULL);
+    ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+    ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+    ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, ",;");
+
+    r = MsiInstallProductA(msifile, NULL);
+    todo_wine
+    ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "Alpha,,Beta;");
+
+    r = MsiInstallProductA(msifile, NULL);
+    todo_wine
+    ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
+
+    DeleteFileA(msifile);
+    create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "0");
+
+    r = MsiInstallProductA(msifile, NULL);
+    todo_wine
+    ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
+
     DeleteFileA(msifile);
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "Intel,9999;9999");

@@ -5643,6 +5710,25 @@ static void test_package_validation(void)
         ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
         ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
         ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64;");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64");
+
+        r = MsiInstallProductA(msifile, NULL);
+        todo_wine
+        {
+            ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+            ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+            ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+        }
     }
     else if (is_wow64)
     {
@@ -5669,6 +5755,25 @@ static void test_package_validation(void)
         ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
         ok(delete_pf("msitest\maximus", TRUE), "file exists\n");
         ok(delete_pf("msitest", FALSE), "directory exists\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64;");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file exists\n");
+        ok(delete_pf("msitest", FALSE), "directory exists\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64");
+
+        r = MsiInstallProductA(msifile, NULL);
+        todo_wine
+        {
+            ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+            ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+            ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+        }
     }
     else
     {
@@ -5680,6 +5785,7 @@ static void test_package_validation(void)
         ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
         ok(delete_pf("msitest", FALSE), "directory does not exist\n");

+
         DeleteFileA(msifile);
         create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Alpha,Beta,Intel;0");

@@ -5688,6 +5794,22 @@ static void test_package_validation(void)
         ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
         ok(delete_pf("msitest", FALSE), "directory does not exist\n");

+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Alpha,Beta,Intel;");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Alpha,Beta,Intel");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
+
         DeleteFileA(msifile);
         create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "x64;0");

@@ -5703,6 +5825,22 @@ static void test_package_validation(void)
         ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
         ok(!delete_pf("msitest\maximus", TRUE), "file exists\n");
         ok(!delete_pf("msitest", FALSE), "directory exists\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64;");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
+        ok(!delete_pf("msitest\maximus", TRUE), "file exists\n");
+        ok(!delete_pf("msitest", FALSE), "directory exists\n");
+
+        DeleteFileA(msifile);
+        create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64");
+
+        r = MsiInstallProductA(msifile, NULL);
+        ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);
+        ok(!delete_pf("msitest\maximus", TRUE), "file exists\n");
+        ok(!delete_pf("msitest", FALSE), "directory exists\n");
     }

error:

[PATCH 2/2] msi: Allow package template summary info strings without semicolon separator.

[Andrew Nguyen]

From: Andrew Nguyen

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52687
---
 dlls/msi/package.c       | 13 ++++---------
 dlls/msi/tests/install.c | 37 ++++++++++++-------------------------
 2 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/dlls/msi/package.c b/dlls/msi/package.c
index fb5463823e3..617a937af2c 100644
--- a/dlls/msi/package.c
+++ b/dlls/msi/package.c
@@ -1107,13 +1107,8 @@ static UINT parse_suminfo( MSISUMMARYINFO *si, MSIPACKAGE *package )
     TRACE("template: %s\n", debugstr_w(template));

p = wcschr( template, ';' );
-    if (!p)
-    {
-        WARN("invalid template string %s\n", debugstr_w(template));
-        free( template );
-        return ERROR_PATCH_PACKAGE_INVALID;
-    }
-    *p = 0;
+    if (p) *p++ = 0;
+
     platform = template;
     if ((q = wcschr( platform, ',' ))) *q = 0;
     package->platform = parse_platform( platform );
@@ -1129,8 +1124,8 @@ static UINT parse_suminfo( MSISUMMARYINFO *si, MSIPACKAGE *package )
         free( template );
         return ERROR_INSTALL_PLATFORM_UNSUPPORTED;
     }
-    p++;
-    if (!*p)
+
+    if (!p || !*p)
     {
         free( template );
         return ERROR_SUCCESS;
diff --git a/dlls/msi/tests/install.c b/dlls/msi/tests/install.c
index d29eedf78a3..3bf28cc1e54 100644
--- a/dlls/msi/tests/install.c
+++ b/dlls/msi/tests/install.c
@@ -5551,12 +5551,9 @@ static void test_package_validation(void)
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "");

r = MsiInstallProductA(msifile, NULL);
-    todo_wine
-    {
-        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
-        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
-        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
-    }
+    ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+    ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+    ok(delete_pf("msitest", FALSE), "directory does not exist\n");

DeleteFileA(msifile);
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, ";");
@@ -5570,12 +5567,9 @@ static void test_package_validation(void)
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Intel");

r = MsiInstallProductA(msifile, NULL);
-    todo_wine
-    {
-        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
-        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
-        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
-    }
+    ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+    ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+    ok(delete_pf("msitest", FALSE), "directory does not exist\n");

DeleteFileA(msifile);
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 100, "Intel;");
@@ -5611,7 +5605,6 @@ static void test_package_validation(void)
     create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "0");

r = MsiInstallProductA(msifile, NULL);
-    todo_wine
     ok(r == ERROR_INSTALL_PLATFORM_UNSUPPORTED, "Expected ERROR_INSTALL_PLATFORM_UNSUPPORTED, got %u\n", r);

DeleteFileA(msifile);
@@ -5723,12 +5716,9 @@ static void test_package_validation(void)
         create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64");

r = MsiInstallProductA(msifile, NULL);
-        todo_wine
-        {
-            ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
-            ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
-            ok(delete_pf("msitest", FALSE), "directory does not exist\n");
-        }
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
     }
     else if (is_wow64)
     {
@@ -5768,12 +5758,9 @@ static void test_package_validation(void)
         create_database_template(msifile, pv_tables, ARRAY_SIZE(pv_tables), 200, "x64");

r = MsiInstallProductA(msifile, NULL);
-        todo_wine
-        {
-            ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
-            ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
-            ok(delete_pf("msitest", FALSE), "directory does not exist\n");
-        }
+        ok(r == ERROR_SUCCESS, "Expected ERROR_SUCCESS, got %u\n", r);
+        ok(delete_pf("msitest\maximus", TRUE), "file does not exist\n");
+        ok(delete_pf("msitest", FALSE), "directory does not exist\n");
     }
     else
     {

[PATCH 2/2] msi: Allow package template summary info strings without semicolon separator.

[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=148154

Your paranoid android.

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

winmm:
mci: Timeout

[PATCH 0/2] MR6429: msi: Allow package template summary info strings without semicolon separator. - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.