sparkle:os seems to be ignored

[Foaly]

Hello there!

Consider the following: I have a windows application in version 0.9.0 and the following appcast.xml

<?xml version="1.0" encoding="UTF-8"?>
<rss version="2.0" xmlns:sparkle="http://www.andymatuschak.org/xml-namespaces/sparkle" xmlns:dc="http://purl.org/dc/elements/1.1/">
  <channel>
    <title>UpdaterExample Release</title>
    <description>The latest stable release of UpdaterExample.</description>
    <language>en</language>

    <item>
      <title>Version 1.0.0</title>
      <pubDate>Tue, 21 May 2024 14:31:00 +0000</pubDate>
      <enclosure sparkle:os="macos"
                 sparkle:version="1.0.0"
                 url="https://www.link.to.a.dmg"
                 length="2620494"
                 type="application/octet-stream"/>
    </item>
  </channel>
</rss>

If I run the update command WinSparkle will download and run the dmg from the macos enclosure url. I would expect is to say no new version available. This seems like a bug to me. Please let me know if I am missing anything.

Thank you for your work and this great library.

[Foaly]

I did read around in the code a bit and might have found something.

When loading the appcast file the following code is executed: https://github.com/vslavik/winsparkle/blob/master/src/appcast.cpp#L371-L386

First all available items are check with is_suitable_windows_item (https://github.com/vslavik/winsparkle/blob/master/src/appcast.cpp#L203)

This should fail, as the os string should be macos and not windows.

After this check fails all items are checked again using is_windows_version_acceptable (https://github.com/vslavik/winsparkle/blob/master/src/appcast.cpp#L94)

This function for some reason returns true if not minimum version was set.

I think this might be the reason why the item above passes eventhoug it should clearly. I tried adding a <sparkle:minimumSystemVersion> tag to the item and then the problem is gone. I still think this is a bug though and I don't really understand the logic why if no suitable os item is found the version is checked again. Shouldn't the logic be reversed? Only check the version once the os is confirmed to be correct?

I might be missing some points though, as I only had a quick look at the code, which I am not familiar with and I did no real testing!

[vslavik]

I'm not surprised you can't make much sense of the logic; neither can I :(

[Foaly]

So what can we do about it? I think maybe the logic has to be rethought.

[vslavik]

So what can we do about it?

Well, certainly not nagging with this sort of non-contributing comment. You can submit a PR (that would be useful) or you can show some patience.

[Foaly]

Hello and sorry for the misunderstanding. My comment was not meant to be nagging. I was just questioning for the next steps. I think the logic has to be rethought. Do you agree? To my understanding the steps right now are as follows:

1. All items are checked using is_suitable_windows_item 1.1 First the MinOSVersion version is checked, if it does not match the item is discarded 1.2 item.Os is compared in equality to OS_MARKER ("windows") 1.3 the substring from item.Os position 0 to position OS_MARKER_LEN is compared to OS_MARKER, if they are not equal the item is discarded 1.4 item.Os suffixes are compared to check for the platform bitness
2. If a suitable windows item is found it is returned
3. If no suitable item is found, all items are checked again using is_windows_version_acceptable 3.1 if MinOSVersion is empty true is returned
4. the first matching version or empty version is returned
5. if no versions match an empty Appcast() is returned

I'm not very deep into the specifications of the appcast format, but to my logic the following should make sense:

1. Use the first item where the platform string matches
2. If a MinOSVersion is not empty, check if it matches
3. If it does not match use the next item where the platform string matches and repeat with 2
4. If neither platform nor MinOSVersion is defined for any item, use the first item

Am I missing anything?

[i-n-g-o]

Hello.

I ran into a similar situation. This PR should fix this issue: https://github.com/vslavik/winsparkle/pull/279