From 88187b668afcaa1dead0acb43fbdeab9e4884667 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Mon, 18 Nov 2024 17:38:33 +0100 Subject: [PATCH 1/6] Move helpers above XML parser code Move various helper functions above the Expat XML parsing handlers, instead of having them interleaved. No actual changes. --- src/appcast.cpp | 139 +++++++++++++++++++++++++----------------------- 1 file changed, 73 insertions(+), 66 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index c7d78a41..f88cf3d8 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -34,13 +34,82 @@ namespace winsparkle { +namespace +{ + +// Misc helper functions: + +#define OS_MARKER "windows" +#define OS_MARKER_LEN 7 + + +bool is_windows_version_acceptable(const Appcast &item) +{ + if (item.MinOSVersion.empty()) + return true; + + OSVERSIONINFOEXW osvi = { sizeof(osvi), 0, 0, 0, 0, { 0 }, 0, 0 }; + DWORDLONG const dwlConditionMask = VerSetConditionMask( + VerSetConditionMask( + VerSetConditionMask( + 0, VER_MAJORVERSION, VER_GREATER_EQUAL), + VER_MINORVERSION, VER_GREATER_EQUAL), + VER_SERVICEPACKMAJOR, VER_GREATER_EQUAL); + + sscanf(item.MinOSVersion.c_str(), "%lu.%lu.%hu", &osvi.dwMajorVersion, + &osvi.dwMinorVersion, &osvi.wServicePackMajor); + + return VerifyVersionInfoW(&osvi, VER_MAJORVERSION | VER_MINORVERSION | + VER_SERVICEPACKMAJOR, dwlConditionMask) != FALSE; +} + + +/** + * Returns true if item os is exactly "windows" + * or if item is "windows-arm64" on 64bit ARM + * or if item is "windows-x64" on 64bit Intel/AMD + * or if item is "windows-x86" on 32bit + * and is above minimum version + */ +bool is_suitable_windows_item(const Appcast& item) +{ + if (!is_windows_version_acceptable(item)) + return false; + + if (item.Os == OS_MARKER) + return true; + + if (item.Os.compare(0, OS_MARKER_LEN, OS_MARKER) != 0) + return false; + + // Check suffix for matching bitness +#ifdef _WIN64 +#if defined(__AARCH64EL__) || defined(_M_ARM64) + return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-arm64") == 0; +#else + return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x64") == 0; +#endif // defined(__AARCH64EL__) || defined(_M_ARM64) +#else + return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x86") == 0; +#endif // _WIN64 +} + + +void trim_whitespace(std::string& s) +{ + size_t startpos = s.find_first_not_of(" \t\r\n"); + if (startpos != std::string::npos) + s = s.substr(startpos); + size_t endpos = s.find_last_not_of(" \t\r\n"); + if (endpos != std::string::npos) + s = s.substr(0, endpos + 1); +} + + /*--------------------------------------------------------------------------* XML parsing *--------------------------------------------------------------------------*/ -namespace -{ - #define MVAL(x) x #define CONCAT3(a,b,c) MVAL(a)##MVAL(b)##MVAL(c) @@ -66,8 +135,7 @@ namespace #define NODE_VERSION ATTR_VERSION // These can be nodes or #define NODE_SHORTVERSION ATTR_SHORTVERSION // attributes. #define NODE_DSASIGNATURE ATTR_DSASIGNATURE -#define OS_MARKER "windows" -#define OS_MARKER_LEN 7 + // context data for the parser struct ContextData @@ -91,25 +159,6 @@ struct ContextData std::vector items; }; -bool is_windows_version_acceptable(const Appcast &item) -{ - if (item.MinOSVersion.empty()) - return true; - - OSVERSIONINFOEXW osvi = { sizeof(osvi), 0, 0, 0, 0, { 0 }, 0, 0 }; - DWORDLONG const dwlConditionMask = VerSetConditionMask( - VerSetConditionMask( - VerSetConditionMask( - 0, VER_MAJORVERSION, VER_GREATER_EQUAL), - VER_MINORVERSION, VER_GREATER_EQUAL), - VER_SERVICEPACKMAJOR, VER_GREATER_EQUAL); - - sscanf(item.MinOSVersion.c_str(), "%lu.%lu.%hu", &osvi.dwMajorVersion, - &osvi.dwMinorVersion, &osvi.wServicePackMajor); - - return VerifyVersionInfoW(&osvi, VER_MAJORVERSION | VER_MINORVERSION | - VER_SERVICEPACKMAJOR, dwlConditionMask) != FALSE; -} void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) { @@ -193,48 +242,6 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) } -/** - * Returns true if item os is exactly "windows" - * or if item is "windows-arm64" on 64bit ARM - * or if item is "windows-x64" on 64bit Intel/AMD - * or if item is "windows-x86" on 32bit - * and is above minimum version - */ -bool is_suitable_windows_item(const Appcast &item) -{ - if (!is_windows_version_acceptable(item)) - return false; - - if (item.Os == OS_MARKER) - return true; - - if (item.Os.compare(0, OS_MARKER_LEN, OS_MARKER) != 0) - return false; - - // Check suffix for matching bitness -#ifdef _WIN64 - #if defined(__AARCH64EL__) || defined(_M_ARM64) - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-arm64") == 0; - #else - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x64") == 0; - #endif // defined(__AARCH64EL__) || defined(_M_ARM64) -#else - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x86") == 0; -#endif // _WIN64 -} - - -void trim_whitespace(std::string& s) -{ - size_t startpos = s.find_first_not_of(" \t\r\n"); - if (startpos != std::string::npos) - s = s.substr(startpos); - size_t endpos = s.find_last_not_of(" \t\r\n"); - if (endpos != std::string::npos) - s = s.substr(0, endpos + 1); -} - - void XMLCALL OnEndElement(void *data, const char *name) { ContextData& ctxt = *static_cast(data); From 860adcdb3bfb502b18e7f792ac89f0b4ae538cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Tue, 19 Nov 2024 11:15:22 +0100 Subject: [PATCH 2/6] Split enclosure data into Appcast::Enclosure struct Refactoring to enable working with multiple enclosures. --- src/appcast.cpp | 20 ++++++++++---------- src/appcast.h | 33 ++++++++++++++++++++------------- src/ui.cpp | 2 +- src/updatechecker.cpp | 4 ++-- src/updatedownloader.cpp | 4 ++-- 5 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index f88cf3d8..b2092794 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -76,21 +76,21 @@ bool is_suitable_windows_item(const Appcast& item) if (!is_windows_version_acceptable(item)) return false; - if (item.Os == OS_MARKER) + if (item.enclosure.OS == OS_MARKER) return true; - if (item.Os.compare(0, OS_MARKER_LEN, OS_MARKER) != 0) + if (item.enclosure.OS.compare(0, OS_MARKER_LEN, OS_MARKER) != 0) return false; // Check suffix for matching bitness #ifdef _WIN64 #if defined(__AARCH64EL__) || defined(_M_ARM64) - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-arm64") == 0; + return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-arm64") == 0; #else - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x64") == 0; + return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-x64") == 0; #endif // defined(__AARCH64EL__) || defined(_M_ARM64) #else - return item.Os.compare(OS_MARKER_LEN, std::string::npos, "-x86") == 0; + return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-x86") == 0; #endif // _WIN64 } @@ -219,17 +219,17 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) const char* value = attrs[i + 1]; if (strcmp(name, ATTR_URL) == 0) - item.DownloadURL = value; + item.enclosure.DownloadURL = value; else if (strcmp(name, ATTR_VERSION) == 0) item.Version = value; else if (strcmp(name, ATTR_SHORTVERSION) == 0) item.ShortVersionString = value; else if (strcmp(name, ATTR_DSASIGNATURE) == 0) - item.DsaSignature = value; + item.enclosure.DsaSignature = value; else if (strcmp(name, ATTR_OS) == 0) - item.Os = value; + item.enclosure.OS = value; else if (strcmp(name, ATTR_ARGUMENTS) == 0) - item.InstallerArguments = value; + item.enclosure.InstallerArguments = value; } } } @@ -333,7 +333,7 @@ void XMLCALL OnText(void *data, const char *s, int len) } else if (ctxt.in_dsasignature) { - item.DsaSignature.append(s, len); + item.enclosure.DsaSignature.append(s, len); } else if (ctxt.in_min_os_version) { diff --git a/src/appcast.h b/src/appcast.h index 62899700..9d4227d9 100644 --- a/src/appcast.h +++ b/src/appcast.h @@ -40,12 +40,6 @@ struct Appcast std::string Version; std::string ShortVersionString; - /// URL of the update - std::string DownloadURL; - - /// Signing signature of the update - std::string DsaSignature; - /// URL of the release notes page std::string ReleaseNotesURL; @@ -58,18 +52,31 @@ struct Appcast /// Description of the update std::string Description; - // Operating system - std::string Os; - // Minimum OS version required for update std::string MinOSVersion; - // Arguments passed on the the updater executable - std::string InstallerArguments; - // CriticalUpdate? bool CriticalUpdate = false; + struct Enclosure + { + /// URL of the update + std::string DownloadURL; + + /// Signing signature of the update + std::string DsaSignature; + + // Operating system + std::string OS; + + // Arguments passed on the the updater executable + std::string InstallerArguments; + + bool IsValid() const { return !DownloadURL.empty(); } + }; + + Enclosure enclosure; + /** Initializes the struct with data from XML appcast feed. @@ -90,7 +97,7 @@ struct Appcast /// If true, then download and install the update ourselves. /// If false, launch a web browser to WebBrowserURL. - bool HasDownload() const { return !DownloadURL.empty(); } + bool HasDownload() const { return enclosure.IsValid(); } }; } // namespace winsparkle diff --git a/src/ui.cpp b/src/ui.cpp index 453b80ee..4966d844 100644 --- a/src/ui.cpp +++ b/src/ui.cpp @@ -1336,7 +1336,7 @@ void App::OnUpdateDownloaded(wxThreadEvent& event) if ( m_win ) { EventPayload payload(event.GetPayload()); - m_win->StateUpdateDownloaded(payload.updateFile, payload.appcast.InstallerArguments); + m_win->StateUpdateDownloaded(payload.updateFile, payload.appcast.enclosure.InstallerArguments); } } diff --git a/src/updatechecker.cpp b/src/updatechecker.cpp index a83e9ea3..4a653533 100644 --- a/src/updatechecker.cpp +++ b/src/updatechecker.cpp @@ -233,8 +233,8 @@ void UpdateChecker::PerformUpdateCheck() Appcast appcast = Appcast::Load(appcast_xml.data); if (!appcast.ReleaseNotesURL.empty()) CheckForInsecureURL(appcast.ReleaseNotesURL, "release notes"); - if (!appcast.DownloadURL.empty()) - CheckForInsecureURL(appcast.DownloadURL, "update file"); + if (!appcast.enclosure.DownloadURL.empty()) + CheckForInsecureURL(appcast.enclosure.DownloadURL, "update file"); Settings::WriteConfigValue("LastCheckTime", time(NULL)); diff --git a/src/updatedownloader.cpp b/src/updatedownloader.cpp index 1327d02a..52be8dfc 100644 --- a/src/updatedownloader.cpp +++ b/src/updatedownloader.cpp @@ -177,12 +177,12 @@ void UpdateDownloader::Run() Settings::WriteConfigValue("UpdateTempDir", tmpdir); UpdateDownloadSink sink(*this, tmpdir); - DownloadFile(m_appcast.DownloadURL, &sink, this); + DownloadFile(m_appcast.enclosure.DownloadURL, &sink, this); sink.Close(); if (Settings::HasDSAPubKeyPem()) { - SignatureVerifier::VerifyDSASHA1SignatureValid(sink.GetFilePath(), m_appcast.DsaSignature); + SignatureVerifier::VerifyDSASHA1SignatureValid(sink.GetFilePath(), m_appcast.enclosure.DsaSignature); } else { From 90421faac60a93193a943f33380245f23740db8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= Date: Tue, 19 Nov 2024 14:57:47 +0100 Subject: [PATCH 3/6] Clarify handling of legacy elements/attributes --- src/appcast.cpp | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index b2092794..0352b774 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -146,15 +146,24 @@ struct ContextData in_version(0), in_shortversion(0), in_dsasignature(0), in_min_os_version(0) {} + // call when entering element + void reset_for_new_item() + { + legacy_dsa_signature.clear(); + } + // the parser we're using XML_Parser& parser; // is inside , or , , <description>, or <link> respectively? int in_channel, in_item, in_relnotes, in_title, in_description, in_link; - // is inside <sparkle:version> or <sparkle:shortVersionString> node? + // is inside <sparkle:version> or <sparkle:shortVersionString> etc. node? int in_version, in_shortversion, in_dsasignature, in_min_os_version; + // signature present as <sparkle:dsaSignature>, not enclosure attribute + std::string legacy_dsa_signature; + // parsed <item>s std::vector<Appcast> items; }; @@ -173,6 +182,8 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) ctxt.in_item++; Appcast item; ctxt.items.push_back(item); + + ctxt.reset_for_new_item(); } else if ( ctxt.in_item ) { @@ -220,16 +231,17 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) if (strcmp(name, ATTR_URL) == 0) item.enclosure.DownloadURL = value; - else if (strcmp(name, ATTR_VERSION) == 0) - item.Version = value; - else if (strcmp(name, ATTR_SHORTVERSION) == 0) - item.ShortVersionString = value; else if (strcmp(name, ATTR_DSASIGNATURE) == 0) item.enclosure.DsaSignature = value; else if (strcmp(name, ATTR_OS) == 0) item.enclosure.OS = value; else if (strcmp(name, ATTR_ARGUMENTS) == 0) item.enclosure.InstallerArguments = value; + // legacy syntax where version info was on enclosure, not item: + else if (strcmp(name, ATTR_VERSION) == 0) + item.Version = value; + else if (strcmp(name, ATTR_SHORTVERSION) == 0) + item.ShortVersionString = value; } } } @@ -283,7 +295,13 @@ void XMLCALL OnEndElement(void *data, const char *name) else if (strcmp(name, NODE_ITEM) == 0) { ctxt.in_item--; - if (is_suitable_windows_item(ctxt.items[ctxt.items.size() - 1])) + + Appcast& item = ctxt.items.back(); + + if (!ctxt.legacyDsaSignature.empty() && item.enclosure.DsaSignature.empty()) + item.enclosure.DsaSignature = ctxt.legacy_dsa_signature; + + if (is_suitable_windows_item(item)) XML_StopParser(ctxt.parser, XML_TRUE); } } @@ -333,7 +351,7 @@ void XMLCALL OnText(void *data, const char *s, int len) } else if (ctxt.in_dsasignature) { - item.enclosure.DsaSignature.append(s, len); + ctxt.legacy_dsa_signature.assign(s, len); } else if (ctxt.in_min_os_version) { From d2618689ddb879dfc18f92538fa2faa28d82e1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= <vaclav@slavik.io> Date: Tue, 19 Nov 2024 15:13:01 +0100 Subject: [PATCH 4/6] Simplify <item> parsing by keeping a temp object Instead of aways manipulating the last element of std::vector<Appcast> (with necessary checks), simply keep the current work-in-progress item as a variable and push it to the vector after parsing is done. --- src/appcast.cpp | 72 +++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index 0352b774..555c79d2 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -149,6 +149,7 @@ struct ContextData // call when entering <item> element void reset_for_new_item() { + current = Appcast(); legacy_dsa_signature.clear(); } @@ -161,11 +162,14 @@ struct ContextData // is inside <sparkle:version> or <sparkle:shortVersionString> etc. node? int in_version, in_shortversion, in_dsasignature, in_min_os_version; + // currently parsed item + Appcast current; + // signature present as <sparkle:dsaSignature>, not enclosure attribute std::string legacy_dsa_signature; // parsed <item>s - std::vector<Appcast> items; + std::vector<Appcast> all_items; }; @@ -180,9 +184,6 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) else if ( ctxt.in_channel && strcmp(name, NODE_ITEM) == 0 ) { ctxt.in_item++; - Appcast item; - ctxt.items.push_back(item); - ctxt.reset_for_new_item(); } else if ( ctxt.in_item ) @@ -221,34 +222,30 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) } else if (strcmp(name, NODE_ENCLOSURE) == 0) { - if (!ctxt.items.empty()) + Appcast& item = ctxt.current; + for (int i = 0; attrs[i]; i += 2) { - Appcast& item = ctxt.items.back(); - for (int i = 0; attrs[i]; i += 2) - { - const char* name = attrs[i]; - const char* value = attrs[i + 1]; - - if (strcmp(name, ATTR_URL) == 0) - item.enclosure.DownloadURL = value; - else if (strcmp(name, ATTR_DSASIGNATURE) == 0) - item.enclosure.DsaSignature = value; - else if (strcmp(name, ATTR_OS) == 0) - item.enclosure.OS = value; - else if (strcmp(name, ATTR_ARGUMENTS) == 0) - item.enclosure.InstallerArguments = value; - // legacy syntax where version info was on enclosure, not item: - else if (strcmp(name, ATTR_VERSION) == 0) - item.Version = value; - else if (strcmp(name, ATTR_SHORTVERSION) == 0) - item.ShortVersionString = value; - } + const char* name = attrs[i]; + const char* value = attrs[i + 1]; + + if (strcmp(name, ATTR_URL) == 0) + item.enclosure.DownloadURL = value; + else if (strcmp(name, ATTR_DSASIGNATURE) == 0) + item.enclosure.DsaSignature = value; + else if (strcmp(name, ATTR_OS) == 0) + item.enclosure.OS = value; + else if (strcmp(name, ATTR_ARGUMENTS) == 0) + item.enclosure.InstallerArguments = value; + // legacy syntax where version info was on enclosure, not item: + else if (strcmp(name, ATTR_VERSION) == 0) + item.Version = value; + else if (strcmp(name, ATTR_SHORTVERSION) == 0) + item.ShortVersionString = value; } } else if (strcmp(name, NODE_CRITICAL_UPDATE) == 0) { - if (!ctxt.items.empty()) - ctxt.items.back().CriticalUpdate = true; + ctxt.current.CriticalUpdate = true; } } } @@ -296,11 +293,13 @@ void XMLCALL OnEndElement(void *data, const char *name) { ctxt.in_item--; - Appcast& item = ctxt.items.back(); + Appcast& item = ctxt.current; - if (!ctxt.legacyDsaSignature.empty() && item.enclosure.DsaSignature.empty()) + if (!ctxt.legacy_dsa_signature.empty() && item.enclosure.DsaSignature.empty()) item.enclosure.DsaSignature = ctxt.legacy_dsa_signature; + ctxt.all_items.push_back(item); + if (is_suitable_windows_item(item)) XML_StopParser(ctxt.parser, XML_TRUE); } @@ -318,10 +317,7 @@ void XMLCALL OnEndElement(void *data, const char *name) void XMLCALL OnText(void *data, const char *s, int len) { ContextData& ctxt = *static_cast<ContextData*>(data); - if (ctxt.items.empty()) - return; - - Appcast& item = ctxt.items.back(); + Appcast& item = ctxt.current; if (ctxt.in_relnotes) { @@ -390,7 +386,7 @@ Appcast Appcast::Load(const std::string& xml) XML_ParserFree(p); - if (ctxt.items.empty()) + if (ctxt.all_items.empty()) return Appcast(); // invalid /* @@ -398,13 +394,13 @@ Appcast Appcast::Load(const std::string& xml) * or "windows-x64"/"windows-arm64"/"windows-x86" based on this modules bitness and meets the minimum * os version, if set. If none, use the first item that meets the minimum os version, if set. */ - std::vector<Appcast>::iterator it = std::find_if(ctxt.items.begin(), ctxt.items.end(), is_suitable_windows_item); - if (it != ctxt.items.end()) + std::vector<Appcast>::iterator it = std::find_if(ctxt.all_items.begin(), ctxt.all_items.end(), is_suitable_windows_item); + if (it != ctxt.all_items.end()) return *it; else { - it = std::find_if(ctxt.items.begin(), ctxt.items.end(), is_windows_version_acceptable); - if (it != ctxt.items.end()) + it = std::find_if(ctxt.all_items.begin(), ctxt.all_items.end(), is_windows_version_acceptable); + if (it != ctxt.all_items.end()) return *it; else return Appcast(); // There are no items that meet the set minimum os version From 26867f0a1c0aa79fb660ac22be93eead39e31767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= <vaclav@slavik.io> Date: Tue, 19 Nov 2024 15:58:57 +0100 Subject: [PATCH 5/6] Rework enclosure matching to be readable & correct Rework the sparkle:os matching code to be more readable and, above all, correct even in presence of multiple enclosures. To do this, this commit does the following: - Parse all enclosures into a vector, then filter out unsuitable enclosures - From the remaining ones, pick the most specific (e.g. "windows-x64" is used over "windows" or os-less, if both are present) - Skip over appcast items that have only unsuitable enclosures - Renames helper functions to have more clear names. Fixes #276, fixes #281. --- src/appcast.cpp | 138 ++++++++++++++++++++++++++++++------------------ 1 file changed, 87 insertions(+), 51 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index 555c79d2..b5ed404f 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -27,8 +27,9 @@ #include "error.h" #include <expat.h> -#include <vector> #include <algorithm> +#include <iterator> +#include <vector> #include <windows.h> namespace winsparkle @@ -37,13 +38,23 @@ namespace winsparkle namespace { -// Misc helper functions: +// OS identification strings: + +#define OS_MARKER_GENERIC "windows" +#ifdef _WIN64 + #if defined(__AARCH64EL__) || defined(_M_ARM64) + #define OS_MARKER_ARCH "windows-arm64" + #else + #define OS_MARKER_ARCH "windows-x64" + #endif // defined(__AARCH64EL__) || defined(_M_ARM64) +#else + #define OS_MARKER_ARCH "windows-x86" +#endif // _WIN64 -#define OS_MARKER "windows" -#define OS_MARKER_LEN 7 +// Misc helper functions: -bool is_windows_version_acceptable(const Appcast &item) +bool is_compatible_with_windows_version(const Appcast &item) { if (item.MinOSVersion.empty()) return true; @@ -64,34 +75,45 @@ bool is_windows_version_acceptable(const Appcast &item) } -/** - * Returns true if item os is exactly "windows" - * or if item is "windows-arm64" on 64bit ARM - * or if item is "windows-x64" on 64bit Intel/AMD - * or if item is "windows-x86" on 32bit - * and is above minimum version - */ -bool is_suitable_windows_item(const Appcast& item) +// Checks if the item is compatible with the running OS, that is: +// - is not for a different OS +// - is either for current architecture or is architecture-independent +// +// E.g. returns true if item is +// - "windows-arm64" on 64bit ARM +// - "windows-x64" on 64bit Intel/AMD +// - "windows-x86" on 32bit +// - "windows" on any Windows arch +// - empty string on any OS +inline bool is_compatible_with_os_arch(const Appcast::Enclosure& enclosure) { - if (!is_windows_version_acceptable(item)) - return false; + return enclosure.OS.empty() || enclosure.OS == OS_MARKER_GENERIC || enclosure.OS == OS_MARKER_ARCH; +} - if (item.enclosure.OS == OS_MARKER) - return true; - if (item.enclosure.OS.compare(0, OS_MARKER_LEN, OS_MARKER) != 0) - return false; +// Finds the best enclosure for the current OS and architecture. +Appcast::Enclosure find_best_enclosure_for_os_arch(const std::vector<Appcast::Enclosure>& enclosures) +{ + // filter out incompatible enclosures: + std::vector<Appcast::Enclosure> compatible; + std::copy_if(enclosures.begin(), enclosures.end(), std::back_inserter(compatible), is_compatible_with_os_arch); + if (compatible.empty()) + return Appcast::Enclosure(); + + // is there arch-specific enclosure? + auto it = std::find_if(compatible.begin(), compatible.end(), + [](const Appcast::Enclosure& e) { return e.OS == OS_MARKER_ARCH; }); + if (it != compatible.end()) + return *it; + + // is there an enclosure explicitly marked as for windows? + it = std::find_if(compatible.begin(), compatible.end(), + [](const Appcast::Enclosure& e) { return e.OS == OS_MARKER_GENERIC; }); + if (it != compatible.end()) + return *it; - // Check suffix for matching bitness -#ifdef _WIN64 -#if defined(__AARCH64EL__) || defined(_M_ARM64) - return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-arm64") == 0; -#else - return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-x64") == 0; -#endif // defined(__AARCH64EL__) || defined(_M_ARM64) -#else - return item.enclosure.OS.compare(OS_MARKER_LEN, std::string::npos, "-x86") == 0; -#endif // _WIN64 + // because all enclosures are compatible, any one will do, e.g. the first one: + return compatible.front(); } @@ -150,6 +172,7 @@ struct ContextData void reset_for_new_item() { current = Appcast(); + enclosures.clear(); legacy_dsa_signature.clear(); } @@ -165,6 +188,9 @@ struct ContextData // currently parsed item Appcast current; + // enclosures encountered so far + std::vector<Appcast::Enclosure> enclosures; + // signature present as <sparkle:dsaSignature>, not enclosure attribute std::string legacy_dsa_signature; @@ -223,25 +249,33 @@ void XMLCALL OnStartElement(void *data, const char *name, const char **attrs) else if (strcmp(name, NODE_ENCLOSURE) == 0) { Appcast& item = ctxt.current; + Appcast::Enclosure enclosure; + for (int i = 0; attrs[i]; i += 2) { const char* name = attrs[i]; const char* value = attrs[i + 1]; if (strcmp(name, ATTR_URL) == 0) - item.enclosure.DownloadURL = value; + enclosure.DownloadURL = value; else if (strcmp(name, ATTR_DSASIGNATURE) == 0) - item.enclosure.DsaSignature = value; + enclosure.DsaSignature = value; else if (strcmp(name, ATTR_OS) == 0) - item.enclosure.OS = value; + enclosure.OS = value; else if (strcmp(name, ATTR_ARGUMENTS) == 0) - item.enclosure.InstallerArguments = value; + enclosure.InstallerArguments = value; + // legacy syntax where version info was on enclosure, not item: else if (strcmp(name, ATTR_VERSION) == 0) item.Version = value; else if (strcmp(name, ATTR_SHORTVERSION) == 0) item.ShortVersionString = value; } + + // note: we intentionally include incompatible enclosures in the list so that + // we can check for that case later in OnEndElement() and skip the entire <item> + if (enclosure.IsValid()) + ctxt.enclosures.push_back(enclosure); } else if (strcmp(name, NODE_CRITICAL_UPDATE) == 0) { @@ -298,10 +332,25 @@ void XMLCALL OnEndElement(void *data, const char *name) if (!ctxt.legacy_dsa_signature.empty() && item.enclosure.DsaSignature.empty()) item.enclosure.DsaSignature = ctxt.legacy_dsa_signature; - ctxt.all_items.push_back(item); + if (!ctxt.enclosures.empty()) + { + item.enclosure = find_best_enclosure_for_os_arch(ctxt.enclosures); + if (!item.enclosure.IsValid()) + { + // There are enclosures (e.g. weblink is not used), but all enclosures are + // incompatible. This means the <item> is not meant for this OS and should be + // skipped (as Sparkle does; there may be another <item> for us). + return; + } + } + + if (is_compatible_with_windows_version(item)) + { + ctxt.all_items.push_back(item); - if (is_suitable_windows_item(item)) + // FIXME: this is premature, we should sort appcast items by date and pick the newest (as Sparkle does) XML_StopParser(ctxt.parser, XML_TRUE); + } } } else if (strcmp(name, NODE_CHANNEL) == 0 ) @@ -389,22 +438,9 @@ Appcast Appcast::Load(const std::string& xml) if (ctxt.all_items.empty()) return Appcast(); // invalid - /* - * Search for first <item> which specifies with the attribute sparkle:os set to "windows" - * or "windows-x64"/"windows-arm64"/"windows-x86" based on this modules bitness and meets the minimum - * os version, if set. If none, use the first item that meets the minimum os version, if set. - */ - std::vector<Appcast>::iterator it = std::find_if(ctxt.all_items.begin(), ctxt.all_items.end(), is_suitable_windows_item); - if (it != ctxt.all_items.end()) - return *it; - else - { - it = std::find_if(ctxt.all_items.begin(), ctxt.all_items.end(), is_windows_version_acceptable); - if (it != ctxt.all_items.end()) - return *it; - else - return Appcast(); // There are no items that meet the set minimum os version - } + // the items were already filtered to only include those compatible with the current OS + arch + // and meeting minimum OS version requirements, so we can just return the first one + return ctxt.all_items.front(); } } // namespace winsparkle From c36170d39cd1db23bc7cf0ad09cb69139f398cf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Va=CC=81clav=20Slavi=CC=81k?= <vaclav@slavik.io> Date: Tue, 19 Nov 2024 16:00:11 +0100 Subject: [PATCH 6/6] Check appcast item correctness Check if an item is valid, i.e. has either a usable enclosure or a link to open in the browser. --- src/appcast.cpp | 2 +- src/appcast.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/appcast.cpp b/src/appcast.cpp index b5ed404f..efc3588c 100644 --- a/src/appcast.cpp +++ b/src/appcast.cpp @@ -344,7 +344,7 @@ void XMLCALL OnEndElement(void *data, const char *name) } } - if (is_compatible_with_windows_version(item)) + if (item.IsValid() && is_compatible_with_windows_version(item)) { ctxt.all_items.push_back(item); diff --git a/src/appcast.h b/src/appcast.h index 9d4227d9..017ac9fc 100644 --- a/src/appcast.h +++ b/src/appcast.h @@ -93,7 +93,7 @@ struct Appcast static Appcast Load(const std::string& xml); /// Returns true if the struct constains valid data. - bool IsValid() const { return !Version.empty(); } + bool IsValid() const { return !Version.empty() && (HasDownload() || !WebBrowserURL.empty()); } /// If true, then download and install the update ourselves. /// If false, launch a web browser to WebBrowserURL.