phase1geo / Minder

Mind-mapping application for Elementary OS
GNU General Public License v3.0
1.02k stars 67 forks source link

Flatpak app doesn't error when saving in wrong location #158

Open Xiol opened 4 years ago

Xiol commented 4 years ago

Hello,

I installed the Flatpak version on Fedora 32, but I've lost a map I spent hours working on because the application doesn't error when attempting to save in the wrong location.

The Flatpak has permissions to xdg-documents, so effectively ~/Documents, however the save file dialogue shows my entire $HOME directory. I chose a location outside of ~/Documents to save my map and it appeared to save file - the windows titlebar changed and repeatedly saving did not cause any errors.

Upon reopening the application after the weekend I could no longer locate the saved map because it simply didn't exist on disk. The application opened with the map name in the titlebar, but an empty blank map (no root node).

Saving files in ~/Documents works fine, as expected, but I would expect the application to fail loudly when saving in the wrong location. Alternatively, only locations where saving is allowed should be shown in the save dialogue.

Thanks

phase1geo commented 4 years ago

@bilelmoussaoui I was wondering if you have any insights about how to properly handle this issue. Do I need to check to see if the directory exists after the user selects a file from the save file dialog? Or is there something else that I need to do. The application currently doesn't expect invalid pathnames to come from the open/save file dialogs.

bilelmoussaoui commented 4 years ago

@phase1geo The directory do exists, it's just inside the sandbox. I would suggest using a NativeFileChooser which would allow you to remove the documents permissions while having access to whatever files selects with the file dialog.

phase1geo commented 4 years ago

The application is using FileChooserNative in this case. Is there a method I need to call to keep this issue from happening?

bilelmoussaoui commented 4 years ago

Do you mind pointing me to the code when you handle save/export?

phase1geo commented 4 years ago

src/MainWindow.cc line 926 (save_file method)

bilelmoussaoui commented 4 years ago

In https://github.com/phase1geo/Minder/blob/master/src/MainWindow.vala#L935-L945 you're using the file path to save the content. The file paths are not a "correct" thing if your application is sandboxed. The documents portal, gives you permanent access to a file URI that's not named the same way as outside of it, that file URI is stored on the permissions stores. Those URI's are the files you have access to and are mounted when you run the application so you read/edit/save them by their URI.

So you just need to stop using the file path whenever you save/open a file and you can remove the xdg-documents permission :)

phase1geo commented 4 years ago

So, I should be calling get_uri() instead of get_filename() on line 936 then?

bilelmoussaoui commented 4 years ago

Yes, and you can't add a suffix to a file as it wouldn't be same file you had access to any more

bilelmoussaoui commented 4 years ago

If you need a stream later on to save your Document on it, I would suggest just using Gio.File directly while you're at it. You get the file with dialog.get_file()

phase1geo commented 4 years ago

I am not able to replicate the original issue (other than I can also navigate to folders outside of the Documents directory). Since I don't personally support the Flatpak (this was created by others, not myself), I will review a pull request for this issue, but I'm not planning on making any changes to support this behavior.

MarSik commented 3 years ago

This is related to #191