search

freedomofpress / dangerzone

Take potentially dangerous PDFs, office documents, or images and convert them to safe PDFs
https://dangerzone.rocks/
GNU Affero General Public License v3.0
3.36k stars 153 forks source link

Dangerzone is listed as a PDF viewer to open safe documents with on Linux #790

Closed naglis closed 1 month ago

naglis commented 2 months ago

Tested on Arch Linux/Ubuntu 22.04 when Dangerzone 0.6.0 is installed as a package. This only applies to Linux systems.

Screenshot of Dangerzone 0.6.0 running on Ubuntu 22.04 showing Dangerzone listed as an application to open safe PDF files after conversion

On Linux, Dangerzone allows to select a PDF viewer to use to open safe PDF documents after conversion. The list of available PDF viewers is populated by parsing the .desktop entries of all available applications and returning ones that can open files with application/pdf mimetype. Currently, Dangerzone itself is included in this list (if Dangerzone is installed as a package system-wide), although in the code there is a specific check to exclude Dangerzone from the list, however the check is done on the lowercase name dangerzone and the application name from the .desktop entry (which is Dangerzone) and so the condition never matches.

I believe it makes sense to exclude Dangerzone from the list of PDF viewers as Dangerzone cannot display PDFs (at least, not currently, see also #424) and repeatedly converting a safe document does not make much sense.

The trivial fix would be to use Dangerzone as the application name in the check, however, IIUC pyxdg DesktopEntry.getName() returns a localized name and if Dangerzone is translated in the future (#132), the Name key in the Dangerzone's .desktop file might differ in other languages. To avoid that, DesktopEntry.get('Name', locale=False) could be used to ignore localizations. Another option could be to use a different way to identify the Dangerzone application, e.g. the name of the .desktop file (press.freedom.dangerzone).

apyrgio commented 2 months ago

That's a very nice dig! I'd go with the trivial fix for now and not future-proofing the application, since a potential localization effort will radically rewrite the strings in any case. I'll include a better check in a PR I'll send soon.

deeplow commented 2 months ago

Damn. Great example of how fixing a bug created another one.

naglis commented 2 months ago

Damn. Great example of how fixing a bug created another one.

IIUC by refactoring _find_pdf_viewers a bit (e.g. to allow controlling the search paths) we could write tests for it to check Dangerzone is excluded/some example application is found.

deeplow commented 2 months ago

Honestly I think @apyrgio approach is enough (https://github.com/freedomofpress/dangerzone/commit/5f2b6320badf8ba80bf3889961590388c05ba2a1) for now. At least until localization comes along as he states.