Failed to get MSI property in UPGRADINGPRODUCTCODE, WIX_UPGRADE_DETECTED
Asked Answered
I

2

1

I wanted to skip some of my custom actions during upgrade, uninstallDriver, to achieve this I have tried to retrieve property WIX_UPGRADE_DETECTED and UPGRADINGPRODUCTCODE, but both of them are not set.

Sample code:

UninstallDriver(MSIHANDLE hInstall)  
{
   char szBuff[1024]; DWORD dwValue = 0;

   OutputDebugStringA("UninstallDriver");
   MsiGetPropertyA(hInstall, "UPGRADINGPRODUCTCODE", szBuff, &dwValue);
   OutputDebugStringA("UPGRADINGPRODUCTCODE");OutputDebugStringA(szBuff);

   MsiGetPropertyA(hInstall, "WIX_UPGRADE_DETECTED", szBuff, &dwValue);
   OutputDebugStringA("WIX_UPGRADE_DETECTED");OutputDebugStringA(szBuff);

   result = UninstallDriver();

   return result;
}

And my customAction is,

<Custom Action='UninstallMYDriverAction'
 After='InstallInitialize'>
        REMOVE~="ALL" OR REINSTALL</Custom>
Inconvertible answered 26/12, 2018 at 9:16 Comment(1)
One question: are you live with the first version? If so, then you need to patch the released version in order to prevent it from running its embedded uninstall sequence. This is a different matter again, and involves using a minor upgrade to patch the installed product (which does not uninstall the existing product but upgrades it in place) instead of a major upgrade (which is an uninstall of the old version and a reinstall of the new version). Somwhat similar issue.Wunderlich
O
2

Ignoring the debug strings, it's easier to see that the buffer handling is incorrect. I would suggest also outputting the return values from MsiGetPropertyA() and the value in dwValue to confirm, but here's what I think is happening (comments refer to dwValue):

char szBuff[1024]; DWORD dwValue = 0;
MsiGetPropertyA(hInstall, "UPGRADINGPRODUCTCODE", szBuff, &dwValue); // passes 0, updated to ?x?
MsiGetPropertyA(hInstall, "WIX_UPGRADE_DETECTED", szBuff, &dwValue); // passes ?x?, updated to ?y?

When requesting UPGRADINGPRODUCTCODE property with a claimed buffer length of zero, the fetch will never succeed as it must always accept at least a null character. Thus this will return ERROR_MORE_DATA and set dwValue to the length excluding a null character (?x?).

Then it will request the value of WIX_UPGRADE_DETECTED with a claimed buffer length of (?x?). If the new length (?y?) is less than the old length (?x?) you'll get its contents in your buffer; otherwise it will also just query the length of this new property.

Since WIX_UPGRADE_DETECTED contains a list of one or more GUIDs, and UPGRADINGPRODUCTCODE only contains one, and this code never increments dwValue to account for the null, it will only possibly succeed if the latter is ?y? is 0 (empty) and ?x? is non-empty. But note that this second call passed an unverified value as the length of your buffer, this pattern is a buffer overflow waiting to happen.

So fix your buffer handling. The pattern I like to use (below) is similar to what Stein describes, but avoids the second call if I know a good default size for the buffer. In your case, it sounds like you're happy with the 1024 element buffer, but do consider if you ever need to handle more than 1024 / len(GUID) related upgrade codes.

(My guess is you're fine. But at least think it through. And even though GUIDs are ASCII so the contents won't matter, please please please build UNICODE these days...)

WCHAR szBuf[1024];
DWORD cchBuf = 1024; // or _countof(szBuf);
DWORD dwErr = MsiGetPropertyW(hInstall, L"UPGRADINGPRODUCTCODE", szBuf, &cchBuf);
if (dwErr != ERROR_MORE_DATA) {
    // exercise: increment cchBuf for null, adjust buffer, call MsiGetPropertyW again
}
if (dwErr != ERROR_SUCCESS) {
    // per https://learn.microsoft.com/en-us/windows/desktop/msi/custom-action-return-values
    return ERROR_INSTALL_FAILURE;
}

// reset buffer length for next call, in case second property is longer than first
cchBuf = 1024;
dwErr = MsiGetPropertyW(hInstall, L"WIX_UPGRADE_DETECTED", szBuf, &cchBuf);
// : : :
Organography answered 29/12, 2018 at 4:31 Comment(2)
Great explanation. I am just wondering why one should call the Unicode version of the function directly? Shouldn't you rely on the MsiGetProperty call and let the compiler and linker do the rest? For example if there suddenly is yet another function for a new version of Unicode? (ze trutz izt out zhere!). C++ is an astonishing landscape of obscure "plumbing" and boilerplate code, isn't it? I also can't tell if I should use L"Text here" or ATL macros instead. L"" is part of the language itself? Very confusing. Anyway, another issue. Thanks for the detailed explanation on string buffers.Wunderlich
@SteinÅsmul There's gotta be questions dedicated to that. Speaking for myself, I usually just specify Unicode in the VS project settings and use TCHARs and the un-suffixed function names in my code. Here I used explicit WCHAR and ...W functions primarily to contrast with the original code. There's no new transition on the horizon, so I'd say explicit is valid but unusual. The L string literal prefix is part of the language. In string literals, L"" and TEXT("") are fine while CA2CW("") or similar is wasted work; see e.g. https://mcmap.net/q/104507/-what-exactly-is-the-l-prefix-in-c.Organography
S
1

Custom Action Condition: You should be able to condition that custom action outright without going via a custom action? MSI conditions are notorious for being difficult to get just right since there are so many installation modes to deal with: install, repair, self-repair, modify and minor upgrade patching, uninstall, major upgrade initiated uninstalls, etc... There are several more.

In your case it might be sufficient to add to the existing condition:

AND (NOT UPGRADINGPRODUCTCODE)

This will make the condition invalid if an upgrade is detected.

I would actually try: (REMOVE~="ALL") AND (NOT UPGRADINGPRODUCTCODE) (as seen in this answer).


Top Tip: You can debug MSI conditions pretty efficiently by the approach described here: How to execute conditional custom action on install and modify only? (see bottom section). You use VBScript and a few MSI API calls to determine what conditions are true, and then you can change the sequencing to check the conditions at different times. They might change.

Note: This is just a sorry-for-itself and poor man's MSI debugger, but it works.


Property String Size: I am not a C++ expert, but here is from the MSI SDK: You need to call MsiGetProperty with an empty string first and then add 1 for null termination. As usual high "line noise" with C++. A lot of plumbing and formalities for very little effect - but let's just deal with it :-):

The below C++ sample is from the MSI SDK:

UINT __stdcall MyCustomAction(MSIHANDLE hInstall)
{
    TCHAR* szValueBuf = NULL;
    DWORD cchValueBuf = 0;
    UINT uiStat =  MsiGetProperty(hInstall, TEXT("MyProperty"), TEXT(""), &cchValueBuf);
    //cchValueBuf now contains the size of the property's string, without null termination
    if (ERROR_MORE_DATA == uiStat)
    {
        ++cchValueBuf; // add 1 for null termination
        szValueBuf = new TCHAR[cchValueBuf];
        if (szValueBuf)
        {
            uiStat = MsiGetProperty(hInstall, TEXT("MyProperty"), szValueBuf, &cchValueBuf);
        }
    }
    if (ERROR_SUCCESS != uiStat)
    {
        if (szValueBuf != NULL) 
           delete[] szValueBuf;
        return ERROR_INSTALL_FAILURE;
    }   

// custom action uses MyProperty
// ...

delete[] szValueBuf;

return ERROR_SUCCESS;

}

VBScript: For testing purposes VBScript can be used easily (so you can determine if you have a coding issue or not):

MsgBox Session.Property("PROPERTYNAME")

Some Further Links (just for reference and easy retrieval, not needed for you I think):

Shipowner answered 26/12, 2018 at 20:55 Comment(6)
Thanks Stein, I checked coding issue, no coding issue. One more query currently My both Installers, are of version2..0.0.0 but different builds, BUILD1, BUILD2, in that case too, UPGRADINGPRODUCTCODE is SET? Or only in case one of version numbers (2.1.0.0) changedInconvertible
If I dont want to run my customAction, any kind of upgrade (1.0.0.0 to 1.0.0.0/1.0.0.0-1.1.0.0/1.0.0.0.-2.0.0.0 etc.) then how can I put condition.Inconvertible
Basically I wanted to execute my CA only in Uninstall /repair caseInconvertible
UPGRADINGPRODUCTCODE is only set in a major upgrade in the older setup that is uninstalling. If there is no major upgrade happening, it is not set. I don't have the time to test the exact condition for this right now, but see the links provided above for lots of information. Try the VBScript approach to validate the conditions that I mention in the answer above (top tip section).Wunderlich
UPGRADINGPRODUCTCODE only when major upgrade means? only 1st version is changed? 1.0.0.0 to 2.0.0.0 not for others 1.0.0.0 to 1.1.0.0? This may novice/silly question but forgive me.Inconvertible
Any change in the product version accompanied by a change in the product code and package code constitute a major upgrade. See the table at the bottom here.Wunderlich

© 2022 - 2024 — McMap. All rights reserved.