desktop: Fix filesystem access dialog

This patch fixes the following two issues:

1. On Windows the allowed path check was not always working, due to
   Windows paths being Windows paths, now paths are
   canonicalized before comparison.

2. On some systems the dialog was not rendered when the main SWF wasn't
   loaded, and the dialog was not able to allow the main SWF.
   Now the dialog allows the SWF instantly when constructed.

desktop/src/backends.rs

@@ -6,4 +6,5 @@ mod ui;
 pub use external_interface::DesktopExternalInterfaceProvider;
 pub use fscommand::DesktopFSCommandProvider;
 pub use navigator::DesktopNavigatorInterface;
+pub use navigator::PathAllowList;
 pub use ui::DesktopUiBackend;

desktop/src/backends/navigator.rs

@@ -20,15 +20,66 @@ use crate::gui::dialogs::network_access_dialog::{
 use crate::gui::DialogDescriptor;
 use crate::util::open_url;
 
+// TODO Make this more generic, maybe a manager?
+#[derive(Clone)]
+pub struct PathAllowList {
+    allowed_path_prefixes: Arc<Mutex<Vec<PathBuf>>>,
+}
+
+impl PathAllowList {
+    fn new(movie_path: Option<PathBuf>) -> Self {
+        let mut allowed_path_prefixes = Vec::new();
+        if let Some(movie_path) = movie_path {
+            if let Some(parent) = movie_path.parent() {
+                // TODO Be smarter here, we do not necessarily want to allow
+                //   access to the SWF's dir, but we also do not want to present
+                //   the dialog to the user too often.
+                allowed_path_prefixes.push(parent.to_path_buf());
+            }
+            allowed_path_prefixes.push(movie_path);
+        }
+        Self {
+            allowed_path_prefixes: Arc::new(Mutex::new(allowed_path_prefixes)),
+        }
+    }
+
+    /// Checks whether the user already allowed access to the file.
+    pub fn is_path_allowed(&self, path: &Path) -> bool {
+        for path_prefix in self
+            .allowed_path_prefixes
+            .lock()
+            .expect("Non-poisoned lock")
+            .as_slice()
+        {
+            let Ok(path_prefix) = path_prefix.canonicalize() else {
+                continue;
+            };
+            let Ok(path) = path.canonicalize() else {
+                continue;
+            };
+            if path.starts_with(path_prefix) {
+                return true;
+            }
+        }
+        false
+    }
+
+    pub fn add_allowed_path_prefix(&self, path_prefix: PathBuf) {
+        self.allowed_path_prefixes
+            .lock()
+            .expect("Non-poisoned lock")
+            .push(path_prefix);
+    }
+}
+
 #[derive(Clone)]
 pub struct DesktopNavigatorInterface {
     // Arc + Mutex due to macOS
     event_loop: Arc<Mutex<EventLoopProxy<RuffleEvent>>>,
 
     filesystem_access_mode: FilesystemAccessMode,
 
-    // TODO Make this more generic, maybe a manager?
-    allowed_paths: Arc<Mutex<Vec<PathBuf>>>,
+    allow_list: PathAllowList,
 }
 
 impl DesktopNavigatorInterface {
@@ -37,19 +88,9 @@ impl DesktopNavigatorInterface {
         movie_path: Option<PathBuf>,
         filesystem_access_mode: FilesystemAccessMode,
     ) -> Self {
-        let mut allowed_paths = Vec::new();
-        if let Some(movie_path) = movie_path {
-            if let Some(parent) = movie_path.parent() {
-                // TODO Be smarter here, we do not necessarily want to allow
-                //   access to the SWF's dir, but we also do not want to present
-                //   the dialog to the user too often.
-                allowed_paths.push(parent.to_path_buf());
-            }
-            allowed_paths.push(movie_path);
-        }
         Self {
             event_loop: Arc::new(Mutex::new(event_loop)),
-            allowed_paths: Arc::new(Mutex::new(allowed_paths)),
+            allow_list: PathAllowList::new(movie_path),
             filesystem_access_mode,
         }
     }
@@ -63,7 +104,7 @@ impl DesktopNavigatorInterface {
             .send_event(RuffleEvent::OpenDialog(DialogDescriptor::FilesystemAccess(
                 FilesystemAccessDialogConfiguration::new(
                     notifier,
-                    self.allowed_paths.clone(),
+                    self.allow_list.clone(),
                     path.to_path_buf(),
                 ),
             )));
@@ -89,10 +130,14 @@ impl NavigatorInterface for DesktopNavigatorInterface {
     async fn open_file(&self, path: &Path) -> io::Result<File> {
         let path = &path.canonicalize()?;
 
-        let allow = match self.filesystem_access_mode {
-            FilesystemAccessMode::Allow => true,
-            FilesystemAccessMode::Deny => false,
-            FilesystemAccessMode::Ask => self.ask_for_filesystem_access(path).await,
+        let allow = if self.allow_list.is_path_allowed(path) {
+            true
+        } else {
+            match self.filesystem_access_mode {
+                FilesystemAccessMode::Allow => true,
+                FilesystemAccessMode::Deny => false,
+                FilesystemAccessMode::Ask => self.ask_for_filesystem_access(path).await,
+            }
         };
 
         if !allow {

desktop/src/gui/dialogs/filesystem_access_dialog.rs

@@ -1,9 +1,6 @@
-use crate::gui::text;
+use crate::{backends::PathAllowList, gui::text};
 use egui::{Align2, ComboBox, Ui, Window};
-use std::{
-    path::PathBuf,
-    sync::{Arc, Mutex},
-};
+use std::path::PathBuf;
 use tokio::sync::oneshot::Sender;
 use unic_langid::LanguageIdentifier;
 
@@ -17,9 +14,7 @@ pub struct FilesystemAccessDialogConfiguration {
     notifier: Option<Sender<FilesystemAccessDialogResult>>,
 
     /// Collection of already allowed paths that can be updated.
-    ///
-    /// TODO Make this more generic, maybe a manager?
-    allowed_paths: Arc<Mutex<Vec<PathBuf>>>,
+    allow_list: PathAllowList,
 
     /// Path of the file to access.
     path: PathBuf,
@@ -28,12 +23,12 @@ pub struct FilesystemAccessDialogConfiguration {
 impl FilesystemAccessDialogConfiguration {
     pub fn new(
         notifier: Sender<FilesystemAccessDialogResult>,
-        allowed_paths: Arc<Mutex<Vec<PathBuf>>>,
+        allow_list: PathAllowList,
         path: PathBuf,
     ) -> Self {
         Self {
             notifier: Some(notifier),
-            allowed_paths,
+            allow_list,
             path,
         }
     }
@@ -63,20 +58,28 @@ impl Drop for FilesystemAccessDialog {
 
 impl FilesystemAccessDialog {
     pub fn new(config: FilesystemAccessDialogConfiguration) -> Self {
-        let allowed = Self::is_path_allowed(&config);
+        // This check needs to be done as late as possible, because we want the
+        // user's decision to apply to every future dialog,
+        // not only those requested after the decision.
+        let allowed = config.allow_list.is_path_allowed(&config.path);
+
         let selectable_paths = Self::get_selectable_paths(&config);
         let selected_path = selectable_paths
             .first()
             .cloned()
             .unwrap_or_else(PathBuf::new);
 
-        Self {
+        let mut dialog = Self {
             config,
             allowed,
             remember_access: false,
             selected_path,
             selectable_paths,
+        };
+        if allowed {
+            dialog.respond(FilesystemAccessDialogResult::Allow);
         }
+        dialog
     }
 
     /// Returns paths that will be shown in the dropdown menu.
@@ -102,25 +105,6 @@ impl FilesystemAccessDialog {
         selectable_paths
     }
 
-    /// Checks whether the user already allowed access to the file.
-    ///
-    /// This check needs to be done as late as possible, because we want the
-    /// user's decision to apply to every future dialog,
-    /// not only those requested after the decision.
-    fn is_path_allowed(config: &FilesystemAccessDialogConfiguration) -> bool {
-        for path_prefix in config
-            .allowed_paths
-            .lock()
-            .expect("Non-poisoned lock")
-            .as_slice()
-        {
-            if config.path.starts_with(path_prefix) {
-                return true;
-            }
-        }
-        false
-    }
-
     fn respond(&mut self, result: FilesystemAccessDialogResult) {
         if let Some(notifier) = std::mem::take(&mut self.config.notifier) {
             let _ = notifier.send(result);
@@ -129,7 +113,6 @@ impl FilesystemAccessDialog {
 
     pub fn show(&mut self, locale: &LanguageIdentifier, egui_ctx: &egui::Context) -> bool {
         if self.allowed {
-            self.respond(FilesystemAccessDialogResult::Allow);
             return false;
         }
 
@@ -175,10 +158,8 @@ impl FilesystemAccessDialog {
                 if ui.button(primary_text).clicked() {
                     if self.remember_access {
                         self.config
-                            .allowed_paths
-                            .lock()
-                            .expect("Non-poisoned lock")
-                            .push(self.selected_path.clone());
+                            .allow_list
+                            .add_allowed_path_prefix(self.selected_path.clone());
                     }
                     self.respond(FilesystemAccessDialogResult::Allow);
                     should_close = true;