qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
28.47k stars 3.99k forks source link

Avoid reapplying Mark-of-the-Web when it already exists #21806

Closed Chocobo1 closed 1 week ago

Chocobo1 commented 1 week ago

Also use scope guards to handle resources.

Related #21788.

glassez commented 1 week ago

Cannot check macOS part. As of Windows one... what if zone id stream exists but has unexpected data? Will it be overwritten?

Chocobo1 commented 1 week ago

As of Windows one... what if zone id stream exists but has unexpected data? Will it be overwritten?

Yes. The code will try matching some values (leniently) and will overwrite it if it diverges too much.

glassez commented 1 week ago

As of Windows one... what if zone id stream exists but has unexpected data? Will it be overwritten?

Yes. The code will try matching some values (leniently) and will overwrite it if it diverges too much.

I would just like to make sure that after a previously performed read, it still overwrites the file, and does not add new data to the end of existing ones.

Chocobo1 commented 1 week ago

I would just like to make sure that after a previously performed read, it still overwrites the file, and does not add new data to the end of existing ones.

Thanks for the hint! It is addressed.

glassez commented 1 week ago

Closes #21788.

On a closer look, this PR isn't able to fix such problems in the general case. It seems that the problem is not only that qBittorrent overwrites existing Mark-of-the-Web data, but that it writes it even if the files were not actually downloaded by qBittorrent, but already existed before adding the torrent, i.e. they were downloaded earlier or obtained from some other source (a suitable example may probably be even the case when the user created some torrent and added it to the qBittorrent).

Chocobo1 commented 1 week ago

It seems that the problem is not only that qBittorrent overwrites existing Mark-of-the-Web data, but that it writes it even if the files were not actually downloaded by qBittorrent, but already existed before adding the torrent, i.e. they were downloaded earlier or obtained from some other source (a suitable example may probably be even the case when the user created some torrent and added it to the qBittorrent).

No, that is why this PR adds the lenient checks to verify whether the MOTW is valid. Only when it is not, then qbt overwrites it.

Chocobo1 commented 1 week ago

a suitable example may probably be even the case when the user created some torrent and added it to the qBittorrent

Do you know how to differentiate between an existing file on disk and a newly downloaded/created file when adding a torrent? That may be the key to avoid setting MOTW on seeding files.

glassez commented 1 week ago

It seems that the problem is not only that qBittorrent overwrites existing Mark-of-the-Web data, but that it writes it even if the files were not actually downloaded by qBittorrent, but already existed before adding the torrent, i.e. they were downloaded earlier or obtained from some other source (a suitable example may probably be even the case when the user created some torrent and added it to the qBittorrent).

No, that is why this PR adds the lenient checks to verify whether the MOTW is valid. Only when it is not, then qbt overwrites it.

It is also confirmed by #21788 author that qBittorrent write MOTW in existing files that didn't contain it before. https://github.com/qbittorrent/qBittorrent/issues/21788#issuecomment-2471844634

glassez commented 1 week ago

Do you know how to differentiate between an existing file on disk and a newly downloaded/created file when adding a torrent? That may be the key to avoid setting MOTW on seeding files.

I think it is possible to detect such a case, although it may not be so trivial. I could do it, but I don't want v5.0.2 to wait for this problem to be solved.

Chocobo1 commented 1 week ago

I think it is possible to detect such a case, although it may not be so trivial. I could do it, but I don't want v5.0.2 to wait for this problem to be solved.

OK, great! Your fix can be backported later.

I would still like to see this PR in v5.0.2. This PR actually does its job: avoiding reapplying MOTW when it already exists.

Some random thoughts:

glassez commented 1 week ago

I would still like to see this PR in v5.0.2. This PR actually does its job: avoiding reapplying MOTW when it already exists.

Sure.

Chocobo1 commented 1 week ago

The added_on date might be useful. qbt can check whether the file is created before or after the added_on date. If it is before, skip MOTW. Otherwise, check/apply MOTW.

It seems this simple check would be enough... I'll submit a PR and we can discuss it there.

glassez commented 1 week ago

Added to backport PR.