From 8e1b2fca59d71d2e6ab404238d7b38975adb3665 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 3 Oct 2022 17:45:01 +0200
Subject: [PATCH] fix(npm): panic on invalid package name (#16123)

---
 cli/graph_util.rs                                    | 9 ++++-----
 cli/npm/resolution.rs                                | 3 ++-
 cli/tests/integration/npm_tests.rs                   | 7 +++++++
 cli/tests/testdata/npm/invalid_package_name/main.js  | 1 +
 cli/tests/testdata/npm/invalid_package_name/main.out | 2 ++
 5 files changed, 16 insertions(+), 6 deletions(-)
 create mode 100644 cli/tests/testdata/npm/invalid_package_name/main.js
 create mode 100644 cli/tests/testdata/npm/invalid_package_name/main.out

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index f73fd55593..dca7a16b76 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -81,11 +81,10 @@ impl GraphData {
         continue;
       }
       if specifier.scheme() == "npm" {
-        // the loader enforces npm specifiers are valid, so it's ok to unwrap here
-        let reference =
-          NpmPackageReference::from_specifier(&specifier).unwrap();
-        self.npm_packages.insert(reference.req);
-        continue;
+        if let Ok(reference) = NpmPackageReference::from_specifier(&specifier) {
+          self.npm_packages.insert(reference.req);
+          continue;
+        }
       }
       if let Some(found) = graph.redirects.get(&specifier) {
         let module_entry = ModuleEntry::Redirect(found.clone());
diff --git a/cli/npm/resolution.rs b/cli/npm/resolution.rs
index 15fffdf04e..d56cc87bc8 100644
--- a/cli/npm/resolution.rs
+++ b/cli/npm/resolution.rs
@@ -8,6 +8,7 @@ use std::collections::VecDeque;
 use deno_ast::ModuleSpecifier;
 use deno_core::anyhow::bail;
 use deno_core::anyhow::Context;
+use deno_core::error::generic_error;
 use deno_core::error::AnyError;
 use deno_core::futures;
 use deno_core::parking_lot::RwLock;
@@ -52,7 +53,7 @@ impl NpmPackageReference {
     let parts = specifier.split('/').collect::<Vec<_>>();
     let name_part_len = if specifier.starts_with('@') { 2 } else { 1 };
     if parts.len() < name_part_len {
-      bail!("Not a valid package: {}", specifier);
+      return Err(generic_error(format!("Not a valid package: {}", specifier)));
     }
     let name_parts = &parts[0..name_part_len];
     let last_name_part = &name_parts[name_part_len - 1];
diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs
index d111bd76e6..32ec126569 100644
--- a/cli/tests/integration/npm_tests.rs
+++ b/cli/tests/integration/npm_tests.rs
@@ -181,6 +181,13 @@ itest!(nonexistent_file {
   exit_code: 1,
 });
 
+itest!(invalid_package_name {
+  args: "run --unstable -A --quiet npm/invalid_package_name/main.js",
+  output: "npm/invalid_package_name/main.out",
+  envs: env_vars(),
+  exit_code: 1,
+});
+
 itest!(require_json {
   args: "run --unstable -A --quiet npm/require_json/main.js",
   output: "npm/require_json/main.out",
diff --git a/cli/tests/testdata/npm/invalid_package_name/main.js b/cli/tests/testdata/npm/invalid_package_name/main.js
new file mode 100644
index 0000000000..1e37830541
--- /dev/null
+++ b/cli/tests/testdata/npm/invalid_package_name/main.js
@@ -0,0 +1 @@
+import * as foo from "npm:@foo";
diff --git a/cli/tests/testdata/npm/invalid_package_name/main.out b/cli/tests/testdata/npm/invalid_package_name/main.out
new file mode 100644
index 0000000000..7d2b3754df
--- /dev/null
+++ b/cli/tests/testdata/npm/invalid_package_name/main.out
@@ -0,0 +1,2 @@
+error: Not a valid package: @foo
+    at [WILDCARD]/invalid_package_name/main.js:1:22