Update string versioning.

Update condition parsing to allow specific characters for versions.
Log every time an invalid version is parsed.

9 files changed, 225 insertions, 56 deletions

diff --git a/src/engine/condition.cpp b/src/engine/condition.cpp
index 32a7a0b8..224eb0da 100644
--- a/src/engine/condition.cpp
+++ b/src/engine/condition.cpp
@@ -73,6 +73,12 @@ struct BURN_CONDITION_PARSE_CONTEXT
     BOOL fError;
 };
 
+struct BURN_CONDITION_OPERAND
+{
+    BOOL fHidden;
+    BURN_VARIANT Value;
+};
+
 
 // internal function declarations
 
@@ -92,9 +98,9 @@ static HRESULT ParseTerm(
     __in BURN_CONDITION_PARSE_CONTEXT* pContext,
     __out BOOL* pf
     );
-static HRESULT ParseValue(
+static HRESULT ParseOperand(
     __in BURN_CONDITION_PARSE_CONTEXT* pContext,
-    __out BURN_VARIANT* pValue
+    __out BURN_CONDITION_OPERAND* pOperand
     );
 static HRESULT Expect(
     __in BURN_CONDITION_PARSE_CONTEXT* pContext,
@@ -103,10 +109,10 @@ static HRESULT Expect(
 static HRESULT NextSymbol(
     __in BURN_CONDITION_PARSE_CONTEXT* pContext
     );
-static HRESULT CompareValues(
+static HRESULT CompareOperands(
     __in BURN_SYMBOL_TYPE comparison,
-    __in BURN_VARIANT leftOperand,
-    __in BURN_VARIANT rightOperand,
+    __in BURN_CONDITION_OPERAND* pLeftOperand,
+    __in BURN_CONDITION_OPERAND* pRightOperand,
     __out BOOL* pfResult
     );
 static HRESULT CompareStringValues(
@@ -342,8 +348,8 @@ static HRESULT ParseTerm(
     )
 {
     HRESULT hr = S_OK;
-    BURN_VARIANT firstValue = { };
-    BURN_VARIANT secondValue = { };
+    BURN_CONDITION_OPERAND firstOperand = { };
+    BURN_CONDITION_OPERAND secondOperand = { };
 
     if (BURN_SYMBOL_TYPE_LPAREN == pContext->NextSymbol.Type)
     {
@@ -359,8 +365,8 @@ static HRESULT ParseTerm(
         ExitFunction1(hr = S_OK);
     }
 
-    hr = ParseValue(pContext, &firstValue);
-    ExitOnFailure(hr, "Failed to parse value.");
+    hr = ParseOperand(pContext, &firstOperand);
+    ExitOnFailure(hr, "Failed to parse operand.");
 
     if (COMPARISON & pContext->NextSymbol.Type)
     {
@@ -369,24 +375,24 @@ static HRESULT ParseTerm(
         hr = NextSymbol(pContext);
         ExitOnFailure(hr, "Failed to read next symbol.");
 
-        hr = ParseValue(pContext, &secondValue);
-        ExitOnFailure(hr, "Failed to parse value.");
+        hr = ParseOperand(pContext, &secondOperand);
+        ExitOnFailure(hr, "Failed to parse operand.");
 
-        hr = CompareValues(comparison, firstValue, secondValue, pf);
-        ExitOnFailure(hr, "Failed to compare value.");
+        hr = CompareOperands(comparison, &firstOperand, &secondOperand, pf);
+        ExitOnFailure(hr, "Failed to compare operands.");
     }
     else
     {
         LONGLONG llValue = 0;
         LPWSTR sczValue = NULL;
         VERUTIL_VERSION* pVersion = NULL;
-        switch (firstValue.Type)
+        switch (firstOperand.Value.Type)
         {
         case BURN_VARIANT_TYPE_NONE:
             *pf = FALSE;
             break;
         case BURN_VARIANT_TYPE_STRING:
-            hr = BVariantGetString(&firstValue, &sczValue);
+            hr = BVariantGetString(&firstOperand.Value, &sczValue);
             if (SUCCEEDED(hr))
             {
                 *pf = sczValue && *sczValue;
@@ -394,7 +400,7 @@ static HRESULT ParseTerm(
             StrSecureZeroFreeString(sczValue);
             break;
         case BURN_VARIANT_TYPE_NUMERIC:
-            hr = BVariantGetNumeric(&firstValue, &llValue);
+            hr = BVariantGetNumeric(&firstOperand.Value, &llValue);
             if (SUCCEEDED(hr))
             {
                 *pf = 0 != llValue;
@@ -402,7 +408,7 @@ static HRESULT ParseTerm(
             SecureZeroMemory(&llValue, sizeof(llValue));
             break;
         case BURN_VARIANT_TYPE_VERSION:
-            hr = BVariantGetVersion(&firstValue, &pVersion);
+            hr = BVariantGetVersionHidden(&firstOperand.Value, firstOperand.fHidden, &pVersion);
             if (SUCCEEDED(hr))
             {
                 *pf = 0 != *pVersion->sczVersion;
@@ -415,14 +421,14 @@ static HRESULT ParseTerm(
     }
 
 LExit:
-    BVariantUninitialize(&firstValue);
-    BVariantUninitialize(&secondValue);
+    BVariantUninitialize(&firstOperand.Value);
+    BVariantUninitialize(&secondOperand.Value);
     return hr;
 }
 
-static HRESULT ParseValue(
+static HRESULT ParseOperand(
     __in BURN_CONDITION_PARSE_CONTEXT* pContext,
-    __out BURN_VARIANT* pValue
+    __out BURN_CONDITION_OPERAND* pOperand
     )
 {
     HRESULT hr = S_OK;
@@ -434,16 +440,19 @@ static HRESULT ParseValue(
         Assert(BURN_VARIANT_TYPE_STRING == pContext->NextSymbol.Value.Type);
 
         // find variable
-        hr = VariableGetVariant(pContext->pVariables, pContext->NextSymbol.Value.sczValue, pValue);
+        hr = VariableGetVariant(pContext->pVariables, pContext->NextSymbol.Value.sczValue, &pOperand->Value);
         if (E_NOTFOUND != hr)
         {
             ExitOnRootFailure(hr, "Failed to find variable.");
+
+            hr = VariableIsHidden(pContext->pVariables, pContext->NextSymbol.Value.sczValue, &pOperand->fHidden);
+            ExitOnRootFailure(hr, "Failed to get if variable is hidden.");
         }
 
-        if (BURN_VARIANT_TYPE_FORMATTED == pValue->Type)
+        if (BURN_VARIANT_TYPE_FORMATTED == pOperand->Value.Type)
         {
             // TODO: actually format the value?
-            hr = BVariantChangeType(pValue, BURN_VARIANT_TYPE_STRING);
+            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);
         }
         break;
@@ -451,8 +460,9 @@ static HRESULT ParseValue(
     case BURN_SYMBOL_TYPE_NUMBER: __fallthrough;
     case BURN_SYMBOL_TYPE_LITERAL: __fallthrough;
     case BURN_SYMBOL_TYPE_VERSION:
+        pOperand->fHidden = FALSE;
         // steal value of symbol
-        memcpy_s(pValue, sizeof(BURN_VARIANT), &pContext->NextSymbol.Value, sizeof(BURN_VARIANT));
+        memcpy_s(&pOperand->Value, sizeof(BURN_VARIANT), &pContext->NextSymbol.Value, sizeof(BURN_VARIANT));
         memset(&pContext->NextSymbol.Value, 0, sizeof(BURN_VARIANT));
         break;
 
@@ -692,8 +702,13 @@ static HRESULT NextSymbol(
                 do
                 {
                     ++n;
-                    ::GetStringTypeW(CT_CTYPE1, &pContext->wzRead[n], 1, &charType);
-                } while (L'\0' != pContext->wzRead[n] && C1_BLANK != (C1_BLANK & charType));
+                } while (pContext->wzRead[n] >= L'0' && pContext->wzRead[n] <= L'9' ||
+                         pContext->wzRead[n] >= L'A' && pContext->wzRead[n] <= L'Z' ||
+                         pContext->wzRead[n] >= L'a' && pContext->wzRead[n] <= L'z' ||
+                         pContext->wzRead[n] == L'_' ||
+                         pContext->wzRead[n] == L'+' ||
+                         pContext->wzRead[n] == L'-' ||
+                         pContext->wzRead[n] == L'.');
 
                 // Symbols don't encrypt their value, so can access the value directly.
                 hr = VerParseVersion(&pContext->wzRead[1], n - 1, FALSE, &pContext->NextSymbol.Value.pValue);
@@ -703,6 +718,10 @@ static HRESULT NextSymbol(
                     hr = E_INVALIDDATA;
                     ExitOnRootFailure(hr, "Failed to parse condition \"%ls\". Invalid version format, at position %d.", pContext->wzCondition, iPosition);
                 }
+                else if (pContext->NextSymbol.Value.pValue->fInvalid)
+                {
+                    LogId(REPORT_WARNING, MSG_CONDITION_INVALID_VERSION, pContext->wzCondition, pContext->NextSymbol.Value.pValue->sczVersion);
+                }
 
                 pContext->NextSymbol.Value.Type = BURN_VARIANT_TYPE_VERSION;
                 pContext->NextSymbol.Type = BURN_SYMBOL_TYPE_VERSION;
@@ -755,12 +774,12 @@ LExit:
 }
 
 //
-// CompareValues - compares two variant values using a given comparison.
+// CompareOperands - compares two variant values using a given comparison.
 //
-static HRESULT CompareValues(
+static HRESULT CompareOperands(
     __in BURN_SYMBOL_TYPE comparison,
-    __in BURN_VARIANT leftOperand,
-    __in BURN_VARIANT rightOperand,
+    __in BURN_CONDITION_OPERAND* pLeftOperand,
+    __in BURN_CONDITION_OPERAND* pRightOperand,
     __out BOOL* pfResult
     )
 {
@@ -771,37 +790,39 @@ static HRESULT CompareValues(
     LONGLONG llRight = 0;
     VERUTIL_VERSION* pVersionRight = 0;
     LPWSTR sczRight = NULL;
+    BURN_VARIANT* pLeftValue = &pLeftOperand->Value;
+    BURN_VARIANT* pRightValue = &pRightOperand->Value;
 
     // get values to compare based on type
-    if (BURN_VARIANT_TYPE_STRING == leftOperand.Type && BURN_VARIANT_TYPE_STRING == rightOperand.Type)
+    if (BURN_VARIANT_TYPE_STRING == pLeftValue->Type && BURN_VARIANT_TYPE_STRING == pRightValue->Type)
     {
-        hr = BVariantGetString(&leftOperand, &sczLeft);
+        hr = BVariantGetString(pLeftValue, &sczLeft);
         ExitOnFailure(hr, "Failed to get the left string");
-        hr = BVariantGetString(&rightOperand, &sczRight);
+        hr = BVariantGetString(pRightValue, &sczRight);
         ExitOnFailure(hr, "Failed to get the right string");
         hr = CompareStringValues(comparison, sczLeft, sczRight, pfResult);
     }
-    else if (BURN_VARIANT_TYPE_NUMERIC == leftOperand.Type && BURN_VARIANT_TYPE_NUMERIC == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_NUMERIC == pLeftValue->Type && BURN_VARIANT_TYPE_NUMERIC == pRightValue->Type)
     {
-        hr = BVariantGetNumeric(&leftOperand, &llLeft);
+        hr = BVariantGetNumeric(pLeftValue, &llLeft);
         ExitOnFailure(hr, "Failed to get the left numeric");
-        hr = BVariantGetNumeric(&rightOperand, &llRight);
+        hr = BVariantGetNumeric(pRightValue, &llRight);
         ExitOnFailure(hr, "Failed to get the right numeric");
         hr = CompareIntegerValues(comparison, llLeft, llRight, pfResult);
     }
-    else if (BURN_VARIANT_TYPE_VERSION == leftOperand.Type && BURN_VARIANT_TYPE_VERSION == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_VERSION == pLeftValue->Type && BURN_VARIANT_TYPE_VERSION == pRightValue->Type)
     {
-        hr = BVariantGetVersion(&leftOperand, &pVersionLeft);
+        hr = BVariantGetVersionHidden(pLeftValue, pLeftOperand->fHidden, &pVersionLeft);
         ExitOnFailure(hr, "Failed to get the left version");
-        hr = BVariantGetVersion(&rightOperand, &pVersionRight);
+        hr = BVariantGetVersionHidden(pRightValue, pRightOperand->fHidden, &pVersionRight);
         ExitOnFailure(hr, "Failed to get the right version");
         hr = CompareVersionValues(comparison, pVersionLeft, pVersionRight, pfResult);
     }
-    else if (BURN_VARIANT_TYPE_VERSION == leftOperand.Type && BURN_VARIANT_TYPE_STRING == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_VERSION == pLeftValue->Type && BURN_VARIANT_TYPE_STRING == pRightValue->Type)
     {
-        hr = BVariantGetVersion(&leftOperand, &pVersionLeft);
+        hr = BVariantGetVersionHidden(pLeftValue, pLeftOperand->fHidden, &pVersionLeft);
         ExitOnFailure(hr, "Failed to get the left version");
-        hr = BVariantGetVersion(&rightOperand, &pVersionRight);
+        hr = BVariantGetVersionHidden(pRightValue, pRightOperand->fHidden, &pVersionRight);
         if (FAILED(hr))
         {
             if (DISP_E_TYPEMISMATCH != hr)
@@ -816,11 +837,11 @@ static HRESULT CompareValues(
             hr = CompareVersionValues(comparison, pVersionLeft, pVersionRight, pfResult);
         }
     }
-    else if (BURN_VARIANT_TYPE_STRING == leftOperand.Type && BURN_VARIANT_TYPE_VERSION == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_STRING == pLeftValue->Type && BURN_VARIANT_TYPE_VERSION == pRightValue->Type)
     {
-        hr = BVariantGetVersion(&rightOperand, &pVersionRight);
+        hr = BVariantGetVersionHidden(pRightValue, pRightOperand->fHidden, &pVersionRight);
         ExitOnFailure(hr, "Failed to get the right version");
-        hr = BVariantGetVersion(&leftOperand, &pVersionLeft);
+        hr = BVariantGetVersionHidden(pLeftValue, pLeftOperand->fHidden, &pVersionLeft);
         if (FAILED(hr))
         {
             if (DISP_E_TYPEMISMATCH != hr)
@@ -835,11 +856,11 @@ static HRESULT CompareValues(
             hr = CompareVersionValues(comparison, pVersionLeft, pVersionRight, pfResult);
         }
     }
-    else if (BURN_VARIANT_TYPE_NUMERIC == leftOperand.Type && BURN_VARIANT_TYPE_STRING == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_NUMERIC == pLeftValue->Type && BURN_VARIANT_TYPE_STRING == pRightValue->Type)
     {
-        hr = BVariantGetNumeric(&leftOperand, &llLeft);
+        hr = BVariantGetNumeric(pLeftValue, &llLeft);
         ExitOnFailure(hr, "Failed to get the left numeric");
-        hr = BVariantGetNumeric(&rightOperand, &llRight);
+        hr = BVariantGetNumeric(pRightValue, &llRight);
         if (FAILED(hr))
         {
             if (DISP_E_TYPEMISMATCH != hr)
@@ -854,11 +875,11 @@ static HRESULT CompareValues(
             hr = CompareIntegerValues(comparison, llLeft, llRight, pfResult);
         }
     }
-    else if (BURN_VARIANT_TYPE_STRING == leftOperand.Type && BURN_VARIANT_TYPE_NUMERIC == rightOperand.Type)
+    else if (BURN_VARIANT_TYPE_STRING == pLeftValue->Type && BURN_VARIANT_TYPE_NUMERIC == pRightValue->Type)
     {
-        hr = BVariantGetNumeric(&rightOperand, &llRight);
+        hr = BVariantGetNumeric(pRightValue, &llRight);
         ExitOnFailure(hr, "Failed to get the right numeric");
-        hr = BVariantGetNumeric(&leftOperand, &llLeft);
+        hr = BVariantGetNumeric(pLeftValue, &llLeft);
         if (FAILED(hr))
         {
             if (DISP_E_TYPEMISMATCH != hr)
diff --git a/src/engine/engine.mc b/src/engine/engine.mc
index fb2dd6e9..c8cd6d37 100644
--- a/src/engine/engine.mc
+++ b/src/engine/engine.mc
@@ -114,6 +114,13 @@ Language=English
 Connected to elevated engine.
 .
 
+MessageId=13
+Severity=Warning
+SymbolicName=MSG_MANIFEST_INVALID_VERSION
+Language=English
+The manifest contains an invalid version string: '%1!ls!'
+.
+
 MessageId=51
 Severity=Error
 SymbolicName=MSG_FAILED_PARSE_CONDITION
@@ -156,6 +163,13 @@ Language=English
 Application canceled operation: %2!ls!, error: %1!ls!
 .
 
+MessageId=57
+Severity=Warning
+SymbolicName=MSG_CONDITION_INVALID_VERSION
+Language=English
+Condition '%1!ls!' contains invalid version string '%2!ls!'.
+.
+
 MessageId=100
 Severity=Success
 SymbolicName=MSG_DETECT_BEGIN
@@ -233,6 +247,20 @@ Language=English
 Could not calculate patch applicability for target product code: %1!ls!, context: %2!hs!, reason: 0x%3!x!
 .
 
+MessageId=122
+Severity=Warning
+SymbolicName=MSG_RELATED_PACKAGE_INVALID_VERSION
+Language=English
+Related package: '%1!ls!' has invalid version: %2!ls!
+.
+
+MessageId=123
+Severity=Warning
+SymbolicName=MSG_DETECTED_MSI_PACKAGE_INVALID_VERSION
+Language=English
+Detected msi package with invalid version, product code: '%1!ls!', version: '%2!ls!'
+.
+
 MessageId=151
 Severity=Error
 SymbolicName=MSG_FAILED_DETECT_PACKAGE
@@ -836,6 +864,20 @@ Language=English
 Variable: %1!ls!
 .
 
+MessageId=411
+Severity=Warning
+SymbolicName=MSG_VARIABLE_INVALID_VERSION
+Language=English
+The variable '%1!ls!' is being set with an invalid version string.
+.
+
+MessageId=412
+Severity=Warning
+SymbolicName=MSG_INVALID_VERSION_COERSION
+Language=English
+The string '%1!ls!' could not be coerced to a valid version.
+.
+
 MessageId=420
 Severity=Success
 SymbolicName=MSG_RESUME_AU_STARTING
diff --git a/src/engine/msiengine.cpp b/src/engine/msiengine.cpp
index e274df28..47211309 100644
--- a/src/engine/msiengine.cpp
+++ b/src/engine/msiengine.cpp
@@ -82,6 +82,11 @@ extern "C" HRESULT MsiEngineParsePackageFromXml(
     hr = VerParseVersion(scz, 0, FALSE, &pPackage->Msi.pVersion);
     ExitOnFailure(hr, "Failed to parse @Version: %ls", scz);
 
+    if (pPackage->Msi.pVersion->fInvalid)
+    {
+        LogId(REPORT_WARNING, MSG_MANIFEST_INVALID_VERSION, scz);
+    }
+
     // @UpgradeCode
     hr = XmlGetAttributeEx(pixnMsiPackage, L"UpgradeCode", &pPackage->Msi.sczUpgradeCode);
     if (E_NOTFOUND != hr)
@@ -420,6 +425,11 @@ extern "C" HRESULT MsiEngineDetectPackage(
         hr = VerParseVersion(sczInstalledVersion, 0, FALSE, &pPackage->Msi.pInstalledVersion);
         ExitOnFailure(hr, "Failed to parse installed version: '%ls' for ProductCode: %ls", sczInstalledVersion, pPackage->Msi.sczProductCode);
 
+        if (pPackage->Msi.pInstalledVersion->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_DETECTED_MSI_PACKAGE_INVALID_VERSION, pPackage->Msi.sczProductCode, sczInstalledVersion);
+        }
+
         // compare versions
         hr = VerCompareParsedVersions(pPackage->Msi.pVersion, pPackage->Msi.pInstalledVersion, &nCompareResult);
         ExitOnFailure(hr, "Failed to compare version '%ls' to installed version: '%ls'", pPackage->Msi.pVersion->sczVersion, pPackage->Msi.pInstalledVersion->sczVersion);
@@ -460,6 +470,11 @@ extern "C" HRESULT MsiEngineDetectPackage(
                 hr = VerParseVersion(sczInstalledVersion, 0, FALSE, &pVersion);
                 ExitOnFailure(hr, "Failed to parse dependency version: '%ls' for ProductCode: %ls", sczInstalledVersion, sczInstalledProductCode);
 
+                if (pVersion->fInvalid)
+                {
+                    LogId(REPORT_WARNING, MSG_DETECTED_MSI_PACKAGE_INVALID_VERSION, sczInstalledProductCode, sczInstalledVersion);
+                }
+
                 // compare versions
                 hr = VerCompareParsedVersions(pPackage->Msi.pVersion, pVersion, &nCompareResult);
                 ExitOnFailure(hr, "Failed to compare version '%ls' to dependency version: '%ls'", pPackage->Msi.pVersion->sczVersion, pVersion->sczVersion);
@@ -536,6 +551,11 @@ extern "C" HRESULT MsiEngineDetectPackage(
             hr = VerParseVersion(sczInstalledVersion, 0, FALSE, &pVersion);
             ExitOnFailure(hr, "Failed to parse related installed version: '%ls' for ProductCode: %ls", sczInstalledVersion, wzProductCode);
 
+            if (pVersion->fInvalid)
+            {
+                LogId(REPORT_WARNING, MSG_DETECTED_MSI_PACKAGE_INVALID_VERSION, wzProductCode, sczInstalledVersion);
+            }
+
             // compare versions
             if (pRelatedMsi->fMinProvided)
             {
@@ -1052,6 +1072,11 @@ extern "C" HRESULT MsiEngineAddCompatiblePackage(
 
         hr = VerParseVersion(sczInstalledVersion, 0, FALSE, &pCompatiblePackage->Msi.pVersion);
         ExitOnFailure(hr, "Failed to parse version: '%ls' for ProductCode: %ls", sczInstalledVersion, pCompatiblePackage->Msi.sczProductCode);
+
+        if (pCompatiblePackage->Msi.pVersion->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_DETECTED_MSI_PACKAGE_INVALID_VERSION, pCompatiblePackage->Msi.sczProductCode, sczInstalledVersion);
+        }
     }
 
     // For now, copy enough information to support uninstalling the newer, compatible package.
@@ -1506,6 +1531,11 @@ static HRESULT ParseRelatedMsiFromXml(
         hr = VerParseVersion(scz, 0, FALSE, &pRelatedMsi->pMinVersion);
         ExitOnFailure(hr, "Failed to parse @MinVersion: %ls", scz);
 
+        if (pRelatedMsi->pMinVersion->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_MANIFEST_INVALID_VERSION, scz);
+        }
+
         // flag that we have a min version
         pRelatedMsi->fMinProvided = TRUE;
 
@@ -1523,6 +1553,11 @@ static HRESULT ParseRelatedMsiFromXml(
         hr = VerParseVersion(scz, 0, FALSE, &pRelatedMsi->pMaxVersion);
         ExitOnFailure(hr, "Failed to parse @MaxVersion: %ls", scz);
 
+        if (pRelatedMsi->pMaxVersion->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_MANIFEST_INVALID_VERSION, scz);
+        }
+
         // flag that we have a max version
         pRelatedMsi->fMaxProvided = TRUE;
 
diff --git a/src/engine/registration.cpp b/src/engine/registration.cpp
index 3c3dc95d..4e8c810d 100644
--- a/src/engine/registration.cpp
+++ b/src/engine/registration.cpp
@@ -136,6 +136,11 @@ extern "C" HRESULT RegistrationParseFromXml(
     hr = VerParseVersion(scz, 0, FALSE, &pRegistration->pVersion);
     ExitOnFailure(hr, "Failed to parse @Version: %ls", scz);
 
+    if (pRegistration->pVersion->fInvalid)
+    {
+        LogId(REPORT_WARNING, MSG_MANIFEST_INVALID_VERSION, scz);
+    }
+
     // @ProviderKey
     hr = XmlGetAttributeEx(pixnRegistrationNode, L"ProviderKey", &pRegistration->sczProviderKey);
     ExitOnFailure(hr, "Failed to get @ProviderKey.");
diff --git a/src/engine/relatedbundle.cpp b/src/engine/relatedbundle.cpp
index e6d6516a..7b0da4a4 100644
--- a/src/engine/relatedbundle.cpp
+++ b/src/engine/relatedbundle.cpp
@@ -414,6 +414,11 @@ static HRESULT LoadRelatedBundleFromKey(
     hr = VerParseVersion(sczBundleVersion, 0, FALSE, &pRelatedBundle->pVersion);
     ExitOnFailure(hr, "Failed to parse pseudo bundle version: %ls", sczBundleVersion);
 
+    if (pRelatedBundle->pVersion->fInvalid)
+    {
+        LogId(REPORT_WARNING, MSG_RELATED_PACKAGE_INVALID_VERSION, wzRelatedBundleId, sczBundleVersion);
+    }
+
     hr = RegReadString(hkBundleId, BURN_REGISTRATION_REGISTRY_BUNDLE_CACHE_PATH, &sczCachePath);
     ExitOnFailure(hr, "Failed to read cache path from registry for bundle: %ls", wzRelatedBundleId);
 
diff --git a/src/engine/variable.cpp b/src/engine/variable.cpp
index 3069ccf7..c2192346 100644
--- a/src/engine/variable.cpp
+++ b/src/engine/variable.cpp
@@ -393,6 +393,11 @@ extern "C" HRESULT VariablesParseFromXml(
         hr = BVariantChangeType(&value, valueType);
         ExitOnFailure(hr, "Failed to change variant type.");
 
+        if (BURN_VARIANT_TYPE_VERSION == valueType && value.pValue->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_VARIABLE_INVALID_VERSION, sczId);
+        }
+
         // find existing variable
         hr = FindVariableIndexByName(pVariables, sczId, &iVariable);
         ExitOnFailure(hr, "Failed to find variable value '%ls'.", sczId);
@@ -584,7 +589,7 @@ extern "C" HRESULT VariableGetVersion(
     }
     ExitOnFailure(hr, "Failed to get value of variable: %ls", wzVariable);
 
-    hr = BVariantGetVersion(&pVariable->Value, ppValue);
+    hr = BVariantGetVersionHidden(&pVariable->Value, pVariable->fHidden, ppValue);
     ExitOnFailure(hr, "Failed to get value as version for variable: %ls", wzVariable);
 
 LExit:
@@ -1575,6 +1580,11 @@ static HRESULT SetVariableValue(
                 break;
             }
         }
+
+        if (BURN_VARIANT_TYPE_VERSION == pVariant->Type && pVariant->pValue->fInvalid)
+        {
+            LogId(REPORT_WARNING, MSG_VARIABLE_INVALID_VERSION, wzVariable);
+        }
     }
 
     // Update variable value.
diff --git a/src/engine/variant.cpp b/src/engine/variant.cpp
index 28578691..9fdb82cd 100644
--- a/src/engine/variant.cpp
+++ b/src/engine/variant.cpp
@@ -6,6 +6,12 @@
 
 // internal function declarations
 
+static HRESULT GetVersionInternal(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fHidden,
+    __in BOOL fSilent,
+    __out VERUTIL_VERSION** ppValue
+    );
 static HRESULT BVariantEncryptString(
     __in BURN_VARIANT* pVariant,
     __in BOOL fEncrypt
@@ -130,6 +136,36 @@ extern "C" HRESULT BVariantGetVersion(
     __out VERUTIL_VERSION** ppValue
     )
 {
+    return GetVersionInternal(pVariant, FALSE, FALSE, ppValue);
+}
+
+// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
+extern "C" HRESULT BVariantGetVersionHidden(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fHidden,
+    __out VERUTIL_VERSION** ppValue
+    )
+{
+    return GetVersionInternal(pVariant, fHidden, FALSE, ppValue);
+}
+
+// The contents of ppValue may be sensitive, should keep encrypted and SecureZeroMemory.
+extern "C" HRESULT BVariantGetVersionSilent(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fSilent,
+    __out VERUTIL_VERSION** ppValue
+    )
+{
+    return GetVersionInternal(pVariant, FALSE, fSilent, ppValue);
+}
+
+static HRESULT GetVersionInternal(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fHidden,
+    __in BOOL fSilent,
+    __out VERUTIL_VERSION** ppValue
+    )
+{
     HRESULT hr = S_OK;
     LONGLONG llValue = 0;
     LPWSTR sczValue = NULL;
@@ -152,6 +188,10 @@ extern "C" HRESULT BVariantGetVersion(
             {
                 hr = DISP_E_TYPEMISMATCH;
             }
+            else if (!fSilent && (*ppValue)->fInvalid)
+            {
+                LogId(REPORT_WARNING, MSG_INVALID_VERSION_COERSION, fHidden ? L"*****" : sczValue);
+            }
         }
         StrSecureZeroFreeString(sczValue);
         break;
@@ -282,7 +322,7 @@ extern "C" HRESULT BVariantSetValue(
         StrSecureZeroFreeString(sczValue);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        hr = BVariantGetVersion(pValue, &pVersionValue);
+        hr = BVariantGetVersionSilent(pValue, TRUE, &pVersionValue);
         if (SUCCEEDED(hr))
         {
             hr = BVariantSetVersion(pVariant, pVersionValue);
@@ -333,7 +373,7 @@ extern "C" HRESULT BVariantCopy(
         StrSecureZeroFreeString(sczValue);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        hr = BVariantGetVersion(pSource, &pVersionValue);
+        hr = BVariantGetVersionSilent(pSource, TRUE, &pVersionValue);
         if (SUCCEEDED(hr))
         {
             hr = BVariantSetVersion(pTarget, pVersionValue);
@@ -383,7 +423,7 @@ extern "C" HRESULT BVariantChangeType(
         hr = BVariantGetString(pVariant, &variant.sczValue);
         break;
     case BURN_VARIANT_TYPE_VERSION:
-        hr = BVariantGetVersion(pVariant, &variant.pValue);
+        hr = BVariantGetVersionSilent(pVariant, TRUE, &variant.pValue);
         break;
     default:
         ExitFunction1(hr = E_INVALIDARG);
diff --git a/src/engine/variant.h b/src/engine/variant.h
index 23303e02..34d7a187 100644
--- a/src/engine/variant.h
+++ b/src/engine/variant.h
@@ -51,6 +51,16 @@ HRESULT BVariantGetVersion(
     __in BURN_VARIANT* pVariant,
     __out VERUTIL_VERSION** ppValue
     );
+HRESULT BVariantGetVersionHidden(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fHidden,
+    __out VERUTIL_VERSION** ppValue
+    );
+HRESULT BVariantGetVersionSilent(
+    __in BURN_VARIANT* pVariant,
+    __in BOOL fSilent,
+    __out VERUTIL_VERSION** ppValue
+    );
 HRESULT BVariantSetNumeric(
     __in BURN_VARIANT* pVariant,
     __in LONGLONG llValue
diff --git a/src/test/BurnUnitTest/VariableTest.cpp b/src/test/BurnUnitTest/VariableTest.cpp
index 405c8fab..f5511199 100644
--- a/src/test/BurnUnitTest/VariableTest.cpp
+++ b/src/test/BurnUnitTest/VariableTest.cpp
@@ -273,6 +273,7 @@ namespace Bootstrapper
                 Assert::True(EvaluateConditionHelper(&variables, L"vPROP21 = 1"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP23 = v1.1.1"));
                 Assert::True(EvaluateConditionHelper(&variables, L"v1.1.1 = PROP23"));
+                Assert::False(EvaluateConditionHelper(&variables, L"v1.1.1<>PROP23"));
                 Assert::True(EvaluateConditionHelper(&variables, L"PROP1 <> v1.1.1"));
                 Assert::True(EvaluateConditionHelper(&variables, L"v1.1.1 <> PROP1"));