fix: do not panic running invalid file specifier (#25530)

Co-authored-by: Bedis Nbiba <bedisnbiba@gmail.com>

3 files changed, 88 insertions, 71 deletions

diff --git a/runtime/fs_util.rs b/runtime/fs_util.rs
index fe9736038..15ebafdf6 100644
--- a/runtime/fs_util.rs
+++ b/runtime/fs_util.rs
@@ -1,13 +1,13 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::Context;
-use deno_core::error::uri_error;
 use deno_core::error::AnyError;
-pub use deno_core::normalize_path;
 use std::path::Path;
 use std::path::PathBuf;
 
+pub use deno_core::normalize_path;
+pub use deno_permissions::specifier_to_file_path;
+
 #[inline]
 pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, AnyError> {
   if path.is_absolute() {
@@ -20,49 +20,6 @@ pub fn resolve_from_cwd(path: &Path) -> Result<PathBuf, AnyError> {
   }
 }
 
-/// Attempts to convert a specifier to a file path. By default, uses the Url
-/// crate's `to_file_path()` method, but falls back to try and resolve unix-style
-/// paths on Windows.
-pub fn specifier_to_file_path(
-  specifier: &ModuleSpecifier,
-) -> Result<PathBuf, AnyError> {
-  let result = if specifier.scheme() != "file" {
-    Err(())
-  } else if cfg!(windows) {
-    match specifier.to_file_path() {
-      Ok(path) => Ok(path),
-      Err(()) => {
-        // This might be a unix-style path which is used in the tests even on Windows.
-        // Attempt to see if we can convert it to a `PathBuf`. This code should be removed
-        // once/if https://github.com/servo/rust-url/issues/730 is implemented.
-        if specifier.scheme() == "file"
-          && specifier.host().is_none()
-          && specifier.port().is_none()
-          && specifier.path_segments().is_some()
-        {
-          let path_str = specifier.path();
-          match String::from_utf8(
-            percent_encoding::percent_decode(path_str.as_bytes()).collect(),
-          ) {
-            Ok(path_str) => Ok(PathBuf::from(path_str)),
-            Err(_) => Err(()),
-          }
-        } else {
-          Err(())
-        }
-      }
-    }
-  } else {
-    specifier.to_file_path()
-  };
-  match result {
-    Ok(path) => Ok(path),
-    Err(()) => Err(uri_error(format!(
-      "Invalid file path.\n  Specifier: {specifier}"
-    ))),
-  }
-}
-
 #[cfg(test)]
 mod tests {
   use super::*;
@@ -114,22 +71,4 @@ mod tests {
     let absolute_expected = cwd.join(expected);
     assert_eq!(resolve_from_cwd(expected).unwrap(), absolute_expected);
   }
-
-  #[test]
-  fn test_specifier_to_file_path() {
-    run_success_test("file:///", "/");
-    run_success_test("file:///test", "/test");
-    run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt");
-    run_success_test(
-      "file:///dir/test%20test/test.txt",
-      "/dir/test test/test.txt",
-    );
-
-    fn run_success_test(specifier: &str, expected_path: &str) {
-      let result =
-        specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap())
-          .unwrap();
-      assert_eq!(result, PathBuf::from(expected_path));
-    }
-  }
 }
diff --git a/runtime/permissions/Cargo.toml b/runtime/permissions/Cargo.toml
index b43574896..9821b6148 100644
--- a/runtime/permissions/Cargo.toml
+++ b/runtime/permissions/Cargo.toml
@@ -20,6 +20,7 @@ fqdn = "0.3.4"
 libc.workspace = true
 log.workspace = true
 once_cell.workspace = true
+percent-encoding = { version = "2.3.1", features = [] }
 serde.workspace = true
 which.workspace = true
 
diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs
index ad84b9fc3..b5c870a07 100644
--- a/runtime/permissions/lib.rs
+++ b/runtime/permissions/lib.rs
@@ -1932,7 +1932,7 @@ impl Permissions {
     specifier: &ModuleSpecifier,
   ) -> Result<(), AnyError> {
     match specifier.scheme() {
-      "file" => match specifier.to_file_path() {
+      "file" => match specifier_to_file_path(specifier) {
         Ok(path) => self.read.check(
           &PathQueryDescriptor {
             requested: path.to_string_lossy().into_owned(),
@@ -1952,6 +1952,52 @@ impl Permissions {
   }
 }
 
+/// Attempts to convert a specifier to a file path. By default, uses the Url
+/// crate's `to_file_path()` method, but falls back to try and resolve unix-style
+/// paths on Windows.
+pub fn specifier_to_file_path(
+  specifier: &ModuleSpecifier,
+) -> Result<PathBuf, AnyError> {
+  let result = if specifier.scheme() != "file" {
+    Err(())
+  } else if cfg!(windows) {
+    if specifier.host().is_some() {
+      Err(())
+    } else {
+      match specifier.to_file_path() {
+        Ok(path) => Ok(path),
+        Err(()) => {
+          // This might be a unix-style path which is used in the tests even on Windows.
+          // Attempt to see if we can convert it to a `PathBuf`. This code should be removed
+          // once/if https://github.com/servo/rust-url/issues/730 is implemented.
+          if specifier.scheme() == "file"
+            && specifier.port().is_none()
+            && specifier.path_segments().is_some()
+          {
+            let path_str = specifier.path();
+            match String::from_utf8(
+              percent_encoding::percent_decode(path_str.as_bytes()).collect(),
+            ) {
+              Ok(path_str) => Ok(PathBuf::from(path_str)),
+              Err(_) => Err(()),
+            }
+          } else {
+            Err(())
+          }
+        }
+      }
+    }
+  } else {
+    specifier.to_file_path()
+  };
+  match result {
+    Ok(path) => Ok(path),
+    Err(()) => Err(uri_error(format!(
+      "Invalid file path.\n  Specifier: {specifier}"
+    ))),
+  }
+}
+
 /// Wrapper struct for `Permissions` that can be shared across threads.
 ///
 /// We need a way to have internal mutability for permissions as they might get
@@ -3228,12 +3274,9 @@ mod tests {
 
     let mut test_cases = vec![];
 
-    if cfg!(target_os = "windows") {
-      test_cases.push("file://");
-      test_cases.push("file:///");
-    } else {
-      test_cases.push("file://remotehost/");
-    }
+    test_cases.push("file://dir");
+    test_cases.push("file://asdf/");
+    test_cases.push("file://remotehost/");
 
     for url in test_cases {
       assert!(perms
@@ -4222,4 +4265,38 @@ mod tests {
       );
     }
   }
+
+  #[test]
+  fn test_specifier_to_file_path() {
+    run_success_test("file:///", "/");
+    run_success_test("file:///test", "/test");
+    run_success_test("file:///dir/test/test.txt", "/dir/test/test.txt");
+    run_success_test(
+      "file:///dir/test%20test/test.txt",
+      "/dir/test test/test.txt",
+    );
+
+    assert_no_panic_specifier_to_file_path("file:/");
+    assert_no_panic_specifier_to_file_path("file://");
+    assert_no_panic_specifier_to_file_path("file://asdf/");
+    assert_no_panic_specifier_to_file_path("file://asdf/66666/a.ts");
+
+    fn run_success_test(specifier: &str, expected_path: &str) {
+      let result =
+        specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap())
+          .unwrap();
+      assert_eq!(result, PathBuf::from(expected_path));
+    }
+    fn assert_no_panic_specifier_to_file_path(specifier: &str) {
+      let result =
+        specifier_to_file_path(&ModuleSpecifier::parse(specifier).unwrap());
+      match result {
+        Ok(_) => (),
+        Err(err) => assert_eq!(
+          err.to_string(),
+          format!("Invalid file path.\n  Specifier: {specifier}")
+        ),
+      }
+    }
+  }
 }