[wix-devs] #3640 - Burn will use incorrect payload if local

[Rob Mensching]

These are great problems to get solved in Burn.


> During Detect, count a package as cached if any of a package's payloads exist without checking the file size. This makes Detect faster.

1. Just curious: Is it noticeably faster? I thought checking file size would be a simple NTFS directory table lookup. It must be a thing when there are many, many, many loose payloads, right?


> then the engine will now automatically try to download it if it has a DownloadUrl

2. FYI, this was a (probably undocumented) requirement that the BA was always called before the engine would go to the internet. That requirement came from the early design days of Burn when internet downloads were more costly/less prolific so it's probably okay to toss it. I just wanted to raise it in case it informed/changed anything.


> It is not allowed in the new OnCacheAcquireResolving event, but only because we don't have a good way for the BA to send strings to the engine to interact with a BA event.

3. Most (all?) BAs prompt the user for source during OnResolveSource. If I understand correctly, OnResolveSource is essentially replaced by OnCacheAcquireResolving when args.fFoundLocal == FALSE. If so, that's the most likely case where set source would be called (after the user was prompted for the new source) and thus I think we should make that possible and as easy as possible. Unless the correct solution is to use OnCacheAcquireComplete, if so, see the next point.


> If the web server requires authentication, the BA could set that for every payload in OnCacheAcquireBegin

4. Similar to the above issue, the BA is likely to prompt the user for credentials on demand (in this case after fail to get a file due to authentication required). If the solution is to handle failure in OnCacheAcquireComplete by setting the source credentials then retrying, I expect that's cool.


-----Original Message-----
From: wix-devs <wix-devs-bounces at lists.wixtoolset.org> On Behalf Of Sean Hall via wix-devs
Sent: Tuesday, April 6, 2021 12:59 PM
To: WiX Toolset Developer Mailing List <wix-devs at lists.wixtoolset.org>
Cc: Sean Hall <r.sean.hall at gmail.com>
Subject: [wix-devs] #3640 - Burn will use incorrect payload if local

I have implemented a solution for this at https://github.com/wixtoolset/burn/pull/48. There's still some work left to get the progress right. This pull request is fixing some design flaws:

During Detect, a payload was assumed as cached if the file existed and had the right size. In v3 and v4, this could have been a false positive if it had the wrong hash. In v3, this could have been a false negative if the payload was supposed to be verified through Authenticode. This partial verification was done because full verification could make Detect take too long.

When trying to acquire a payload and the engine found a suitable file in one of its probe paths, the BA never got a chance to change the source even if the payload verification failed.

The engine made it impossible for the BA to request that an embedded be downloaded instead of acquired through the container (#5253).


The fixes for these flaws are:

During Detect, count a package as cached if any of a package's payloads exist without checking the file size. This makes Detect faster.

Cache planning is done at the package level instead of the payload level, which ensures that all payloads for packages being cached go through full verification. This also allows the BA to request that an embedded payload is downloaded instead of extracted from a container.

The BA can set the source during the OnCacheAcquireBegin/Complete events, which are always sent to the BA if the payload wasn't already present and verified.


The API has changed between the engine and the BA, full details can be seen in the PR. The OnResolveSource event has been removed. This event was basically doing two things: allowing the BA to request that the BA be downloaded and allowing the BA to set the source. A new OnCacheAcquireResolving event has been added.

For the request to download, this should be needed rarely after this pull request. If the payload can't be found locally and isn't embedded in a container, then the engine will now automatically try to download it if it has a DownloadUrl. That being said, the BA can request to download from OnCacheAcquireBegin or the new OnCacheAcquireResolving event.

For setting the source, this is now allowed in OnCacheAcquireBegin and OnCacheAcquireComplete. It is not allowed in the new OnCacheAcquireResolving event, but only because we don't have a good way for the BA to send strings to the engine to interact with a BA event.

Some examples:

1. Uncompressed bundle, web download

In this scenario, the user downloads the bundle .exe by itself and everything is downloaded as needed. All payloads have a DownloadUrl. In v3, the BA would have to request Download in OnResolveSource for every payload.
With this PR, no BA code is required because the engine will automatically try to use the DownloadUrl. If the web server requires authentication, the BA could set that for every payload in OnCacheAcquireBegin or like v3 could just use the BA manifest to go through all the payloads and set them before Apply.

2. Uncompressed bundle, DVD layout

In this scenario, the user runs the bundle .exe from a DVD that has all the payloads next to it. There should be no change in behavior here, the engine should find all payloads next to the bundle with no BA code required.

3. Fully compressed bundle

In this scenario, the bundle was installed. The user modifies the installation and installs a package not already present. In v3, the only way to get the required payload would be to provide the original bundle.
With this PR, the BA could set the DownloadUrl during OnCacheAcquireBegin and request Download to avoid the engine trying to locate the original bundle. There would be a corresponding change in Core that allows the authoring of a DownloadUrl for embedded payloads so that the BA would only have to request download during OnCacheAcquireBegin or OnCacheAcquireResolving.

An aside:

The .NET Core team solves the string problem by providing callbacks. For example, their new API that they added to list the runtimes and SDKs on the
machine: https://github.com/dotnet/runtime/issues/46128.

For these events where we want to allow changing the source, we could pass in additional parameters that are callback functions. For example:

struct BA_ONCACHEACQUIRERESOLVING_ARGS
{
    PFN_BOOTSTRAPPER_ENGINE_SETLOCALSOURCE pfnSetLocalSource;
    LPVOID pvContext;
}

extern "C" typedef HRESULT(WINAPI *PFN_BOOTSTRAPPER_ENGINE_SETLOCALSOURCE)(
    __in LPCWSTR wzPath,
    __in LPVOID pvContext
    );

To keep the binary compatibility goal, it would probably have to instead use the ARGS and RESULTS struct paradigm but this is easier to write.
____________________________________________________________________
WiX Toolset Developer Mailing List provided by FireGiant http://www.firegiant.com/