Rewrite `contains_symlink`

2 files changed, 41 insertions, 29 deletions

diff --git a/src/file_upload.rs b/src/file_upload.rs
index b9974aa..919ac2c 100644
--- a/src/file_upload.rs
+++ b/src/file_upload.rs
@@ -110,10 +110,16 @@ async fn handle_multipart(
         })?;
 
         // Ensure there are no illegal symlinks
-        if !allow_symlinks && contains_symlink(&absolute_path) {
-            return Err(ContextualError::InsufficientPermissionsError(
-                user_given_path.display().to_string(),
-            ));
+        if !allow_symlinks {
+            match contains_symlink(&absolute_path) {
+                Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                    err.to_string(),
+                ))?,
+                Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                    "{user_given_path:?} traverses through a symlink"
+                )))?,
+                Ok(false) => (),
+            }
         }
 
         std::fs::create_dir_all(&absolute_path).map_err(|e| {
@@ -135,10 +141,16 @@ async fn handle_multipart(
     })?;
 
     // Ensure there are no illegal symlinks in the file upload path
-    if !allow_symlinks && contains_symlink(&path) {
-        return Err(ContextualError::InsufficientPermissionsError(
-            filename.to_string(),
-        ));
+    if !allow_symlinks {
+        match contains_symlink(&path) {
+            Err(err) => Err(ContextualError::InsufficientPermissionsError(
+                err.to_string(),
+            ))?,
+            Ok(true) => Err(ContextualError::InsufficientPermissionsError(format!(
+                "{path:?} traverses through a symlink"
+            )))?,
+            Ok(false) => (),
+        }
     }
 
     save_file(field, path.join(filename_path), overwrite_files).await
diff --git a/src/file_utils.rs b/src/file_utils.rs
index ba976de..114a08f 100644
--- a/src/file_utils.rs
+++ b/src/file_utils.rs
@@ -1,4 +1,7 @@
-use std::path::{Component, Path, PathBuf};
+use std::{
+    io,
+    path::{Component, Path, PathBuf},
+};
 
 /// Guarantee that the path is relative and cannot traverse back to parent directories
 /// and optionally prevent traversing hidden directories.
@@ -29,26 +32,23 @@ pub fn sanitize_path(path: impl AsRef<Path>, traverse_hidden: bool) -> Option<Pa
     Some(buf)
 }
 
-/// Returns if a path goes through a symbolic link
-pub fn contains_symlink(path: impl AsRef<Path>) -> bool {
-    let mut joined_path = PathBuf::new();
-    for path_slice in path.as_ref().iter() {
-        joined_path = joined_path.join(path_slice);
-        if !joined_path.exists() {
-            // On Windows, `\\?\` won't exist even though it's the root
-            // So, we can't just return here
-            // But we don't need to check if it's a symlink since it won't be
-            continue;
-        }
-        if joined_path
-            .symlink_metadata()
-            .map(|m| m.file_type().is_symlink())
-            .unwrap_or(false)
-        {
-            return true;
-        }
-    }
-    false
+/// Checks if any segment of the path is a symlink.
+///
+/// This function fails if [`std::fs::symlink_metadata`] fails, which usually
+/// means user has no permission to access the path.
+pub fn contains_symlink(path: impl AsRef<Path>) -> io::Result<bool> {
+    let contains_symlink = path
+        .as_ref()
+        .ancestors()
+        // On Windows, `\\?\` won't exist even though it's the root, but there's no need to check it
+        // So we filter it out
+        .filter(|p| p.exists())
+        .map(|p| p.symlink_metadata())
+        .collect::<Result<Vec<_>, _>>()?
+        .into_iter()
+        .any(|p| p.file_type().is_symlink());
+
+    Ok(contains_symlink)
 }
 
 #[cfg(test)]