Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add icon extraction to installer metadata collection #3235

Merged
merged 15 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/actions/spelling/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ BEFACEF
bfd
BFirst
bght
BITMAPINFOHEADER
bitmask
bkup
blargle
Expand All @@ -45,6 +46,7 @@ bomgar
BOMs
boop
boundparms
bpp
Browsable
BSODs
buildtransitive
Expand Down Expand Up @@ -137,12 +139,15 @@ GHS
gity
goku
Google
GRPICONDIR
GRPICONDIRENTRY
guiddef
Hackathon
hashtable
helplib
helplibrary
hhx
highcontrast
HINSTANCE
hkey
hlocal
Expand All @@ -152,6 +157,9 @@ hre
hresults
IARP
IAttachment
ICONDIR
ICONDIRENTRY
ICONIMAGE
idl
idx
IEnum
Expand All @@ -162,10 +170,12 @@ IISOn
inet
inproc
installinprogress
INSTALLPROPERTY
installshield
instream
insufficientmemory
Intelli
INTRESOURCE
invalidparameter
isable
ishelp
Expand Down Expand Up @@ -195,9 +205,15 @@ Linq
liv
liwpx
localizationpriority
LOWORD
LPBYTE
LPCWSTR
LPDWORD
LPGRPICONDIR
LPGRPICONDIRENTRY
LPICONDIR
LPICONDIRENTRY
LPICONIMAGE
lpitemidlist
LPW
maclachlan
Expand Down Expand Up @@ -247,6 +263,7 @@ netfx
netlify
NETSDK
Newtonsoft
NOCRLF
NOEXPAND
NOLINKINFO
nonetwork
Expand Down Expand Up @@ -297,6 +314,7 @@ powertoys
pri
processthreads
productcode
PRODUCTICON
pscustomobject
pseudocode
PSHOST
Expand All @@ -321,6 +339,7 @@ relativefilepath
remoting
reparse
restsource
RGBQUAD
rgex
rgp
rgpsz
Expand Down
7 changes: 7 additions & 0 deletions src/AppInstallerCLITests/AppInstallerCLITests.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@
<ClCompile Include="GroupPolicy.cpp" />
<ClCompile Include="HashCommand.cpp" />
<ClCompile Include="HttpClientHelper.cpp" />
<ClCompile Include="IconExtraction.cpp" />
<ClCompile Include="ImportFlow.cpp" />
<ClCompile Include="InstallDependenciesFlow.cpp" />
<ClCompile Include="InstallerMetadataCollectionContext.cpp" />
Expand Down Expand Up @@ -813,6 +814,12 @@
<CopyFileToFolders Include="TestData\Node-Types.yaml">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\notepad.exe">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\notepad.ico">
<DeploymentContent>true</DeploymentContent>
</CopyFileToFolders>
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\AppInstallerCLICore\AppInstallerCLICore.vcxproj">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@
<ClCompile Include="RestInterface_1_4.cpp">
<Filter>Source Files\Repository</Filter>
</ClCompile>
<ClCompile Include="IconExtraction.cpp">
<Filter>Source Files\Repository</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
Expand Down Expand Up @@ -837,5 +840,11 @@
<CopyFileToFolders Include="TestData\Node-Types.yaml">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\notepad.exe">
<Filter>TestData</Filter>
</CopyFileToFolders>
<CopyFileToFolders Include="TestData\notepad.ico">
<Filter>TestData</Filter>
</CopyFileToFolders>
</ItemGroup>
</Project>
18 changes: 18 additions & 0 deletions src/AppInstallerCLITests/IconExtraction.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
#include "pch.h"
#include "TestCommon.h"
#include <winget/IconExtraction.h>
#include <AppInstallerStrings.h>

using namespace AppInstaller::Repository;

TEST_CASE("ExtractIconFromBinaryFile", "IconExtraction")
{
auto extracted = ExtractIconFromBinaryFile(TestCommon::TestDataFile{ "notepad.exe" }.GetPath());

std::ifstream expectedIconFile{ TestCommon::TestDataFile{ "notepad.ico" }.GetPath(), std::ios::in | std::ios::binary};
auto expected = AppInstaller::Utility::ReadEntireStreamAsByteArray(expectedIconFile);

REQUIRE(expected == extracted);
}
105 changes: 101 additions & 4 deletions src/AppInstallerCLITests/InstallerMetadataCollectionContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "pch.h"
#include "TestCommon.h"
#include "TestSource.h"
#include "TestHooks.h"

#include <winget/InstallerMetadataCollectionContext.h>

Expand Down Expand Up @@ -912,6 +913,17 @@ TEST_CASE("MetadataCollection_NewPackage_1_2", "[metadata_collection]")

installedFilesData->InstallationMetadata = std::move(installedFiles);

std::vector<AppInstaller::Repository::ExtractedIconInfo> testIcons;
AppInstaller::Repository::ExtractedIconInfo iconInfo;
iconInfo.IconContent = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b700");
iconInfo.IconSha256 = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b759");
iconInfo.IconFileType = Manifest::IconFileTypeEnum::Ico;
iconInfo.IconResolution = Manifest::IconResolutionEnum::Custom;
iconInfo.IconTheme = Manifest::IconThemeEnum::Default;
testIcons.emplace_back(std::move(iconInfo));

TestHook::SetExtractIconFromArpEntryResult_Override iconsOverride{ testIcons };

InstallerMetadataCollectionContext context = CreateTestContext(std::move(correlationData), std::move(installedFilesData), input);
TestOutput output = GetOutput(context);

Expand Down Expand Up @@ -943,10 +955,16 @@ TEST_CASE("MetadataCollection_NewPackage_1_2", "[metadata_collection]")
REQUIRE(entry.StartupLinkFiles->size() == 1);
REQUIRE(entry.StartupLinkFiles->at(0).RelativeFilePath == "TestApp.lnk");
REQUIRE(entry.StartupLinkFiles->at(0).FileType == Manifest::InstalledFileTypeEnum::Launch);
REQUIRE(entry.Icons.size() == 1);
REQUIRE(entry.Icons[0].IconContent == Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b700"));
REQUIRE(entry.Icons[0].IconSha256 == Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b759"));
REQUIRE(entry.Icons[0].IconFileType == Manifest::IconFileTypeEnum::Ico);
REQUIRE(entry.Icons[0].IconResolution == Manifest::IconResolutionEnum::Custom);
REQUIRE(entry.Icons[0].IconTheme == Manifest::IconThemeEnum::Default);
REQUIRE(output.Metadata->HistoricalMetadataList.empty());
}

TEST_CASE("MetadataCollection_NewPackage_NoInstallationMetadata", "[metadata_collection]")
TEST_CASE("MetadataCollection_NewPackage_NoInstallationMetadata_NoIcons", "[metadata_collection]")
{
TestInput input(MinimalDefaults);
input.SupportedMetadataVersion = "1.2";
Expand All @@ -964,6 +982,9 @@ TEST_CASE("MetadataCollection_NewPackage_NoInstallationMetadata", "[metadata_col

correlationData->CorrelateForNewlyInstalledResult.Package = std::make_shared<TestPackageVersion>(manifest, metadata);

std::vector<AppInstaller::Repository::ExtractedIconInfo> testIcons;
TestHook::SetExtractIconFromArpEntryResult_Override iconsOverride{ testIcons };

InstallerMetadataCollectionContext context = CreateTestContext(std::move(correlationData), input);
TestOutput output = GetOutput(context);

Expand All @@ -976,9 +997,10 @@ TEST_CASE("MetadataCollection_NewPackage_NoInstallationMetadata", "[metadata_col
REQUIRE(entry.Scope.empty());
REQUIRE_FALSE(entry.InstalledFiles.has_value());
REQUIRE_FALSE(entry.StartupLinkFiles.has_value());
REQUIRE(entry.Icons.size() == 0);
}

TEST_CASE("MetadataCollection_SameSubmission_SameInstaller_InstallationMetadata", "[metadata_collection]")
TEST_CASE("MetadataCollection_SameSubmission_SameInstaller_InstallationMetadata_Icons", "[metadata_collection]")
{
std::string version = "1.3.5";
std::string productCode = "{guid}";
Expand Down Expand Up @@ -1006,6 +1028,16 @@ TEST_CASE("MetadataCollection_SameSubmission_SameInstaller_InstallationMetadata"
startupLinkFiles.emplace_back(std::move(startupLink));
input.CurrentMetadata->InstallerMetadataMap.begin()->second.StartupLinkFiles = std::move(startupLinkFiles);

std::vector<AppInstaller::Repository::ExtractedIconInfo> testIcons;
AppInstaller::Repository::ExtractedIconInfo iconInfo;
iconInfo.IconContent = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b700");
iconInfo.IconSha256 = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b759");
iconInfo.IconFileType = Manifest::IconFileTypeEnum::Ico;
iconInfo.IconResolution = Manifest::IconResolutionEnum::Custom;
iconInfo.IconTheme = Manifest::IconThemeEnum::Default;
testIcons.emplace_back(std::move(iconInfo));
input.CurrentMetadata->InstallerMetadataMap.begin()->second.Icons = std::move(testIcons);

auto correlationData = std::make_unique<TestARPCorrelationData>();
auto installedFilesData = std::make_unique<TestInstalledFilesCorrelation>();

Expand Down Expand Up @@ -1038,6 +1070,17 @@ TEST_CASE("MetadataCollection_SameSubmission_SameInstaller_InstallationMetadata"

installedFilesData->InstallationMetadata = std::move(newInstalledFiles);

std::vector<AppInstaller::Repository::ExtractedIconInfo> newTestIcons;
AppInstaller::Repository::ExtractedIconInfo newIconInfo;
newIconInfo.IconContent = Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6");
newIconInfo.IconSha256 = Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df8499");
newIconInfo.IconFileType = Manifest::IconFileTypeEnum::Jpeg;
newIconInfo.IconResolution = Manifest::IconResolutionEnum::Square16;
newIconInfo.IconTheme = Manifest::IconThemeEnum::Light;
newTestIcons.emplace_back(std::move(newIconInfo));

TestHook::SetExtractIconFromArpEntryResult_Override iconsOverride{ newTestIcons };

InstallerMetadataCollectionContext context = CreateTestContext(std::move(correlationData), std::move(installedFilesData), input);
TestOutput output = GetOutput(context);

Expand All @@ -1054,6 +1097,13 @@ TEST_CASE("MetadataCollection_SameSubmission_SameInstaller_InstallationMetadata"
// Non duplicate startup links get added.
REQUIRE(entry.StartupLinkFiles.has_value());
REQUIRE(entry.StartupLinkFiles->size() == 2);
// New detected icons always take over
REQUIRE(entry.Icons.size() == 1);
REQUIRE(entry.Icons[0].IconContent == Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6"));
REQUIRE(entry.Icons[0].IconSha256 == Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df8499"));
REQUIRE(entry.Icons[0].IconFileType == Manifest::IconFileTypeEnum::Jpeg);
REQUIRE(entry.Icons[0].IconResolution == Manifest::IconResolutionEnum::Square16);
REQUIRE(entry.Icons[0].IconTheme == Manifest::IconThemeEnum::Light);
}

TEST_CASE("MetadataCollection_Merge_SameInstaller_InstalledFiles", "[metadata_collection]")
Expand All @@ -1073,11 +1123,12 @@ TEST_CASE("MetadataCollection_Merge_SameInstaller_InstalledFiles", "[metadata_co

mergeData.Metadatas->at(0).SchemaVersion = { "1.2" };
mergeData.Metadatas->at(0).InstallerMetadataMap.begin()->second.InstalledFiles = installedFiles;
mergeData.Metadatas->at(1).SchemaVersion = { "1.2" };

// Different default install location clears whole data
Manifest::InstallationMetadataInfo newInstalledFiles = installedFiles;
newInstalledFiles.DefaultInstallLocation = "%TEMP%\\NewTestApp";
mergeData.Metadatas->at(1).SchemaVersion = { "1.2" };

mergeData.Metadatas->at(1).InstallerMetadataMap.begin()->second.InstalledFiles = newInstalledFiles;
std::wstring mergeResult = InstallerMetadataCollectionContext::Merge(mergeData.ToJSON(), 0, {});
REQUIRE(!mergeResult.empty());
Expand Down Expand Up @@ -1139,11 +1190,12 @@ TEST_CASE("MetadataCollection_Merge_SameInstaller_StartupLinkFiles", "[metadata_

mergeData.Metadatas->at(0).SchemaVersion = { "1.2" };
mergeData.Metadatas->at(0).InstallerMetadataMap.begin()->second.StartupLinkFiles = startupLinkFiles;
mergeData.Metadatas->at(1).SchemaVersion = { "1.2" };

// Different relative file path gets added
std::vector<InstalledStartupLinkFile> newStartupLinkFiles = startupLinkFiles;
newStartupLinkFiles[0].RelativeFilePath = "TestApp2.lnk";

mergeData.Metadatas->at(1).SchemaVersion = { "1.2" };
mergeData.Metadatas->at(1).InstallerMetadataMap.begin()->second.StartupLinkFiles = newStartupLinkFiles;
std::wstring mergeResult = InstallerMetadataCollectionContext::Merge(mergeData.ToJSON(), 0, {});
REQUIRE(!mergeResult.empty());
Expand All @@ -1169,4 +1221,49 @@ TEST_CASE("MetadataCollection_Merge_SameInstaller_StartupLinkFiles", "[metadata_
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.StartupLinkFiles->size() == 1);
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.StartupLinkFiles->at(0).RelativeFilePath == "TestApp.lnk");
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.StartupLinkFiles->at(0).FileType == Manifest::InstalledFileTypeEnum::Unknown);
}

TEST_CASE("MetadataCollection_Merge_SameInstaller_Icons", "[metadata_collection]")
{
TestMerge mergeData{ MinimalDefaults };
mergeData.Metadatas->emplace_back(MakeProductMetadata());

std::vector<AppInstaller::Repository::ExtractedIconInfo> testIcons;
AppInstaller::Repository::ExtractedIconInfo iconInfo;
iconInfo.IconContent = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b700");
iconInfo.IconSha256 = Utility::SHA256::ConvertToBytes("d2a45116709136462ee7a1c42f0e75f0efa258fe959b1504dc8ea4573451b759");
iconInfo.IconFileType = Manifest::IconFileTypeEnum::Ico;
iconInfo.IconResolution = Manifest::IconResolutionEnum::Custom;
iconInfo.IconTheme = Manifest::IconThemeEnum::Default;
testIcons.emplace_back(std::move(iconInfo));

mergeData.Metadatas->at(0).SchemaVersion = { "1.2" };
mergeData.Metadatas->at(0).InstallerMetadataMap.begin()->second.Icons = testIcons;

// Different test icons
std::vector<AppInstaller::Repository::ExtractedIconInfo> newTestIcons;
AppInstaller::Repository::ExtractedIconInfo newIconInfo;
newIconInfo.IconContent = Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6");
newIconInfo.IconSha256 = Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df8499");
newIconInfo.IconFileType = Manifest::IconFileTypeEnum::Jpeg;
newIconInfo.IconResolution = Manifest::IconResolutionEnum::Square16;
newIconInfo.IconTheme = Manifest::IconThemeEnum::Light;
newTestIcons.emplace_back(std::move(newIconInfo));

mergeData.Metadatas->at(1).SchemaVersion = { "1.2" };
mergeData.Metadatas->at(1).InstallerMetadataMap.begin()->second.Icons = newTestIcons;
std::wstring mergeResult = InstallerMetadataCollectionContext::Merge(mergeData.ToJSON(), 0, {});
REQUIRE(!mergeResult.empty());

ProductMetadata mergeMetadata;
mergeMetadata.FromJson(web::json::value::parse(mergeResult));

// New data always take over
REQUIRE(mergeMetadata.InstallerMetadataMap.size() == 1);
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons.size() == 1);
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons[0].IconContent == Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df84b6"));
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons[0].IconSha256 == Utility::SHA256::ConvertToBytes("011048877dfaef109801b3f3ab2b60afc74f3fc4f7b3430e0c897f5da1df8499"));
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons[0].IconFileType == Manifest::IconFileTypeEnum::Jpeg);
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons[0].IconResolution == Manifest::IconResolutionEnum::Square16);
REQUIRE(mergeMetadata.InstallerMetadataMap.begin()->second.Icons[0].IconTheme == Manifest::IconThemeEnum::Light);
}
Binary file added src/AppInstallerCLITests/TestData/notepad.exe
Binary file not shown.
Binary file added src/AppInstallerCLITests/TestData/notepad.ico
Binary file not shown.
18 changes: 18 additions & 0 deletions src/AppInstallerCLITests/TestHooks.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <winget/UserSettings.h>
#include <winget/Filesystem.h>
#include <winget/WindowsFeature.h>
#include <winget/IconExtraction.h>

#ifdef AICLI_DISABLE_TEST_HOOKS
static_assert(false, "Test hooks have been disabled");
Expand All @@ -33,6 +34,7 @@ namespace AppInstaller
{
void TestHook_SetSourceFactoryOverride(const std::string& type, std::function<std::unique_ptr<ISourceFactory>()>&& factory);
void TestHook_ClearSourceFactoryOverrides();
void TestHook_SetExtractIconFromArpEntryResult_Override(std::vector<AppInstaller::Repository::ExtractedIconInfo>* result);
}

namespace Repository::Microsoft
Expand Down Expand Up @@ -210,4 +212,20 @@ namespace TestHook
private:
AppInstaller::WindowsFeature::DismRestartType m_restartType;
};

struct SetExtractIconFromArpEntryResult_Override
{
SetExtractIconFromArpEntryResult_Override(std::vector<AppInstaller::Repository::ExtractedIconInfo> extractedIcons) : m_extractedIcons(std::move(extractedIcons))
{
AppInstaller::Repository::TestHook_SetExtractIconFromArpEntryResult_Override(&m_extractedIcons);
}

~SetExtractIconFromArpEntryResult_Override()
{
AppInstaller::Repository::TestHook_SetExtractIconFromArpEntryResult_Override(nullptr);
}

private:
std::vector<AppInstaller::Repository::ExtractedIconInfo> m_extractedIcons;
};
}
Loading