core: clearly define when module lookup is path-based vs URL-based

The rules are now as follows:

* In `import` statements, as mandated by the WHATWG specification,
  the import specifier is always treated as a URL.
  If it is a relative URL, it must start with either / or ./ or ../

* A script name passed to deno as a command line argument may be either
  an absolute URL or a local path.
  - If the name starts with a valid URI scheme followed by a colon, e.g.
    'http:', 'https:', 'file:', 'foo+bar:', it always interpreted as a
    URL (even if Deno doesn't support the indicated protocol).
  - Otherwise, the script name is interpreted as a local path. The local
    path may be relative, and operating system semantics determine how
    it is resolved. Prefixing a relative path with ./ is not required.

cli/compiler.rs

@@ -249,7 +249,7 @@ mod tests {
     tokio_util::init(|| {
       let specifier = "./tests/002_hello.ts";
       use deno::ModuleSpecifier;
-      let module_name = ModuleSpecifier::resolve_root(specifier)
+      let module_name = ModuleSpecifier::resolve_url_or_path(specifier)
         .unwrap()
         .to_string();
 
@@ -296,7 +296,7 @@ mod tests {
   fn test_bundle_async() {
     let specifier = "./tests/002_hello.ts";
     use deno::ModuleSpecifier;
-    let module_name = ModuleSpecifier::resolve_root(specifier)
+    let module_name = ModuleSpecifier::resolve_url_or_path(specifier)
       .unwrap()
       .to_string();
 

cli/deno_error.rs

@@ -110,7 +110,8 @@ impl DenoError {
         use ModuleResolutionError::*;
         match err {
           InvalidUrl(err) | InvalidBaseUrl(err) => Self::url_error_kind(err),
-          ImportPathPrefixMissing => ErrorKind::ImportPathPrefixMissing,
+          InvalidPath => ErrorKind::InvalidPath,
+          ImportPrefixMissing => ErrorKind::ImportPrefixMissing,
         }
       }
       Repr::Diagnostic(ref _err) => ErrorKind::Diagnostic,

cli/flags.rs

@@ -630,7 +630,9 @@ pub enum DenoSubcommand {
 
 fn get_default_bundle_filename(source_file: &str) -> String {
   use deno::ModuleSpecifier;
-  let url = ModuleSpecifier::resolve_root(source_file).unwrap().to_url();
+  let url = ModuleSpecifier::resolve_url_or_path(source_file)
+    .unwrap()
+    .to_url();
   let path_segments = url.path_segments().unwrap();
   let last = path_segments.last().unwrap();
   String::from(last.trim_end_matches(".ts").trim_end_matches(".js"))

cli/import_map.rs

@@ -178,9 +178,7 @@ impl ImportMap {
         continue;
       }
 
-      let normalized_address = ModuleSpecifier::resolve(&url_string, ".")
-        .expect("Address should be valid module specifier");
-      normalized_addresses.push(normalized_address);
+      normalized_addresses.push(url.into());
     }
 
     normalized_addresses

cli/msg.fbs

@@ -102,6 +102,7 @@ enum ErrorKind: byte {
   WouldBlock,
   InvalidInput,
   InvalidData,
+  InvalidPath,
   TimedOut,
   Interrupted,
   WriteZero,
@@ -141,7 +142,7 @@ enum ErrorKind: byte {
   NoAsyncSupport,
   NoSyncSupport,
   ImportMapError,
-  ImportPathPrefixMissing,
+  ImportPrefixMissing,
 
   // other kinds
   Diagnostic,

cli/ops.rs

@@ -2053,7 +2053,7 @@ fn op_create_worker(
   err_check(worker.execute("denoMain()"));
   err_check(worker.execute("workerMain()"));
 
-  let module_specifier = ModuleSpecifier::resolve_root(specifier)?;
+  let module_specifier = ModuleSpecifier::resolve_url_or_path(specifier)?;
 
   let op =
     worker

cli/state.rs

@@ -170,7 +170,8 @@ impl Loader for ThreadSafeState {
       }
     }
 
-    ModuleSpecifier::resolve(specifier, referrer).map_err(DenoError::from)
+    ModuleSpecifier::resolve_import(specifier, referrer)
+      .map_err(DenoError::from)
   }
 
   /// Given an absolute url, load its source code.
@@ -252,7 +253,7 @@ impl ThreadSafeState {
       None
     } else {
       let root_specifier = argv_rest[1].clone();
-      match ModuleSpecifier::resolve_root(&root_specifier) {
+      match ModuleSpecifier::resolve_url_or_path(&root_specifier) {
         Ok(specifier) => Some(specifier),
         Err(e) => {
           // TODO: handle unresolvable specifier

cli/worker.rs

@@ -141,7 +141,7 @@ mod tests {
   #[test]
   fn execute_mod_esm_imports_a() {
     let module_specifier =
-      ModuleSpecifier::resolve_root("tests/esm_imports_a.js").unwrap();
+      ModuleSpecifier::resolve_url_or_path("tests/esm_imports_a.js").unwrap();
     let argv = vec![String::from("./deno"), module_specifier.to_string()];
     let state = ThreadSafeState::new(
       flags::DenoFlags::default(),
@@ -169,7 +169,7 @@ mod tests {
   #[test]
   fn execute_mod_circular() {
     let module_specifier =
-      ModuleSpecifier::resolve_root("tests/circular1.js").unwrap();
+      ModuleSpecifier::resolve_url_or_path("tests/circular1.js").unwrap();
     let argv = vec![String::from("./deno"), module_specifier.to_string()];
     let state = ThreadSafeState::new(
       flags::DenoFlags::default(),
@@ -197,7 +197,7 @@ mod tests {
   #[test]
   fn execute_006_url_imports() {
     let module_specifier =
-      ModuleSpecifier::resolve_root("tests/006_url_imports.ts").unwrap();
+      ModuleSpecifier::resolve_url_or_path("tests/006_url_imports.ts").unwrap();
     let argv = vec![String::from("deno"), module_specifier.to_string()];
     let mut flags = flags::DenoFlags::default();
     flags.reload = true;
@@ -327,7 +327,7 @@ mod tests {
       // "foo" is not a valid module specifier so this should return an error.
       let mut worker = create_test_worker();
       let module_specifier =
-        ModuleSpecifier::resolve_root("does-not-exist").unwrap();
+        ModuleSpecifier::resolve_url_or_path("does-not-exist").unwrap();
       let result = worker.execute_mod_async(&module_specifier, false).wait();
       assert!(result.is_err());
     })
@@ -340,7 +340,7 @@ mod tests {
       // tests).
       let mut worker = create_test_worker();
       let module_specifier =
-        ModuleSpecifier::resolve_root("./tests/002_hello.ts").unwrap();
+        ModuleSpecifier::resolve_url_or_path("./tests/002_hello.ts").unwrap();
       let result = worker.execute_mod_async(&module_specifier, false).wait();
       assert!(result.is_ok());
     })

core/module_specifier.rs

@@ -1,3 +1,4 @@
+use std::env::current_dir;
 use std::error::Error;
 use std::fmt;
 use url::ParseError;
@@ -8,7 +9,8 @@ use url::Url;
 pub enum ModuleResolutionError {
   InvalidUrl(ParseError),
   InvalidBaseUrl(ParseError),
-  ImportPathPrefixMissing,
+  InvalidPath,
+  ImportPrefixMissing,
 }
 use ModuleResolutionError::*;
 
@@ -28,7 +30,8 @@ impl fmt::Display for ModuleResolutionError {
       InvalidBaseUrl(ref err) => {
         write!(f, "invalid base URL for relative import: {}", err)
       }
-      ImportPathPrefixMissing => {
+      InvalidPath => write!(f, "invalid module path"),
+      ImportPrefixMissing => {
         write!(f, "relative import path not prefixed with / or ./ or ../")
       }
     }
@@ -46,7 +49,7 @@ impl ModuleSpecifier {
 
   /// Resolves module using this algorithm:
   /// https://html.spec.whatwg.org/multipage/webappapis.html#resolve-a-module-specifier
-  pub fn resolve(
+  pub fn resolve_import(
     specifier: &str,
     base: &str,
   ) -> Result<ModuleSpecifier, ModuleResolutionError> {
@@ -64,7 +67,7 @@ impl ModuleSpecifier {
           || specifier.starts_with("./")
           || specifier.starts_with("../")) =>
       {
-        Err(ImportPathPrefixMissing)?
+        Err(ImportPrefixMissing)?
       }
 
       // 3. Return the result of applying the URL parser to specifier with base
@@ -76,7 +79,7 @@ impl ModuleSpecifier {
 
       // If parsing the specifier as a URL failed for a different reason than
       // it being relative, always return the original error. We don't want to
-      // return `ImportPathPrefixMissing` or `InvalidBaseUrl` if the real
+      // return `ImportPrefixMissing` or `InvalidBaseUrl` if the real
       // problem lies somewhere else.
       Err(err) => Err(InvalidUrl(err))?,
     };
@@ -84,25 +87,70 @@ impl ModuleSpecifier {
     Ok(ModuleSpecifier(url))
   }
 
-  /// Takes a string representing a path or URL to a module, but of the type
-  /// passed through the command-line interface for the main module. This is
-  /// slightly different than specifiers used in import statements: "foo.js" for
-  /// example is allowed here, whereas in import statements a leading "./" is
-  /// required ("./foo.js"). This function is aware of the current working
-  /// directory and returns an absolute URL.
-  pub fn resolve_root(
-    root_specifier: &str,
+  /// Converts a string representing an absulute URL into a ModuleSpecifier.
+  pub fn resolve_url(
+    url_str: &str,
   ) -> Result<ModuleSpecifier, ModuleResolutionError> {
-    let url = match Url::parse(root_specifier) {
-      Ok(url) => url,
-      Err(..) => {
-        let cwd = std::env::current_dir().unwrap();
-        let base = Url::from_directory_path(cwd).unwrap();
-        base.join(&root_specifier).map_err(InvalidUrl)?
-      }
-    };
+    Url::parse(url_str)
+      .map(ModuleSpecifier)
+      .map_err(ModuleResolutionError::InvalidUrl)
+  }
 
-    Ok(ModuleSpecifier(url))
+  /// Takes a string representing either an absolute URL or a file path,
+  /// as it may be passed to deno as a command line argument.
+  /// The string is interpreted as a URL if it starts with a valid URI scheme,
+  /// e.g. 'http:' or 'file:' or 'git+ssh:'. If not, it's interpreted as a
+  /// file path; if it is a relative path it's resolved relative to the current
+  /// working directory.
+  pub fn resolve_url_or_path(
+    specifier: &str,
+  ) -> Result<ModuleSpecifier, ModuleResolutionError> {
+    if Self::specifier_has_uri_scheme(specifier) {
+      Self::resolve_url(specifier)
+    } else {
+      Self::resolve_path(specifier)
+    }
+  }
+
+  /// Converts a string representing a relative or absolute path into a
+  /// ModuleSpecifier. A relative path is considered relative to the current
+  /// working directory.
+  fn resolve_path(
+    path_str: &str,
+  ) -> Result<ModuleSpecifier, ModuleResolutionError> {
+    let path = current_dir().unwrap().join(path_str);
+    Url::from_file_path(path)
+      .map(ModuleSpecifier)
+      .map_err(|()| ModuleResolutionError::InvalidPath)
+  }
+
+  /// Returns true if the input string starts with a sequence of characters
+  /// that could be a valid URI scheme, like 'https:', 'git+ssh:' or 'data:'.
+  ///
+  /// According to RFC 3986 (https://tools.ietf.org/html/rfc3986#section-3.1),
+  /// a valid scheme has the following format:
+  ///   scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+  ///
+  /// We additionally require the scheme to be at least 2 characters long,
+  /// because otherwise a windows path like c:/foo would be treated as a URL,
+  /// while no schemes with a one-letter name actually exist.
+  fn specifier_has_uri_scheme(specifier: &str) -> bool {
+    let mut chars = specifier.chars();
+    let mut len = 0usize;
+    // THe first character must be a letter.
+    match chars.next() {
+      Some(c) if c.is_ascii_alphabetic() => len += 1,
+      _ => return false,
+    }
+    // Second and following characters must be either a letter, number,
+    // plus sign, minus sign, or dot.
+    loop {
+      match chars.next() {
+        Some(c) if c.is_ascii_alphanumeric() || "+-.".contains(c) => len += 1,
+        Some(':') if len >= 2 => return true,
+        _ => return false,
+      }
+    }
   }
 }
 
@@ -123,3 +171,211 @@ impl PartialEq<String> for ModuleSpecifier {
     &self.to_string() == other
   }
 }
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+
+  #[test]
+  fn test_resolve_import() {
+    let tests = vec![
+      (
+        "/dev/core/tests/005_more_imports.ts",
+        "file:///home/yeti",
+        "file:///dev/core/tests/005_more_imports.ts",
+      ),
+      (
+        "//zombo.com/1999.ts",
+        "https://cherry.dev/its/a/thing",
+        "https://zombo.com/1999.ts",
+      ),
+      (
+        "http://deno.land/this/url/is/valid",
+        "base is clearly not a valid url",
+        "http://deno.land/this/url/is/valid",
+      ),
+      (
+        "//server/some/dir/file",
+        "file:///home/yeti/deno",
+        "file://server/some/dir/file",
+      ),
+      // This test is disabled because the url crate does not follow the spec,
+      // dropping the server part from the final result.
+      // (
+      //   "/another/path/at/the/same/server",
+      //   "file://server/some/dir/file",
+      //   "file://server/another/path/at/the/same/server",
+      // ),
+    ];
+
+    for (specifier, base, expected_url) in tests {
+      let url = ModuleSpecifier::resolve_import(specifier, base)
+        .unwrap()
+        .to_string();
+      assert_eq!(url, expected_url);
+    }
+  }
+
+  #[test]
+  fn test_resolve_import_error() {
+    use url::ParseError::*;
+    use ModuleResolutionError::*;
+
+    let tests = vec![
+      (
+        "https://eggplant:b/c",
+        "http://deno.land/core/tests/006_url_imports.ts",
+        InvalidUrl(InvalidPort),
+      ),
+      (
+        "https://eggplant@/c",
+        "http://deno.land/core/tests/006_url_imports.ts",
+        InvalidUrl(EmptyHost),
+      ),
+      (
+        "./foo.ts",
+        "/relative/base/url",
+        InvalidBaseUrl(RelativeUrlWithoutBase),
+      ),
+    ];
+
+    for (specifier, base, expected_err) in tests {
+      let err = ModuleSpecifier::resolve_import(specifier, base).unwrap_err();
+      assert_eq!(err, expected_err);
+    }
+  }
+
+  #[test]
+  fn test_resolve_url_or_path() {
+    // Absolute URL.
+    let mut tests: Vec<(&str, String)> = vec![];
+
+    // The local path tests assume that the cwd is the deno repo root.
+    let cwd = current_dir().unwrap();
+    let cwd_str = cwd.to_str().unwrap();
+
+    if cfg!(target_os = "windows") {
+      // Absolute local path.
+      let expected_url = "file:///C:/deno/tests/006_url_imports.ts";
+      tests.extend(vec![
+        (
+          r"C:/deno/tests/006_url_imports.ts",
+          expected_url.to_string(),
+        ),
+        (
+          r"C:\deno\tests\006_url_imports.ts",
+          expected_url.to_string(),
+        ),
+        (
+          r"\\?\C:\deno\tests\006_url_imports.ts",
+          expected_url.to_string(),
+        ),
+        // Not supported: `Url::from_file_path()` fails.
+        // (r"\\.\C:\deno\tests\006_url_imports.ts", expected_url.to_string()),
+        // Not supported: `Url::from_file_path()` performs the wrong conversion.
+        // (r"//./C:/deno/tests/006_url_imports.ts", expected_url.to_string()),
+      ]);
+
+      // Rooted local path without drive letter.
+      let expected_url = format!(
+        "file:///{}:/deno/tests/006_url_imports.ts",
+        cwd_str.get(..1).unwrap(),
+      );
+      tests.extend(vec![
+        (r"/deno/tests/006_url_imports.ts", expected_url.to_string()),
+        (r"\deno\tests\006_url_imports.ts", expected_url.to_string()),
+      ]);
+
+      // Relative local path.
+      let expected_url = format!(
+        "file:///{}/tests/006_url_imports.ts",
+        cwd_str.replace("\\", "/")
+      );
+      tests.extend(vec![
+        (r"tests/006_url_imports.ts", expected_url.to_string()),
+        (r"tests\006_url_imports.ts", expected_url.to_string()),
+        (r"./tests/006_url_imports.ts", expected_url.to_string()),
+        (r".\tests\006_url_imports.ts", expected_url.to_string()),
+      ]);
+
+      // UNC network path.
+      let expected_url = "file://server/share/deno/cool";
+      tests.extend(vec![
+        (r"\\server\share\deno\cool", expected_url.to_string()),
+        (r"\\server/share/deno/cool", expected_url.to_string()),
+        // Not supported: `Url::from_file_path()` performs the wrong conversion.
+        // (r"//server/share/deno/cool", expected_url.to_string()),
+      ]);
+    } else {
+      // Absolute local path.
+      let expected_url = "file:///deno/tests/006_url_imports.ts";
+      tests.extend(vec![
+        ("/deno/tests/006_url_imports.ts", expected_url.to_string()),
+        ("//deno/tests/006_url_imports.ts", expected_url.to_string()),
+      ]);
+
+      // Relative local path.
+      let expected_url = format!("file://{}/tests/006_url_imports.ts", cwd_str);
+      tests.extend(vec![
+        ("tests/006_url_imports.ts", expected_url.to_string()),
+        ("./tests/006_url_imports.ts", expected_url.to_string()),
+      ]);
+    }
+
+    for (specifier, expected_url) in tests {
+      let url = ModuleSpecifier::resolve_url_or_path(specifier)
+        .unwrap()
+        .to_string();
+      assert_eq!(url, expected_url);
+    }
+  }
+
+  #[test]
+  fn test_resolve_url_or_path_error() {
+    use url::ParseError::*;
+    use ModuleResolutionError::*;
+
+    let mut tests = vec![
+      ("https://eggplant:b/c", InvalidUrl(InvalidPort)),
+      ("https://:8080/a/b/c", InvalidUrl(EmptyHost)),
+    ];
+    if cfg!(target_os = "windows") {
+      tests.push((r"\\.\c:/stuff/deno/script.ts", InvalidPath));
+    }
+
+    for (specifier, expected_err) in tests {
+      let err = ModuleSpecifier::resolve_url_or_path(specifier).unwrap_err();
+      assert_eq!(err, expected_err);
+    }
+  }
+
+  #[test]
+  fn test_specifier_has_uri_scheme() {
+    let tests = vec![
+      ("http://foo.bar/etc", true),
+      ("HTTP://foo.bar/etc", true),
+      ("http:ftp:", true),
+      ("http:", true),
+      ("hTtP:", true),
+      ("ftp:", true),
+      ("mailto:spam@please.me", true),
+      ("git+ssh://git@github.com/denoland/deno", true),
+      ("blob:https://whatwg.org/mumbojumbo", true),
+      ("abc.123+DEF-ghi:", true),
+      ("abc.123+def-ghi:@", true),
+      ("", false),
+      (":not", false),
+      ("http", false),
+      ("c:dir", false),
+      ("X:", false),
+      ("./http://not", false),
+      ("1abc://kinda/but/no", false),
+      ("schluẞ://no/more", false),
+    ];
+
+    for (specifier, expected) in tests {
+      let result = ModuleSpecifier::specifier_has_uri_scheme(specifier);
+      assert_eq!(result, expected);
+    }
+  }
+}

core/modules.rs

@@ -643,11 +643,11 @@ mod tests {
 
       eprintln!(">> RESOLVING, S: {}, R: {}", specifier, referrer);
 
-      let output_specifier = match ModuleSpecifier::resolve(specifier, referrer)
-      {
-        Ok(specifier) => specifier,
-        Err(_e) => return Err(MockError::ResolveErr),
-      };
+      let output_specifier =
+        match ModuleSpecifier::resolve_import(specifier, referrer) {
+          Ok(specifier) => specifier,
+          Err(..) => return Err(MockError::ResolveErr),
+        };
 
       if mock_source_code(&output_specifier.to_string()).is_some() {
         Ok(output_specifier)

tests/error_011_bad_module_specifier.ts.out

@@ -1,4 +1,4 @@
-[WILDCARD]error: Uncaught ImportPathPrefixMissing: relative import path not prefixed with / or ./ or ../
+[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path not prefixed with / or ./ or ../
 [WILDCARD] js/errors.ts:[WILDCARD]
     at DenoError (js/errors.ts:[WILDCARD])
     at maybeError (js/errors.ts:[WILDCARD])

tests/error_012_bad_dynamic_import_specifier.ts.out

@@ -1,4 +1,4 @@
-[WILDCARD]error: Uncaught ImportPathPrefixMissing: relative import path not prefixed with / or ./ or ../
+[WILDCARD]error: Uncaught ImportPrefixMissing: relative import path not prefixed with / or ./ or ../
 [WILDCARD] js/errors.ts:[WILDCARD]
     at DenoError (js/errors.ts:[WILDCARD])
     at maybeError (js/errors.ts:[WILDCARD])