refactor(core): simplify and document push_pattern (#11065)

Author: amrbashir

crates/tauri/src/scope/fs.rs

@@ -77,74 +77,85 @@ fn push_pattern<P: AsRef<Path>, F: Fn(&str) -> Result<Pattern, glob::PatternErro
   pattern: P,
   f: F,
 ) -> crate::Result<()> {
-  let mut path: PathBuf = dunce::simplified(pattern.as_ref()).components().collect();
-
-  if cfg!(windows) {
-    // Canonicalize disk-relative paths before inserting into the list
-    use std::path::{Component, Prefix};
-    let mut components = path.components();
-    if let Some(Component::Prefix(prefix)) = components.next() {
-      if matches!(prefix.kind(), Prefix::Disk(_) | Prefix::VerbatimDisk(_))
-        && !matches!(components.next(), Some(Component::RootDir))
-      {
-        path = dunce::simplified(&path.canonicalize()?).to_path_buf();
+  // Reconstruct pattern path components with appropraite separator
+  // so `some\path/to/dir/**\*` would be `some/path/to/dir/**/*` on Unix
+  // and  `some\path\to\dir\**\*` on Windows.
+  let path: PathBuf = pattern.as_ref().components().collect();
+
+  // Add pattern as is to be matched with paths as is
+  let path_str = path.to_string_lossy();
+  list.insert(f(&path_str)?);
+
+  // On Windows, if path starts with a Prefix, try to strip it if possible
+  // so `\\?\C:\\SomeDir` would result in a scope of:
+  //   - `\\?\C:\\SomeDir`
+  //   - `C:\\SomeDir`
+  #[cfg(windows)]
+  {
+    use std::path::Component;
+
+    if matches!(path.components().next(), Some(Component::Prefix(_))) {
+      let simplified = dunce::simplified(&path);
+      let simplified_str = simplified.to_string_lossy();
+      if simplified_str != path_str {
+        list.insert(f(&simplified_str)?);
       }
     }
   }
 
-  list.insert(f(&path.to_string_lossy())?);
-
-  let mut checked_path = None;
-
-  // attempt to canonicalize parents in case we have a path like `/data/user/0/appid/**`
-  // where `**` obviously does not exist but we need to canonicalize the parent
-  //
-  // example: given the `/data/user/0/appid/assets/*` path,
-  // it's a glob pattern so it won't exist (canonicalize() fails);
-  //
-  // the second iteration needs to check `/data/user/0/appid/assets` and save the `*` component to append later.
-  //
-  // if it also does not exist, a third iteration is required to check `/data/user/0/appid`
-  // with `assets/*` as the cached value (`checked_path` variable)
-  // on Android that gets canonicalized to `/data/data/appid` so the final value will be `/data/data/appid/assets/*`
-  // which is the value we want to check when we execute the `is_allowed` function
-  let canonicalized = loop {
+  // Add canonicalized version of the pattern or canonicalized version of its parents
+  // so `/data/user/0/appid/assets/*` would be canonicalized to `/data/data/appid/assets/*`
+  // and can then be matched against any of them.
+  if let Some(p) = canonicalize_parent(path) {
+    list.insert(f(&p.to_string_lossy())?);
+  }
+
+  Ok(())
+}
+
+/// Attempt to canonicalize path or its parents in case we have a path like `/data/user/0/appid/**`
+/// where `**` obviously does not exist but we need to canonicalize the parent.
+///
+/// example: given the `/data/user/0/appid/assets/*` path,
+/// it's a glob pattern so it won't exist (std::fs::canonicalize() fails);
+///
+/// the second iteration needs to check `/data/user/0/appid/assets` and save the `*` component to append later.
+///
+/// if it also does not exist, a third iteration is required to check `/data/user/0/appid`
+/// with `assets/*` as the cached value (`checked_path` variable)
+/// on Android that gets canonicalized to `/data/data/appid` so the final value will be `/data/data/appid/assets/*`
+/// which is the value we want to check when we execute the `Scope::is_allowed` function
+fn canonicalize_parent(mut path: PathBuf) -> Option<PathBuf> {
+  let mut failed_components = None;
+
+  loop {
     if let Ok(path) = path.canonicalize() {
-      break Some(if let Some(p) = checked_path {
+      break Some(if let Some(p) = failed_components {
         path.join(p)
       } else {
         path
       });
     }
 
-    // get the last component of the path as an OsStr
-    let last = path.iter().next_back().map(PathBuf::from);
-    if let Some(mut p) = last {
-      // remove the last component of the path
-      // so the next iteration checks its parent
+    // grap the last component of the path
+    if let Some(mut last) = path.iter().next_back().map(PathBuf::from) {
+      // remove the last component of the path so the next iteration checks its parent
+      // if there is no more parent components, we failed to canonicalize
       if !path.pop() {
         break None;
       }
+
       // append the already checked path to the last component
-      if let Some(checked_path) = &checked_path {
-        p.push(checked_path);
+      // to construct `<last>/<checked_path>` and saved it for next iteration
+      if let Some(failed_components) = &failed_components {
+        last.push(failed_components);
       }
-      // replace the checked path with the current value
-      checked_path.replace(p);
+      failed_components.replace(last);
     } else {
       break None;
     }
-  };
-
-  if let Some(p) = canonicalized {
-    list.insert(f(&p.to_string_lossy())?);
-  } else if cfg!(windows) && !path.to_string_lossy().starts_with("\\\\") {
-    list.insert(f(&format!("\\\\?\\{}", path.display()))?);
   }
-
-  Ok(())
 }
-
 impl Scope {
   /// Creates a new scope from a [`FsScope`] configuration.
   pub fn new<R: crate::Runtime, M: crate::Manager<R>>(