Let's Code - WiX: Rooting payloads and fixing the system in Burn

Everyone pitched in and we went from 24 issues to 7. We’re now at 8 issues because a solution for supporting VS2017 support is under development. The only problem is that I own 5 of the 8 remaining issues. So I best be getting to work. I’m skipping the oldest bug because I don’t feel like writing documentation tonight. Instead, I’m digging into two issues related to Burn: #5265 and #5307. Let’s code!

Checkout

Starting with #5265. This issue is that someone can use .. to move their bundle’s payloads out of the secure cache locations and generally confuse the heck out of Burn. It never should have been allowed so in WiX v3.11 we’ll add a warning that says, “Bad developer! No cookie for you!”

Bob reminded me he already made this an error in WiX v4.0. So maybe this issue will just be a bit of git-fu plus converting the error to a warning. Now where did Bob put that code?

I found Bob’s pull request and decided that would be a fine place to start. However, I just thought of another case his PR doesn’t handle: bind variables. So, I’ll add support for them as well in a separate commit to help bringing the additional fix back to WiX v4.0 later. Yeah, this fix is going to be as much about git-fu as it is C#.

Now looking at #5307, I drop into variables.cpp in the src\burn\engine folder and pretty quickly see the problem in InitializeVariableSystemFolder(). I did have to look up ::GetSystemWow64Directory() but once I did I just shook my head. This code never worked right.

Then I paused. What if this fix breaks backwards compatbility. Then I remind myself this code never returns the right answer. Raymond Chen just wrote about a similar problem. Although our issue isn’t a bluescreen or a crash. Hmmm… Nope, I’m still fixing it tonight.

Time to start coding.

git fetch upstream
git merge --ff --ff-only upstream/develop
git checkout -b 5265-5307-burn-fixes

Commit

First, let’s grab the Bob’s change out of WiX v4. I have a WiX v4 repo locally so rather than fetch from GitHub, I’ll fetch locally:

git remote add wix4 ..\wix4
git fetch wix4
git cherry-pick 49ec0ad9
git reset head~1

That last command puts all the changes back in my workspace so I can start editing them. For example, in WiX v4 this is case is an error (like it should have been from the very beginning) but we can’t break things now so it’ll just be a warning in WiX v3.11.

git add -A .
git commit

    > Prevent bad use of ".." in payload names in the Compiler.
    >
    > First part of fix for wixtoolset/issues#5265

Then I wanted to catch someone accidentally slipping invalid names in via bind variables (or loc variables). So I dropped a variation of the same code into a new commit so it’ll be easier to cherry pick into WiX v4.0.

git add src\tools\wix\Binder.cs
git commit

    > Prevent bad use of ".." in payload names in the Binder.
    >
    > Fixes wixtoolset/issues#5265

Now on to #5307. The logic here is just wrong. We need to add a check to see if we’re a 32-bit process running on a 64-bit operating system. If so then we do the same logic that is in the WIN64 #ifdef. If we are running on a 32-bit process then we only return a value for 32-bit system folder and we’re done.

I also noticed a comment in the issue that System64Folder isn’t documented. What’s the point in fixing all this if the documentation doesn’t tell you it exists? So I fixed the documentation as well.

It took me longer to step through all the cases in the debugger than it did to make the fix and write this text.

git add src\burn\engine\variable.cpp
git add src\chm\documents\bundle\bundle_built_in_variables.html.md
git commit

    > Correctly set and document SystemFolder and System64Folder in Burn
    >
    > The values for SystemFolder and System64Folder were all mixed up. This
    > fixes that then adds missing and fixes existing documentation about
    > these two Burn variables.
    >
    > Fixes wixtoolset/issues#5307

And we’re done. 2 bugs lighter.

Pull request

Until next time! Keep coding. You know I am.