From d9257a7e8e42f13fe6b5684a8b034429c6a09b3c Mon Sep 17 00:00:00 2001 From: Bevan Weiss Date: Fri, 2 Aug 2024 21:45:46 +1000 Subject: [PATCH 1/2] A basic install / uninstall test for RemoveFoldersEx Contains no files etc to verify that Remove action can still occur without other elements bringing in the RemoveFiles Standard Action Signed-off-by: Bevan Weiss --- src/ext/Util/ca/RemoveFoldersEx.cpp | 2 +- .../RemoveFolderExTest/Package.wxs | 15 ++++ .../RemoveFolderExTest.wixproj | 13 +++ .../RemoveFolderExTests.cs | 82 +++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/Package.wxs create mode 100644 src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/RemoveFolderExTest.wixproj create mode 100644 src/test/msi/WixToolsetTest.MsiE2E/RemoveFolderExTests.cs diff --git a/src/ext/Util/ca/RemoveFoldersEx.cpp b/src/ext/Util/ca/RemoveFoldersEx.cpp index 9523dda7d..f162e929b 100644 --- a/src/ext/Util/ca/RemoveFoldersEx.cpp +++ b/src/ext/Util/ca/RemoveFoldersEx.cpp @@ -232,7 +232,7 @@ extern "C" UINT WINAPI WixRemoveFoldersEx( { hr = S_OK; } - ExitOnFailure(hr, "Failure occured while processing Wix4RemoveFolderEx table"); + ExitOnFailure(hr, "Failure occurred while processing Wix4RemoveFolderEx table"); LExit: #ifndef _WIN64 diff --git a/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/Package.wxs b/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/Package.wxs new file mode 100644 index 000000000..3f8cf3691 --- /dev/null +++ b/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/Package.wxs @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + diff --git a/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/RemoveFolderExTest.wixproj b/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/RemoveFolderExTest.wixproj new file mode 100644 index 000000000..1c71388ae --- /dev/null +++ b/src/test/msi/TestData/RemoveFolderExTests/RemoveFolderExTest/RemoveFolderExTest.wixproj @@ -0,0 +1,13 @@ + + + + {A75B81F4-3335-4B4D-B766-303E136ED374} + true + + + + + + + + diff --git a/src/test/msi/WixToolsetTest.MsiE2E/RemoveFolderExTests.cs b/src/test/msi/WixToolsetTest.MsiE2E/RemoveFolderExTests.cs new file mode 100644 index 000000000..c6780a1b8 --- /dev/null +++ b/src/test/msi/WixToolsetTest.MsiE2E/RemoveFolderExTests.cs @@ -0,0 +1,82 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. + +namespace WixToolsetTest.MsiE2E; + +using System; +using System.IO; +using WixTestTools; +using Xunit; +using Xunit.Abstractions; + +public class RemoveFolderExTests : MsiE2ETests +{ + public RemoveFolderExTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) + { + } + + [Fact] + public void CanRemoveFolderExOnInstallAndUninstall() + { + var removeFolderExTestDir1 = "C:\\RemoveFolderExTest"; + var removeFolderExTestFile1 = Path.Combine(removeFolderExTestDir1, "testfile.txt"); + var removeFolderExTestDir2 = Path.Combine(removeFolderExTestDir1, "TestFolder1"); + var removeFolderExTestFile2 = Path.Combine(removeFolderExTestDir1, "TestFolder1", "testfile"); + + try + { + var product = this.CreatePackageInstaller("RemoveFolderExTest"); + + Directory.CreateDirectory(removeFolderExTestDir1); + File.Create(removeFolderExTestFile1).Dispose(); + Directory.CreateDirectory(removeFolderExTestDir2); + File.Create(removeFolderExTestFile2).Dispose(); + + if( !Directory.Exists(removeFolderExTestDir1) + || !File.Exists(removeFolderExTestFile1) + || !Directory.Exists(removeFolderExTestDir2) + || !File.Exists(removeFolderExTestFile2)) + { + Assert.Fail("Failed to create initial folder and file structure before install test"); + } + + product.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); + + Assert.False(Directory.Exists(removeFolderExTestDir1), $"Failed to remove {removeFolderExTestDir1} on install"); + Assert.False(File.Exists(removeFolderExTestFile1), $"Failed to remove {removeFolderExTestFile1} on install"); + Assert.False(Directory.Exists(removeFolderExTestDir2), $"Failed to remove {removeFolderExTestDir2} on install"); + Assert.False(File.Exists(removeFolderExTestFile1), $"Failed to remove {removeFolderExTestFile2} on install"); + + + Directory.CreateDirectory(removeFolderExTestDir1); + File.Create(removeFolderExTestFile1).Dispose(); + Directory.CreateDirectory(removeFolderExTestDir2); + File.Create(removeFolderExTestFile2).Dispose(); + + if (!Directory.Exists(removeFolderExTestDir1) + || !File.Exists(removeFolderExTestFile1) + || !Directory.Exists(removeFolderExTestDir2) + || !File.Exists(removeFolderExTestFile2)) + { + Assert.Fail("Failed to create initial folder and file structure before uninstall test"); + } + + product.UninstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); + + Assert.False(Directory.Exists(removeFolderExTestDir1), $"Failed to remove {removeFolderExTestDir1} on uninstall"); + Assert.False(File.Exists(removeFolderExTestFile1), $"Failed to remove {removeFolderExTestFile1} on uninstall"); + Assert.False(Directory.Exists(removeFolderExTestDir2), $"Failed to remove {removeFolderExTestDir2} on uninstall"); + Assert.False(File.Exists(removeFolderExTestFile2), $"Failed to remove {removeFolderExTestFile2} on uninstall"); + + } + finally + { + try + { + Directory.Delete(removeFolderExTestDir1, true); + } + catch + { + } + } + } +} From 79948c00b8a6df6a411f298e0a869d7f92515799 Mon Sep 17 00:00:00 2001 From: Bevan Weiss Date: Fri, 2 Aug 2024 22:19:02 +1000 Subject: [PATCH 2/2] Fix up small inaccuracy in logged error message. When ::GetFileAttributesW returns 0xFFFFFFFF it means 'Invalid File/Folder' So we should return a matching error message. To avoid confusing invalid paths with junctions (in error message) Unfortunately the constant for this is not defined. So just define it here as though it would be. Signed-off-by: Bevan Weiss --- src/ext/Util/ca/RemoveFoldersEx.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ext/Util/ca/RemoveFoldersEx.cpp b/src/ext/Util/ca/RemoveFoldersEx.cpp index f162e929b..5e813ca22 100644 --- a/src/ext/Util/ca/RemoveFoldersEx.cpp +++ b/src/ext/Util/ca/RemoveFoldersEx.cpp @@ -2,6 +2,12 @@ #include "precomp.h" +// Whilst ::GetFileAttributesW might return FILE_ATTRIBUTE_INVALID for an invalid path, it's not a header defined variable. +// so define it here, but guard it to avoid redefining it if Microsoft change their mind +#ifndef FILE_ATTRIBUTE_INVALID +#define FILE_ATTRIBUTE_INVALID ((DWORD)-1) +#endif + LPCWSTR vcsRemoveFolderExQuery = L"SELECT `RemoveFolderEx`, `Component_`, `Property`, `InstallMode`, `Wix4RemoveFolderEx`.`Condition`, `Component`.`Attributes` " L"FROM `Wix4RemoveFolderEx`,`Component` " @@ -38,8 +44,14 @@ static HRESULT RecursePath( } #endif - // Do NOT follow junctions. DWORD dwAttributes = ::GetFileAttributesW(wzPath); + if (FILE_ATTRIBUTE_INVALID == dwAttributes) + { + WcaLog(LOGMSG_STANDARD, "Path is invalid: %ls", wzPath); + ExitFunction(); + } + + // Do NOT follow junctions. if (dwAttributes & FILE_ATTRIBUTE_REPARSE_POINT) { WcaLog(LOGMSG_STANDARD, "Path is a junction; skipping: %ls", wzPath);