WIXFEAT:6258 - Format variables when evaluating condition.

6 files changed, 112 insertions, 55 deletions

diff --git a/src/engine/condition.cpp b/src/engine/condition.cpp
index 224eb0da..d6038b34 100644
--- a/src/engine/condition.cpp
+++ b/src/engine/condition.cpp
@@ -432,6 +432,7 @@ static HRESULT ParseOperand(
     )
 {
     HRESULT hr = S_OK;
+    LPWSTR sczFormatted = NULL;
 
     // Symbols don't encrypt their value, so can access the value directly.
     switch (pContext->NextSymbol.Type)
@@ -451,9 +452,11 @@ static HRESULT ParseOperand(
 
         if (BURN_VARIANT_TYPE_FORMATTED == pOperand->Value.Type)
         {
-            // TODO: actually format the value?
-            hr = BVariantChangeType(&pOperand->Value, BURN_VARIANT_TYPE_STRING);
-            ExitOnRootFailure(hr, "Failed to change variable '%ls' type for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
+            hr = VariableGetFormatted(pContext->pVariables, pContext->NextSymbol.Value.sczValue, &sczFormatted, &pOperand->fHidden);
+            ExitOnRootFailure(hr, "Failed to format variable '%ls' for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
+
+            hr = BVariantSetString(&pOperand->Value, sczFormatted, 0, FALSE);
+            ExitOnRootFailure(hr, "Failed to store formatted value for variable '%ls' for condition '%ls'", pContext->NextSymbol.Value.sczValue, pContext->wzCondition);
         }
         break;
 
@@ -477,6 +480,8 @@ static HRESULT ParseOperand(
     ExitOnFailure(hr, "Failed to read next symbol.");
 
 LExit:
+    StrSecureZeroFreeString(sczFormatted);
+
     return hr;
 }
 
diff --git a/src/engine/variable.cpp b/src/engine/variable.cpp
index 3425b92c..8b6b318b 100644
--- a/src/engine/variable.cpp
+++ b/src/engine/variable.cpp
@@ -54,7 +54,14 @@ static HRESULT FormatString(
     __in_z LPCWSTR wzIn,
     __out_z_opt LPWSTR* psczOut,
     __out_opt DWORD* pcchOut,
-    __in BOOL fObfuscateHiddenVariables
+    __in BOOL fObfuscateHiddenVariables,
+    __out BOOL* pfContainsHiddenVariable
+    );
+static HRESULT GetFormatted(
+    __in BURN_VARIABLES* pVariables,
+    __in_z LPCWSTR wzVariable,
+    __out_z LPWSTR* psczValue,
+    __out BOOL* pfContainsHiddenVariable
     );
 static HRESULT AddBuiltInVariable(
     __in BURN_VARIABLES* pVariables,
@@ -629,43 +636,18 @@ LExit:
 extern "C" HRESULT VariableGetFormatted(
     __in BURN_VARIABLES* pVariables,
     __in_z LPCWSTR wzVariable,
-    __out_z LPWSTR* psczValue
+    __out_z LPWSTR* psczValue,
+    __out BOOL* pfContainsHiddenVariable
     )
 {
     HRESULT hr = S_OK;
-    BURN_VARIABLE* pVariable = NULL;
-    LPWSTR scz = NULL;
-
-    ::EnterCriticalSection(&pVariables->csAccess);
 
-    hr = GetVariable(pVariables, wzVariable, &pVariable);
-    if (SUCCEEDED(hr) && BURN_VARIANT_TYPE_NONE == pVariable->Value.Type)
+    if (pfContainsHiddenVariable)
     {
-        ExitFunction1(hr = E_NOTFOUND);
-    }
-    else if (E_NOTFOUND == hr)
-    {
-        ExitFunction();
+        *pfContainsHiddenVariable = FALSE;
     }
-    ExitOnFailure(hr, "Failed to get variable: %ls", wzVariable);
 
-    if (BURN_VARIANT_TYPE_FORMATTED == pVariable->Value.Type)
-    {
-        hr = BVariantGetString(&pVariable->Value, &scz);
-        ExitOnFailure(hr, "Failed to get unformatted string.");
-
-        hr = VariableFormatString(pVariables, scz, psczValue, NULL);
-        ExitOnFailure(hr, "Failed to format value '%ls' of variable: %ls", pVariable->fHidden ? L"*****" : pVariable->Value.sczValue, wzVariable);
-    }
-    else
-    {
-        hr = BVariantGetString(&pVariable->Value, psczValue);
-        ExitOnFailure(hr, "Failed to get value as string for variable: %ls", wzVariable);
-    }
-
-LExit:
-    ::LeaveCriticalSection(&pVariables->csAccess);
-    StrSecureZeroFreeString(scz);
+    hr = GetFormatted(pVariables, wzVariable, psczValue, pfContainsHiddenVariable);
 
     return hr;
 }
@@ -736,7 +718,7 @@ extern "C" HRESULT VariableFormatString(
     __out_opt DWORD* pcchOut
     )
 {
-    return FormatString(pVariables, wzIn, psczOut, pcchOut, FALSE);
+    return FormatString(pVariables, wzIn, psczOut, pcchOut, FALSE, NULL);
 }
 
 extern "C" HRESULT VariableFormatStringObfuscated(
@@ -746,7 +728,7 @@ extern "C" HRESULT VariableFormatStringObfuscated(
     __out_opt DWORD* pcchOut
     )
 {
-    return FormatString(pVariables, wzIn, psczOut, pcchOut, TRUE);
+    return FormatString(pVariables, wzIn, psczOut, pcchOut, TRUE, NULL);
 }
 
 extern "C" HRESULT VariableEscapeString(
@@ -1116,7 +1098,8 @@ static HRESULT FormatString(
     __in_z LPCWSTR wzIn,
     __out_z_opt LPWSTR* psczOut,
     __out_opt DWORD* pcchOut,
-    __in BOOL fObfuscateHiddenVariables
+    __in BOOL fObfuscateHiddenVariables,
+    __out BOOL* pfContainsHiddenVariable
     )
 {
     HRESULT hr = S_OK;
@@ -1204,20 +1187,22 @@ static HRESULT FormatString(
             }
             else
             {
-                if (fObfuscateHiddenVariables)
+                hr = VariableIsHidden(pVariables, scz, &fHidden);
+                ExitOnFailure(hr, "Failed to determine variable visibility: '%ls'.", scz);
+
+                if (pfContainsHiddenVariable)
                 {
-                    hr = VariableIsHidden(pVariables, scz, &fHidden);
-                    ExitOnFailure(hr, "Failed to determine variable visibility: '%ls'.", scz);
+                    *pfContainsHiddenVariable |= fHidden;
                 }
 
-                if (fHidden)
+                if (fObfuscateHiddenVariables && fHidden)
                 {
                     hr = StrAllocString(&rgVariables[cVariables], L"*****", 0);
                 }
                 else
                 {
                     // get formatted variable value
-                    hr = VariableGetFormatted(pVariables, scz, &rgVariables[cVariables]);
+                    hr = GetFormatted(pVariables, scz, &rgVariables[cVariables], pfContainsHiddenVariable);
                     if (E_NOTFOUND == hr) // variable not found
                     {
                         hr = StrAllocStringSecure(&rgVariables[cVariables], L"", 0);
@@ -1327,6 +1312,57 @@ LExit:
     return hr;
 }
 
+// The contents of psczOut may be sensitive, should keep encrypted and SecureZeroFree.
+static HRESULT GetFormatted(
+    __in BURN_VARIABLES* pVariables,
+    __in_z LPCWSTR wzVariable,
+    __out_z LPWSTR* psczValue,
+    __out BOOL* pfContainsHiddenVariable
+    )
+{
+    HRESULT hr = S_OK;
+    BURN_VARIABLE* pVariable = NULL;
+    LPWSTR scz = NULL;
+
+    ::EnterCriticalSection(&pVariables->csAccess);
+
+    hr = GetVariable(pVariables, wzVariable, &pVariable);
+    if (SUCCEEDED(hr) && BURN_VARIANT_TYPE_NONE == pVariable->Value.Type)
+    {
+        ExitFunction1(hr = E_NOTFOUND);
+    }
+    else if (E_NOTFOUND == hr)
+    {
+        ExitFunction();
+    }
+    ExitOnFailure(hr, "Failed to get variable: %ls", wzVariable);
+
+    if (pfContainsHiddenVariable)
+    {
+        *pfContainsHiddenVariable |= pVariable->fHidden;
+    }
+
+    if (BURN_VARIANT_TYPE_FORMATTED == pVariable->Value.Type)
+    {
+        hr = BVariantGetString(&pVariable->Value, &scz);
+        ExitOnFailure(hr, "Failed to get unformatted string.");
+
+        hr = FormatString(pVariables, scz, psczValue, NULL, FALSE, pfContainsHiddenVariable);
+        ExitOnFailure(hr, "Failed to format value '%ls' of variable: %ls", pVariable->fHidden ? L"*****" : pVariable->Value.sczValue, wzVariable);
+    }
+    else
+    {
+        hr = BVariantGetString(&pVariable->Value, psczValue);
+        ExitOnFailure(hr, "Failed to get value as string for variable: %ls", wzVariable);
+    }
+
+LExit:
+    ::LeaveCriticalSection(&pVariables->csAccess);
+    StrSecureZeroFreeString(scz);
+
+    return hr;
+}
+
 static HRESULT AddBuiltInVariable(
     __in BURN_VARIABLES* pVariables,
     __in LPCWSTR wzVariable,
diff --git a/src/engine/variable.h b/src/engine/variable.h
index 6437c32f..713fe6e3 100644
--- a/src/engine/variable.h
+++ b/src/engine/variable.h
@@ -95,7 +95,8 @@ HRESULT VariableGetVariant(
 HRESULT VariableGetFormatted(
     __in BURN_VARIABLES* pVariables,
     __in_z LPCWSTR wzVariable,
-    __out_z LPWSTR* psczValue
+    __out_z LPWSTR* psczValue,
+    __out BOOL* pfContainsHiddenVariable
     );
 HRESULT VariableSetNumeric(
     __in BURN_VARIABLES* pVariables,
diff --git a/src/test/BurnUnitTest/VariableHelpers.cpp b/src/test/BurnUnitTest/VariableHelpers.cpp
index 99ba492a..40f958f8 100644
--- a/src/test/BurnUnitTest/VariableHelpers.cpp
+++ b/src/test/BurnUnitTest/VariableHelpers.cpp
@@ -98,13 +98,13 @@ namespace Bootstrapper
         }
     }
 
-    String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable)
+    String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, BOOL* pfContainsHiddenVariable)
     {
         HRESULT hr = S_OK;
         LPWSTR scz = NULL;
         try
         {
-            hr = VariableGetFormatted(pVariables, wzVariable, &scz);
+            hr = VariableGetFormatted(pVariables, wzVariable, &scz, pfContainsHiddenVariable);
             TestThrowOnFailure1(hr, L"Failed to get formatted: %s", wzVariable);
 
             return gcnew String(scz);
diff --git a/src/test/BurnUnitTest/VariableHelpers.h b/src/test/BurnUnitTest/VariableHelpers.h
index 96122219..d460c60f 100644
--- a/src/test/BurnUnitTest/VariableHelpers.h
+++ b/src/test/BurnUnitTest/VariableHelpers.h
@@ -20,7 +20,7 @@ void VariableSetVersionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, LP
 System::String^ VariableGetStringHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
 __int64 VariableGetNumericHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
 System::String^ VariableGetVersionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
-System::String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable);
+System::String^ VariableGetFormattedHelper(BURN_VARIABLES* pVariables, LPCWSTR wzVariable, BOOL* pfContainsHiddenVariable);
 System::String^ VariableFormatStringHelper(BURN_VARIABLES* pVariables, LPCWSTR wzIn);
 System::String^ VariableEscapeStringHelper(LPCWSTR wzIn);
 bool EvaluateConditionHelper(BURN_VARIABLES* pVariables, LPCWSTR wzCondition);
diff --git a/src/test/BurnUnitTest/VariableTest.cpp b/src/test/BurnUnitTest/VariableTest.cpp
index f5511199..676c134e 100644
--- a/src/test/BurnUnitTest/VariableTest.cpp
+++ b/src/test/BurnUnitTest/VariableTest.cpp
@@ -80,6 +80,7 @@ namespace Bootstrapper
             HRESULT hr = S_OK;
             IXMLDOMElement* pixeBundle = NULL;
             BURN_VARIABLES variables = { };
+            BOOL fContainsHiddenData = FALSE;
             try
             {
                 LPCWSTR wzDocument =
@@ -90,6 +91,7 @@ namespace Bootstrapper
                     L"    <Variable Id='Var4' Hidden='no' Persisted='no' />"
                     L"    <Variable Id='Var5' Type='string' Value='' Hidden='no' Persisted='no' />"
                     L"    <Variable Id='Var6' Type='formatted' Value='[Formatted]' Hidden='no' Persisted='no' />"
+                    L"    <Variable Id='Formatted' Type='formatted' Value='supersecret' Hidden='yes' Persisted='no' />"
                     L"</Bundle>";
 
                 hr = VariableInitialize(&variables);
@@ -99,7 +101,7 @@ namespace Bootstrapper
                 LoadBundleXmlHelper(wzDocument, &pixeBundle);
 
                 hr = VariablesParseFromXml(&variables, pixeBundle);
-                TestThrowOnFailure(hr, L"Failed to parse searches from XML.");
+                TestThrowOnFailure(hr, L"Failed to parse variables from XML.");
 
                 // get and verify variable values
                 Assert::Equal((int)BURN_VARIANT_TYPE_NUMERIC, VariableGetTypeHelper(&variables, L"Var1"));
@@ -112,6 +114,12 @@ namespace Bootstrapper
                 Assert::Equal<String^>(gcnew String(L"String value."), VariableGetStringHelper(&variables, L"Var2"));
                 Assert::Equal<String^>(gcnew String(L"1.2.3.4"), VariableGetVersionHelper(&variables, L"Var3"));
                 Assert::Equal<String^>(gcnew String(L"[Formatted]"), VariableGetStringHelper(&variables, L"Var6"));
+                Assert::Equal<String^>(gcnew String(L"supersecret"), VariableGetFormattedHelper(&variables, L"Formatted", &fContainsHiddenData));
+                Assert::Equal<BOOL>(TRUE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"supersecret"), VariableGetFormattedHelper(&variables, L"Var6", &fContainsHiddenData));
+                Assert::Equal<BOOL>(TRUE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"String value."), VariableGetFormattedHelper(&variables, L"Var2", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
             }
             finally
             {
@@ -127,6 +135,7 @@ namespace Bootstrapper
             BURN_VARIABLES variables = { };
             LPWSTR scz = NULL;
             DWORD cch = 0;
+            BOOL fContainsHiddenData = FALSE;
             try
             {
                 hr = VariableInitialize(&variables);
@@ -155,12 +164,18 @@ namespace Bootstrapper
                 Assert::Equal<String^>(gcnew String(L"]"), VariableFormatStringHelper(&variables, L"[\\]]"));
                 Assert::Equal<String^>(gcnew String(L"[]"), VariableFormatStringHelper(&variables, L"[]"));
                 Assert::Equal<String^>(gcnew String(L"[NONE"), VariableFormatStringHelper(&variables, L"[NONE"));
-                Assert::Equal<String^>(gcnew String(L"VAL2"), VariableGetFormattedHelper(&variables, L"PROP2"));
-                Assert::Equal<String^>(gcnew String(L"3"), VariableGetFormattedHelper(&variables, L"PROP3"));
-                Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP4"));
-                Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP5"));
-                Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP6"));
-                Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP7"));
+                Assert::Equal<String^>(gcnew String(L"VAL2"), VariableGetFormattedHelper(&variables, L"PROP2", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"3"), VariableGetFormattedHelper(&variables, L"PROP3", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP4", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP5", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"[PROP1]"), VariableGetFormattedHelper(&variables, L"PROP6", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
+                Assert::Equal<String^>(gcnew String(L"[PROP2]"), VariableGetFormattedHelper(&variables, L"PROP7", &fContainsHiddenData));
+                Assert::Equal<BOOL>(FALSE, fContainsHiddenData);
 
                 hr = VariableFormatString(&variables, L"PRE [PROP1] POST", &scz, &cch);
                 TestThrowOnFailure(hr, L"Failed to format string");
@@ -235,8 +250,8 @@ namespace Bootstrapper
                 Assert::True(EvaluateConditionHelper(&variables, L"_PROP9"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP16"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP17"));
-                Assert::True(EvaluateConditionHelper(&variables, L"PROP24"));
-                Assert::True(EvaluateConditionHelper(&variables, L"PROP25"));
+                Assert::True(EvaluateConditionHelper(&variables, L"PROP24=\"VAL1\""));
+                Assert::False(EvaluateConditionHelper(&variables, L"PROP25"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP26"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP27"));