Skip to content

Commit 60a18ba

Browse files
DrusTheAxeDrusTheAxe
authored
EnsurePackage*Async() handling options.RegisterNewerIfAvailable(true). RegisterPackageByPackageFull/FamilyNameAsync crash. Test + Warning fixes (#4845)
EnsurePackage*Async() not handling options.RegisterNewerIfAvailable(true) RegisterPackageByPackageFullNameAsync() and RegisterPackageByPackageFullNameAsync() take their target parameter as const hstring& resulting in the reference captured by their implementations' co_await and not the actual value. The caller deallocates the memory before Register... is done with it potentially leading to BadMojo(TM). Tests don't notice it as they're too simple to notice this use-after-free -- the memory may be deallocated but the memory's not overwritten (yet) in simple loads (like the tests). Larger scale (real world) use with more diverse memory patterns notice the problem (usually as a crash). ApplicationDataTests_Elevated test cases were blocked state due to user context vs environmental state vs access control (ACLs whee!). Changed RunAs and associated test process setup to play nice together. Fixed some PackageManagerTests that had incomplete state setup potentially causing false errors if a past previous test failed. Fixed 400+ build warnings (mostly CS8305 due to [Experimental] attribute, almost all from OAuth generated C#/WinRT) Removed redundant .props import (fixing a VS warning) Fixed misleading log message Removed wrong test cases Added diagnostics to all Setup fixtures to hunt down Setup failures Change IsReady tests to RunAs RestrictedUser Fixed compiler warnings Split ApplicationDataTests_Elevated to separate source file. Added diagnostics to ApplicationDataTests[_Elevated]. Moved TAEF display+compare support for C++/WinRT hstring into shared file (test/inc/WindowsAppRuntime.Test.TAEF.cppwinrt.h). Added Parameters to ApplicationData tests to (hopefully) troubleshoot failure. Added whomi /all to build pipeline test output. Changed ApplicationData tests' default to IsolationLevel=Class (not TAEF's default =Module) Changed PackageManager tests' default to IsolationLevel=Class (not TAEF's default =Module) Factored tests into separate runs to aid troubleshooting failures Added suppression of CS8305 (Feature is experimental... duh) as not helpful and negatively helpful (400+ warnings that are, for us, features). https://task.ms/54835036 https://task.ms/54858998 https://task.ms/54884960
1 parent 6a5ac0c commit 60a18ba

51 files changed

Lines changed: 1385 additions & 514 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎WindowsAppRuntime.sln‎

Lines changed: 84 additions & 69 deletions
Large diffs are not rendered by default.

‎build/AzurePipelinesTemplates/WindowsAppSDK-RunTests-Steps.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,9 @@ steps:
143143
Write-Host "Get-WinSystemLocale"
144144
Get-WinSystemLocale
145145
146+
Write-Host "WhoAmI"
147+
Write-Host (whoami /user /groups /priv)
148+
146149
- task: PowerShell@2
147150
displayName: 'Run TAEF Tests'
148151
inputs:

‎dev/ApplicationData/M.W.S.ApplicationData.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ namespace winrt::Microsoft::Windows::Storage::implementation
9292
throw;
9393
}
9494
}
95-
winrt::Microsoft::Windows::Storage::ApplicationData ApplicationData::GetForUnpackaged(hstring const& publisher, hstring const& product)
95+
winrt::Microsoft::Windows::Storage::ApplicationData ApplicationData::GetForUnpackaged(hstring const& /*publisher*/, hstring const& /*product*/)
9696
{
9797
// TODO implement GetForUnpackaged
9898
throw hresult_not_implemented();

‎dev/PackageManager/API/M.W.M.D.PackageDeploymentManager.cpp‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,17 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
429429
const double c_progressPercentageStartOfIsReady{ 0.01 };
430430
packageDeploymentProgress.Progress = c_progressPercentageStartOfIsReady;
431431
progress(packageDeploymentProgress);
432-
if (IsPackageSetReady(packageSet))
432+
bool isReady{};
433+
if (options.RegisterNewerIfAvailable())
434+
{
435+
THROW_HR_IF_MSG(E_NOTIMPL, !IsPackageDeploymentFeatureSupported(winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentFeature::IsPackageReadyOrNewerAvailable), "RegisterNewerIfAvailable is not supported on this system");
436+
isReady = (IsPackageSetReadyOrNewerAvailable(packageSet) == winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::Ready);
437+
}
438+
else
439+
{
440+
isReady = IsPackageSetReady(packageSet);
441+
}
442+
if (isReady)
433443
{
434444
co_return winrt::make<PackageDeploymentResult>(PackageDeploymentStatus::CompletedSuccess, winrt::guid{});
435445
}
@@ -1222,7 +1232,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
12221232
catch (...)
12231233
{
12241234
const auto exception{ hresult_error(to_hresult(), take_ownership_from_abi) };
1225-
error = LOG_HR_MSG(exception.code(), "ExtendedError:0x%08X PackageFamilyName:%ls PackageUri:%ls",
1235+
error = LOG_HR_MSG(exception.code(), "ExtendedError:0x%08X PackageFullName:%ls PackageUri:%ls",
12261236
extendedError, packageFullName, packageUriAsString.c_str());
12271237
}
12281238
if (FAILED(error))
@@ -1392,7 +1402,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
13921402
catch (...)
13931403
{
13941404
const auto exception{ hresult_error(to_hresult(), take_ownership_from_abi) };
1395-
error = LOG_HR_MSG(exception.code(), "ExtendedError:0x%08X PackageFamilyName:%ls PackageUri:%ls",
1405+
error = LOG_HR_MSG(exception.code(), "ExtendedError:0x%08X PackageFullName:%ls PackageUri:%ls",
13961406
extendedError, packageFullName, packageUriAsString.c_str());
13971407
}
13981408
if (FAILED(error))
@@ -1612,7 +1622,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
16121622
}
16131623

16141624
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress>
1615-
PackageDeploymentManager::RegisterPackageByPackageFamilyNameAsync(winrt::hstring const& packageFamilyName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options)
1625+
PackageDeploymentManager::RegisterPackageByPackageFamilyNameAsync(const winrt::hstring packageFamilyName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options)
16161626
{
16171627
auto logTelemetry{ PackageManagementTelemetry::RegisterPackageByPackageFamilyNameAsync::Start(packageFamilyName) };
16181628

@@ -1660,7 +1670,7 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
16601670
}
16611671

16621672
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress>
1663-
PackageDeploymentManager::RegisterPackageByPackageFullNameAsync(winrt::hstring const& packageFullName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options)
1673+
PackageDeploymentManager::RegisterPackageByPackageFullNameAsync(const winrt::hstring packageFullName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options)
16641674
{
16651675
auto logTelemetry{ PackageManagementTelemetry::RegisterPackageByPackageFullNameAsync::Start(packageFullName) };
16661676

@@ -1892,7 +1902,17 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
18921902
errorText.clear();
18931903
activityId = winrt::guid{};
18941904

1895-
if (IsReady(packageSetItem))
1905+
bool isReady{};
1906+
if (options.RegisterNewerIfAvailable())
1907+
{
1908+
// Our caller already verified PackageDeploymentFeature::IsPackageReadyOrNewerAvailable is supported so no need to check again
1909+
isReady = (IsReadyOrNewerAvailable(packageSetItem) == winrt::Microsoft::Windows::Management::Deployment::PackageReadyOrNewerAvailableStatus::Ready);
1910+
}
1911+
else
1912+
{
1913+
isReady = IsReady(packageSetItem);
1914+
}
1915+
if (isReady)
18961916
{
18971917
return S_OK;
18981918
}

‎dev/PackageManager/API/M.W.M.D.PackageDeploymentManager.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
6060

6161
private:
6262
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress> AddPackageByAppInstallerFileAsync(winrt::Windows::Foundation::Uri packageUri, winrt::Microsoft::Windows::Management::Deployment::AddPackageOptions options);
63-
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress> RegisterPackageByPackageFamilyNameAsync(winrt::hstring const& packageFamilyName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options);
64-
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress> RegisterPackageByPackageFullNameAsync(winrt::hstring const& packageFullName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options);
63+
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress> RegisterPackageByPackageFamilyNameAsync(const winrt::hstring packageFamilyName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options);
64+
winrt::Windows::Foundation::IAsyncOperationWithProgress<winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentResult, winrt::Microsoft::Windows::Management::Deployment::PackageDeploymentProgress> RegisterPackageByPackageFullNameAsync(const winrt::hstring packageFullName, winrt::Microsoft::Windows::Management::Deployment::RegisterPackageOptions options);
6565

6666
private:
6767
winrt::hstring GetUupProductIdIfMsUup(winrt::Windows::Foundation::Uri const& uri) const;

‎dev/PackageManager/API/M.W.M.D.PackageRuntimeManager.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,12 @@ namespace winrt::Microsoft::Windows::Management::Deployment::implementation
103103
THROW_HR_IF_NULL_MSG(E_INVALIDARG, packageUri, "PackageUri:<null>");
104104
}
105105

106-
void PackageRuntimeManager::Validate(winrt::Microsoft::Windows::ApplicationModel::DynamicDependency::CreatePackageDependencyOptions const& options) const
106+
void PackageRuntimeManager::Validate(winrt::Microsoft::Windows::ApplicationModel::DynamicDependency::CreatePackageDependencyOptions const& /*options*/ ) const
107107
{
108108
// Nothing to do!
109109
}
110110

111-
void PackageRuntimeManager::Validate(winrt::Microsoft::Windows::ApplicationModel::DynamicDependency::AddPackageDependencyOptions const& options) const
111+
void PackageRuntimeManager::Validate(winrt::Microsoft::Windows::ApplicationModel::DynamicDependency::AddPackageDependencyOptions const& /*options*/) const
112112
{
113113
// Nothing to do!
114114
}

‎dev/Projections/CS/Microsoft.Security.Authentication.OAuth/Microsoft.Security.Authentication.OAuth.Projection.csproj‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
<IsTrimmable>true</IsTrimmable>
1313
</PropertyGroup>
1414

15+
<!-- Suppress CS8305: Feature is for evaluation purposes only and is subject to change or removal in future updates. -->
16+
<PropertyGroup>
17+
<NoWarn>8305</NoWarn>
18+
</PropertyGroup>
19+
1520
<ItemGroup>
1621
<PackageReference Include="Microsoft.ProjectReunion.InteractiveExperiences.TransportPackage" />
1722
<PackageReference Include="Microsoft.SourceLink.Common">

‎dev/Projections/CS/Microsoft.Windows.AppLifecycle/Microsoft.Windows.AppLifecycle.Projection.csproj‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<Project Sdk="Microsoft.NET.Sdk">
1+
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFramework>net6.0-windows10.0.17763.0</TargetFramework>
44
<TargetPlatformMinVersion>10.0.17763.0</TargetPlatformMinVersion>
@@ -12,6 +12,11 @@
1212
<IsTrimmable>true</IsTrimmable>
1313
</PropertyGroup>
1414

15+
<!-- Suppress CS8305: Feature is for evaluation purposes only and is subject to change or removal in future updates. -->
16+
<PropertyGroup>
17+
<NoWarn>8305</NoWarn>
18+
</PropertyGroup>
19+
1520
<ItemGroup>
1621
<PackageReference Include="Microsoft.SourceLink.Common">
1722
<PrivateAssets>all</PrivateAssets>

‎dev/Projections/CS/Microsoft.Windows.AppNotifications.Builder.Projection/Microsoft.Windows.AppNotifications.Builder.Projection.csproj‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
<IsTrimmable>true</IsTrimmable>
1313
</PropertyGroup>
1414

15+
<!-- Suppress CS8305: Feature is for evaluation purposes only and is subject to change or removal in future updates. -->
16+
<PropertyGroup>
17+
<NoWarn>8305</NoWarn>
18+
</PropertyGroup>
19+
1520
<ItemGroup>
1621
<PackageReference Include="Microsoft.SourceLink.Common">
1722
<PrivateAssets>all</PrivateAssets>

‎dev/Projections/CS/Microsoft.Windows.AppNotifications.Projection/Microsoft.Windows.AppNotifications.Projection.csproj‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212
<IsTrimmable>true</IsTrimmable>
1313
</PropertyGroup>
1414

15+
<!-- Suppress CS8305: Feature is for evaluation purposes only and is subject to change or removal in future updates. -->
16+
<PropertyGroup>
17+
<NoWarn>8305</NoWarn>
18+
</PropertyGroup>
19+
1520
<ItemGroup>
1621
<PackageReference Include="Microsoft.SourceLink.Common">
1722
<PrivateAssets>all</PrivateAssets>

0 commit comments

Comments
 (0)