@joebigelow / wix / commits / b5553689

WIXFEAT:6258 - Format variables when evaluating condition.

Sean Hall committed Nov 1, 2020 at 14:16 UTC b5553689ed1b1bd32f854654f56935c039a9b13b
6 files changed +112 -55
src/engine/condition.cpp
+8 -3
@@ -432,6 +432,7 @@ static HRESULT ParseOperand(
432 )
433 {
434 HRESULT hr = S_OK;
435 + LPWSTR sczFormatted = NULL;
436
437 // Symbols don't encrypt their value, so can access the value directly.
438 switch (pContext->NextSymbol.Type)
@@ -451,9 +452,11 @@ static HRESULT ParseOperand(
452
453 if (BURN_VARIANT_TYPE_FORMATTED == pOperand->Value.Type)
454 {
454 - // TODO: actually format the value?
455 - hr = BVariantChangeType(&pOperand->Value, BURN_VARIANT_TYPE_STRING);
456 - ExitOnRootFailure(hr, "Failed to change variable '%ls' type for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
455 + hr = VariableGetFormatted(pContext->pVariables, pContext->NextSymbol.Value.sczValue, &sczFormatted, &pOperand->fHidden);
456 + ExitOnRootFailure(hr, "Failed to format variable '%ls' for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
457 +
458 + hr = BVariantSetString(&pOperand->Value, sczFormatted, 0, FALSE);
459 + ExitOnRootFailure(hr, "Failed to store formatted value for variable '%ls' for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
460 }
461 break;
462
@@ -477,6 +480,8 @@ static HRESULT ParseOperand(
480 ExitOnFailure(hr, "Failed to read next symbol.");
481
482 LExit:
483 + StrSecureZeroFreeString(sczFormatted);
484 +
485 return hr;
486 }
487
src/engine/variable.cpp
+75 -39
@@ -54,7 +54,14 @@ static HRESULT FormatString(
54 __in_z LPCWSTR wzIn,
55 __out_z_opt LPWSTR* psczOut,
56 __out_opt DWORD* pcchOut,
57 - __in BOOL fObfuscateHiddenVariables
57 + __in BOOL fObfuscateHiddenVariables,
58 + __out BOOL* pfContainsHiddenVariable
59 + );
60 +static HRESULT GetFormatted(
61 + __in BURN_VARIABLES* pVariables,
62 + __in_z LPCWSTR wzVariable,
63 + __out_z LPWSTR* psczValue,
64 + __out BOOL* pfContainsHiddenVariable
65 );
66 static HRESULT AddBuiltInVariable(
67 __in BURN_VARIABLES* pVariables,
@@ -629,43 +636,18 @@ LExit:
636 extern "C" HRESULT VariableGetFormatted(
637 __in BURN_VARIABLES* pVariables,
638 __in_z LPCWSTR wzVariable,
632 - __out_z LPWSTR* psczValue
639 + __out_z LPWSTR* psczValue,
640 + __out BOOL* pfContainsHiddenVariable
641 )
642 {
643 HRESULT hr = S_OK;
636 - BURN_VARIABLE* pVariable = NULL;
637 - LPWSTR scz = NULL;
638 -
639 - ::EnterCriticalSection(&pVariables->csAccess);
644
641 - hr = GetVariable(pVariables, wzVariable, &pVariable);
642 - if (SUCCEEDED(hr) && BURN_VARIANT_TYPE_NONE == pVariable->Value.Type)
645 + if (pfContainsHiddenVariable)
646 {
644 - ExitFunction1(hr = E_NOTFOUND);
645 - }
646 - else if (E_NOTFOUND == hr)
647 - {
648 - ExitFunction();
647 + *pfContainsHiddenVariable = FALSE;
648 }
650 - ExitOnFailure(hr, "Failed to get variable: %ls", wzVariable);
649
652 - if (BURN_VARIANT_TYPE_FORMATTED == pVariable->Value.Type)
653 - {
654 - hr = BVariantGetString(&pVariable->Value, &scz);
655 - ExitOnFailure(hr, "Failed to get unformatted string.");
656 -
657 - hr = VariableFormatString(pVariables, scz, psczValue, NULL);
658 - ExitOnFailure(hr, "Failed to format value '%ls' of variable: %ls", pVariable->fHidden ? L"*****" : pVariable->Value.sczValue, wzVariable);
659 - }
660 - else
661 - {
662 - hr = BVariantGetString(&pVariable->Value, psczValue);
663 - ExitOnFailure(hr, "Failed to get value as string for variable: %ls", wzVariable);
664 - }
665 -
666 -LExit:
667 - ::LeaveCriticalSection(&pVariables->csAccess);
668 - StrSecureZeroFreeString(scz);
650 + hr = GetFormatted(pVariables, wzVariable, psczValue, pfContainsHiddenVariable);
651
652 return hr;
653 }
@@ -736,7 +718,7 @@ extern "C" HRESULT VariableFormatString(
718 __out_opt DWORD* pcchOut
719 )
720 {
739 - return FormatString(pVariables, wzIn, psczOut, pcchOut, FALSE);
721 + return FormatString(pVariables, wzIn, psczOut, pcchOut, FALSE, NULL);
722 }
723
724 extern "C" HRESULT VariableFormatStringObfuscated(
@@ -746,7 +728,7 @@ extern "C" HRESULT VariableFormatStringObfuscated(
728 __out_opt DWORD* pcchOut
729 )
730 {
749 - return FormatString(pVariables, wzIn, psczOut, pcchOut, TRUE);
731 + return FormatString(pVariables, wzIn, psczOut, pcchOut, TRUE, NULL);
732 }
733
734 extern "C" HRESULT VariableEscapeString(
@@ -1116,7 +1098,8 @@ static HRESULT FormatString(
1098 __in_z LPCWSTR wzIn,
1099 __out_z_opt LPWSTR* psczOut,
1100 __out_opt DWORD* pcchOut,
1119 - __in BOOL fObfuscateHiddenVariables
1101 + __in BOOL fObfuscateHiddenVariables,
1102 + __out BOOL* pfContainsHiddenVariable
1103 )
1104 {
1105 HRESULT hr = S_OK;
@@ -1204,20 +1187,22 @@ static HRESULT FormatString(
1187 }
1188 else
1189 {
1207 - if (fObfuscateHiddenVariables)
1190 + hr = VariableIsHidden(pVariables, scz, &fHidden);
1191 + ExitOnFailure(hr, "Failed to determine variable visibility: '%ls'.", scz);
1192 +
1193 + if (pfContainsHiddenVariable)
1194 {
1209 - hr = VariableIsHidden(pVariables, scz, &fHidden);
1210 - ExitOnFailure(hr, "Failed to determine variable visibility: '%ls'.", scz);
1195 + *pfContainsHiddenVariable |= fHidden;
1196 }
1197
1213 - if (fHidden)
1198 + if (fObfuscateHiddenVariables && fHidden)
1199 {
1200 hr = StrAllocString(&rgVariables[cVariables], L"*****", 0);
1201 }
1202 else
1203 {
1204 // get formatted variable value
1220 - hr = VariableGetFormatted(pVariables, scz, &rgVariables[cVariables]);
1205 + hr = GetFormatted(pVariables, scz, &rgVariables[cVariables], pfContainsHiddenVariable);
1206 if (E_NOTFOUND == hr) // variable not found
1207 {
1208 hr = StrAllocStringSecure(&rgVariables[cVariables], L"", 0);
@@ -1327,6 +1312,57 @@ LExit:
1312 return hr;
1313 }
1314
1315 +// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree.
1316 +static HRESULT GetFormatted(
1317 + __in BURN_VARIABLES* pVariables,
1318 + __in_z LPCWSTR wzVariable,
1319 + __out_z LPWSTR* psczValue,
1320 + __out BOOL* pfContainsHiddenVariable
1321 + )
1322 +{
1323 + HRESULT hr = S_OK;
1324 + BURN_VARIABLE* pVariable = NULL;
1325 + LPWSTR scz = NULL;
1326 +
1327 + ::EnterCriticalSection(&pVariables->csAccess);
1328 +
1329 + hr = GetVariable(pVariables, wzVariable, &pVariable);
1330 + if (SUCCEEDED(hr) && BURN_VARIANT_TYPE_NONE == pVariable->Value.Type)
1331 + {
1332 + ExitFunction1(hr = E_NOTFOUND);
1333 + }
1334 + else if (E_NOTFOUND == hr)
1335 + {
1336 + ExitFunction();
1337 + }
1338 + ExitOnFailure(hr, "Failed to get variable: %ls", wzVariable);
1339 +
1340 + if (pfContainsHiddenVariable)
1341 + {
1342 + *pfContainsHiddenVariable |= pVariable->fHidden;
1343 + }
1344 +
1345 + if (BURN_VARIANT_TYPE_FORMATTED == pVariable->Value.Type)
1346 + {
1347 + hr = BVariantGetString(&pVariable->Value, &scz);
1348 + ExitOnFailure(hr, "Failed to get unformatted string.");
1349 +
1350 + hr = FormatString(pVariables, scz, psczValue, NULL, FALSE, pfContainsHiddenVariable);
1351 + ExitOnFailure(hr, "Failed to format value '%ls' of variable: %ls", pVariable->fHidden ? L"*****" : pVariable->Value.sczValue, wzVariable);
1352 + }
1353 + else
1354 + {
1355 + hr = BVariantGetString(&pVariable->Value, psczValue);
1356 + ExitOnFailure(hr, "Failed to get value as string for variable: %ls", wzVariable);
1357 + }
1358 +
1359 +LExit:
1360 + ::LeaveCriticalSection(&pVariables->csAccess);
1361 + StrSecureZeroFreeString(scz);
1362 +
1363 + return hr;
1364 +}
1365 +
1366 static HRESULT AddBuiltInVariable(
1367 __in BURN_VARIABLES* pVariables,
1368 __in LPCWSTR wzVariable,
src/engine/variable.h
+2 -1
@@ -95,7 +95,8 @@ HRESULT VariableGetVariant(
95 HRESULT VariableGetFormatted(
96 __in BURN_VARIABLES* pVariables,
97 __in_z LPCWSTR wzVariable,
98 - __out_z LPWSTR* psczValue
98 + __out_z LPWSTR* psczValue,
99 + __out BOOL* pfContainsHiddenVariable
100 );
101 HRESULT VariableSetNumeric(
102 __in BURN_VARIABLES* pVariables,
src/test/BurnUnitTest/VariableHelpers.cpp
+2 -2
@@ -98,13 +98,13 @@ namespace Bootstrapper
98 }
99 }
100
101 - String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable)
101 + String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, BOOL* pfContainsHiddenVariable)
102 {
103 HRESULT hr = S_OK;
104 LPWSTR scz = NULL;
105 try
106 {
107 - hr = VariableGetFormatted(pVariables, wzVariable, &scz);
107 + hr = VariableGetFormatted(pVariables, wzVariable, &scz, pfContainsHiddenVariable);
108 TestThrowOnFailure1(hr, L"Failed to get formatted: %s", wzVariable);
109
110 return gcnew String(scz);
src/test/BurnUnitTest/VariableHelpers.h
+1 -1
@@ -20,7 +20,7 @@ void VariableSetVersionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, LP
20 System::String^ VariableGetStringHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
21 __int64 VariableGetNumericHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
22 System::String^ VariableGetVersionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
23 -System::String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
23 +System::String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, BOOL* pfContainsHiddenVariable);
24 System::String^ VariableFormatStringHelper(BURN_VARIABLES* pVariables, LPCWSTR wzIn);
25 System::String^ VariableEscapeStringHelper(LPCWSTR wzIn);
26 bool EvaluateConditionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzCondition);
src/test/BurnUnitTest/VariableTest.cpp
+24 -9
@@ -80,6 +80,7 @@ namespace Bootstrapper
80 HRESULT hr = S_OK;
81 IXMLDOMElement* pixeBundle = NULL;
82 BURN_VARIABLES variables = { };
83 + BOOL fContainsHiddenData = FALSE;
84 try
85 {
86 LPCWSTR wzDocument =
@@ -90,6 +91,7 @@ namespace Bootstrapper
91 L" <Variable Id='Var4' Hidden='no' Persisted='no' />"
92 L" <Variable Id='Var5' Type='string' Value='' Hidden='no' Persisted='no' />"
93 L" <Variable Id='Var6' Type='formatted' Value='[Formatted]' Hidden='no' Persisted='no' />"
94 + L" <Variable Id='Formatted' Type='formatted' Value='supersecret' Hidden='yes' Persisted='no' />"
95 L"</Bundle>";
96
97 hr = VariableInitialize(&variables);
@@ -99,7 +101,7 @@ namespace Bootstrapper
101 LoadBundleXmlHelper(wzDocument, &pixeBundle);
102
103 hr = VariablesParseFromXml(&variables, pixeBundle);
102 - TestThrowOnFailure(hr, L"Failed to parse searches from XML.");
104 + TestThrowOnFailure(hr, L"Failed to parse variables from XML.");
105
106 // get and verify variable values
107 Assert::Equal((int)BURN_VARIANT_TYPE_NUMERIC, VariableGetTypeHelper(&variables, L"Var1"));
@@ -112,6 +114,12 @@ namespace Bootstrapper
114 Assert::Equal<String^>(gcnew String(L"String value."), VariableGetStringHelper(&variables, L"Var2"));
115 Assert::Equal<String^>(gcnew String(L"1.2.3.4"), VariableGetVersionHelper(&variables, L"Var3"));
116 Assert::Equal<String^>(gcnew String(L"[Formatted]"), VariableGetStringHelper(&variables, L"Var6"));
117 + Assert::Equal<String^>(gcnew String(L"supersecret"), VariableGetFormattedHelper(&variables, L"Formatted", &fContainsHiddenData));
118 + Assert::Equal<BOOL>(TRUE, fContainsHiddenData);
119 + Assert::Equal<String^>(gcnew String(L"supersecret"), VariableGetFormattedHelper(&variables, L"Var6", &fContainsHiddenData));
120 + Assert::Equal<BOOL>(TRUE, fContainsHiddenData);
121 + Assert::Equal<String^>(gcnew String(L"String value."), VariableGetFormattedHelper(&variables, L"Var2", &fContainsHiddenData));
122 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
123 }
124 finally
125 {
@@ -127,6 +135,7 @@ namespace Bootstrapper
135 BURN_VARIABLES variables = { };
136 LPWSTR scz = NULL;
137 DWORD cch = 0;
138 + BOOL fContainsHiddenData = FALSE;
139 try
140 {
141 hr = VariableInitialize(&variables);
@@ -155,12 +164,18 @@ namespace Bootstrapper
164 Assert::Equal<String^>(gcnew String(L"]"), VariableFormatStringHelper(&variables, L"[\\]]"));
165 Assert::Equal<String^>(gcnew String(L"[]"), VariableFormatStringHelper(&variables, L"[]"));
166 Assert::Equal<String^>(gcnew String(L"[NONE"), VariableFormatStringHelper(&variables, L"[NONE"));
158 - Assert::Equal<String^>(gcnew String(L"VAL2"), VariableGetFormattedHelper(&variables, L"PROP2"));
159 - Assert::Equal<String^>(gcnew String(L"3"), VariableGetFormattedHelper(&variables, L"PROP3"));
160 - Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP4"));
161 - Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP5"));
162 - Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP6"));
163 - Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP7"));
167 + Assert::Equal<String^>(gcnew String(L"VAL2"), VariableGetFormattedHelper(&variables, L"PROP2", &fContainsHiddenData));
168 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
169 + Assert::Equal<String^>(gcnew String(L"3"), VariableGetFormattedHelper(&variables, L"PROP3", &fContainsHiddenData));
170 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
171 + Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP4", &fContainsHiddenData));
172 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
173 + Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP5", &fContainsHiddenData));
174 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
175 + Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP6", &fContainsHiddenData));
176 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
177 + Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP7", &fContainsHiddenData));
178 + Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
179
180 hr = VariableFormatString(&variables, L"PRE [PROP1] POST", &scz, &cch);
181 TestThrowOnFailure(hr, L"Failed to format string");
@@ -235,8 +250,8 @@ namespace Bootstrapper
250 Assert::True(EvaluateConditionHelper(&variables, L"_PROP9"));
251 Assert::True(EvaluateConditionHelper(&variables, L"PROP16"));
252 Assert::True(EvaluateConditionHelper(&variables, L"PROP17"));
238 - Assert::True(EvaluateConditionHelper(&variables, L"PROP24"));
239 - Assert::True(EvaluateConditionHelper(&variables, L"PROP25"));
253 + Assert::True(EvaluateConditionHelper(&variables, L"PROP24=\"VAL1\""));
254 + Assert::False(EvaluateConditionHelper(&variables, L"PROP25"));
255 Assert::True(EvaluateConditionHelper(&variables, L"PROP26"));
256 Assert::True(EvaluateConditionHelper(&variables, L"PROP27"));
257