Fix WcaErrorMessage `cArgs==-1` case.
- Count args before creating message record. - Document terminating NULL requirement. - Add terminating NULL in MessageExit* macros. - Enhance tests for problems encountered fixing this nightmare. Fixes https://github.com/wixtoolset/issues/issues/7422. Fixes https://github.com/wixtoolset/issues/issues/7444.
Bob Arnson committed
Apr 24, 2023 at 22:16 UTC
b3b9decfab8a26851e2bc85d777824220ff804b6
4 files changed
+26
-17
src/libs/wcautil/WixToolset.WcaUtil/inc/wcautil.h
+3
-3
@@ -15,9 +15,9 @@ extern "C" {
15
16
#include "dutil.h"
17
18
-#define MessageExitOnLastErrorSource(d, x, e, s, ...) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__); goto LExit; } }
19
-#define MessageExitOnFailureSource(d, x, e, s, ...) if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, INSTALLMESSAGE_ERROR | MB_OK, -1, __VA_ARGS__); goto LExit; }
20
-#define MessageExitOnNullWithLastErrorSource(d, p, x, e, s, ...) if (NULL == p) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (!FAILED(x)) { x = E_FAIL; } ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__); goto LExit; }
18
+#define MessageExitOnLastErrorSource(d, x, e, s, ...) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__, NULL); goto LExit; } }
19
+#define MessageExitOnFailureSource(d, x, e, s, ...) if (FAILED(x)) { ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, INSTALLMESSAGE_ERROR | MB_OK, -1, __VA_ARGS__, NULL); goto LExit; }
20
+#define MessageExitOnNullWithLastErrorSource(d, p, x, e, s, ...) if (NULL == p) { x = ::GetLastError(); x = HRESULT_FROM_WIN32(x); if (!FAILED(x)) { x = E_FAIL; } ExitTraceSource(d, x, s, __VA_ARGS__); WcaErrorMessage(e, x, MB_OK, -1, __VA_ARGS__, NULL); goto LExit; }
21
22
#define MessageExitOnLastError(x, e, s, ...) MessageExitOnLastErrorSource(DUTIL_SOURCE_DEFAULT, x, e, s, __VA_ARGS__)
23
#define MessageExitOnFailure(x, e, s, ...) MessageExitOnFailureSource(DUTIL_SOURCE_DEFAULT, x, e, s, __VA_ARGS__)
src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp
+20
-13
@@ -27,7 +27,9 @@ WcaErrorMessage() - sends an error message from the CustomAction using
27
the Error table
28
29
NOTE: Any and all var_args (...) must be WCHAR*
30
- If you pass -1 to cArgs the count will be determined
30
+ If you pass -1 to cArgs, the count will be determined by
31
+ looking for a trailing NULL argment. If you omit a terminating
32
+ NULL, the results are undefined and probably crashy.
33
********************************************************************/
34
extern "C" UINT __cdecl WcaErrorMessage(
35
__in int iError,
@@ -41,6 +43,22 @@ extern "C" UINT __cdecl WcaErrorMessage(
43
MSIHANDLE hRec = NULL;
44
va_list args = NULL;
45
46
+ if (-1 == cArgs)
47
+ {
48
+ LPCWSTR wzArg = NULL;
49
+ va_list iter = NULL;
50
+
51
+ va_start(iter, cArgs);
52
+ cArgs = 0;
53
+
54
+ while (NULL != (wzArg = va_arg(iter, WCHAR*)) && L'\0' != *wzArg)
55
+ {
56
+ ++cArgs;
57
+ }
58
+
59
+ va_end(iter);
60
+ }
61
+
62
uiType |= INSTALLMESSAGE_ERROR; // ensure error type is set
63
hRec = ::MsiCreateRecord(cArgs + 2);
64
if (!hRec)
@@ -56,18 +74,6 @@ extern "C" UINT __cdecl WcaErrorMessage(
74
ExitOnFailure(HRESULT_FROM_WIN32(er), "failed to set hresult code into error message");
75
76
va_start(args, cArgs);
59
- if (-1 == cArgs)
60
- {
61
- LPCWSTR wzArg = NULL;
62
- va_list iter = args;
63
- cArgs = 0;
64
-
65
- while (NULL != (wzArg = va_arg(iter, WCHAR*)) && L'\0' != *wzArg)
66
- {
67
- ++cArgs;
68
- }
69
- }
70
-
77
for (INT i = 0; i < cArgs; i++)
78
{
79
er = ::MsiRecordSetStringW(hRec, i + 3, va_arg(args, WCHAR*));
@@ -76,6 +82,7 @@ extern "C" UINT __cdecl WcaErrorMessage(
82
va_end(args);
83
84
er = WcaProcessMessage(static_cast<INSTALLMESSAGE>(uiType), hRec);
85
+
86
LExit:
87
if (args)
88
{
src/test/burn/WixTestTools/MSIExec.cs
+1
-1
@@ -111,7 +111,7 @@ namespace WixTestTools
111
this.ForceRestart = false;
112
this.PromptRestart = false;
113
this.LogFile = String.Empty;
114
- this.LoggingOptions = MSIExecLoggingOptions.VOICEWARMUP;
114
+ this.LoggingOptions = MSIExecLoggingOptions.Log_All_Information | MSIExecLoggingOptions.Verbose_Output | MSIExecLoggingOptions.Extra_Debugging_Information; // `/l*vx`
115
this.OtherArguments = String.Empty;
116
}
117
src/test/msi/TestData/UtilExtensionUserTests/ProductNonVitalUserGroup/NonVitalUserGroup.wxs
+2
@@ -10,12 +10,14 @@
10
11
<Fragment>
12
<util:Group Id="ShouldNotExist" Name="Should Not Exist" />
13
+ <util:Group Id="AlsoShouldNotExist" Name="Also Should Not Exist" />
14
15
<Component Id="Component1" Guid="00030829-0000-0000-C000-000000000046" Directory="INSTALLFOLDER">
16
<File Source="$(sys.SOURCEFILEPATH)" KeyPath="yes" />
17
18
<util:User Id="CurrentUser" Name="[LogonUser]" Domain="[%USERDOMAIN]" RemoveOnUninstall="no" Vital="no">
19
<util:GroupRef Id="ShouldNotExist" />
20
+ <util:GroupRef Id="AlsoShouldNotExist" />
21
</util:User>
22
</Component>
23
</Fragment>