fix: Don't shell out to `unzip` in deno upgrade/compile (#24926)

Use the `zip` crate instead

Fixes #23988.

5 files changed, 136 insertions, 81 deletions

diff --git a/cli/Cargo.toml b/cli/Cargo.toml
index 6d9e1b7b2..168f2929c 100644
--- a/cli/Cargo.toml
+++ b/cli/Cargo.toml
@@ -157,6 +157,7 @@ typed-arena = "=2.0.1"
 uuid = { workspace = true, features = ["serde"] }
 which.workspace = true
 zeromq.workspace = true
+zip = { version = "2.1.6", default-features = false, features = ["deflate-flate2"] }
 zstd.workspace = true
 
 [target.'cfg(windows)'.dependencies]
diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs
index ed38e164b..78978cefc 100644
--- a/cli/standalone/binary.rs
+++ b/cli/standalone/binary.rs
@@ -54,6 +54,7 @@ use crate::http_util::HttpClientProvider;
 use crate::npm::CliNpmResolver;
 use crate::npm::InnerCliNpmResolverRef;
 use crate::standalone::virtual_fs::VfsEntry;
+use crate::util::archive;
 use crate::util::fs::canonicalize_path_maybe_not_exists;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::progress_bar::ProgressBarStyle;
@@ -318,72 +319,6 @@ fn u64_from_bytes(arr: &[u8]) -> Result<u64, AnyError> {
   Ok(u64::from_be_bytes(*fixed_arr))
 }
 
-pub fn unpack_into_dir(
-  exe_name: &str,
-  archive_name: &str,
-  archive_data: Vec<u8>,
-  is_windows: bool,
-  temp_dir: &tempfile::TempDir,
-) -> Result<PathBuf, AnyError> {
-  let temp_dir_path = temp_dir.path();
-  let exe_ext = if is_windows { "exe" } else { "" };
-  let archive_path = temp_dir_path.join(exe_name).with_extension("zip");
-  let exe_path = temp_dir_path.join(exe_name).with_extension(exe_ext);
-  assert!(!exe_path.exists());
-
-  let archive_ext = Path::new(archive_name)
-    .extension()
-    .and_then(|ext| ext.to_str())
-    .unwrap();
-  let unpack_status = match archive_ext {
-    "zip" if cfg!(windows) => {
-      fs::write(&archive_path, &archive_data)?;
-      Command::new("tar.exe")
-        .arg("xf")
-        .arg(&archive_path)
-        .arg("-C")
-        .arg(temp_dir_path)
-        .spawn()
-        .map_err(|err| {
-          if err.kind() == std::io::ErrorKind::NotFound {
-            std::io::Error::new(
-              std::io::ErrorKind::NotFound,
-              "`tar.exe` was not found in your PATH",
-            )
-          } else {
-            err
-          }
-        })?
-        .wait()?
-    }
-    "zip" => {
-      fs::write(&archive_path, &archive_data)?;
-      Command::new("unzip")
-        .current_dir(temp_dir_path)
-        .arg(&archive_path)
-        .spawn()
-        .map_err(|err| {
-          if err.kind() == std::io::ErrorKind::NotFound {
-            std::io::Error::new(
-              std::io::ErrorKind::NotFound,
-              "`unzip` was not found in your PATH, please install `unzip`",
-            )
-          } else {
-            err
-          }
-        })?
-        .wait()?
-    }
-    ext => bail!("Unsupported archive type: '{ext}'"),
-  };
-  if !unpack_status.success() {
-    bail!("Failed to unpack archive.");
-  }
-  assert!(exe_path.exists());
-  fs::remove_file(&archive_path)?;
-  Ok(exe_path)
-}
-
 pub struct DenoCompileBinaryWriter<'a> {
   deno_dir: &'a DenoDir,
   file_fetcher: &'a FileFetcher,
@@ -480,13 +415,13 @@ impl<'a> DenoCompileBinaryWriter<'a> {
 
     let archive_data = std::fs::read(binary_path)?;
     let temp_dir = tempfile::TempDir::new()?;
-    let base_binary_path = unpack_into_dir(
-      "denort",
-      &binary_name,
-      archive_data,
-      target.contains("windows"),
-      &temp_dir,
-    )?;
+    let base_binary_path = archive::unpack_into_dir(archive::UnpackArgs {
+      exe_name: "denort",
+      archive_name: &binary_name,
+      archive_data: &archive_data,
+      is_windows: target.contains("windows"),
+      dest_path: temp_dir.path(),
+    })?;
     let base_binary = std::fs::read(base_binary_path)?;
     drop(temp_dir); // delete the temp dir
     Ok(base_binary)
diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs
index 3fad036ef..830f108e6 100644
--- a/cli/tools/upgrade.rs
+++ b/cli/tools/upgrade.rs
@@ -8,7 +8,7 @@ use crate::colors;
 use crate::factory::CliFactory;
 use crate::http_util::HttpClient;
 use crate::http_util::HttpClientProvider;
-use crate::standalone::binary::unpack_into_dir;
+use crate::util::archive;
 use crate::util::progress_bar::ProgressBar;
 use crate::util::progress_bar::ProgressBarStyle;
 use crate::version;
@@ -556,13 +556,13 @@ pub async fn upgrade(
   );
 
   let temp_dir = tempfile::TempDir::new()?;
-  let new_exe_path = unpack_into_dir(
-    "deno",
-    &ARCHIVE_NAME,
-    archive_data,
-    cfg!(windows),
-    &temp_dir,
-  )?;
+  let new_exe_path = archive::unpack_into_dir(archive::UnpackArgs {
+    exe_name: "deno",
+    archive_name: &ARCHIVE_NAME,
+    archive_data: &archive_data,
+    is_windows: cfg!(windows),
+    dest_path: temp_dir.path(),
+  })?;
   fs::set_permissions(&new_exe_path, permissions)?;
   check_exe(&new_exe_path)?;
 
diff --git a/cli/util/archive.rs b/cli/util/archive.rs
new file mode 100644
index 000000000..e2183d57a
--- /dev/null
+++ b/cli/util/archive.rs
@@ -0,0 +1,118 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+
+use std::fs;
+use std::path::Path;
+use std::path::PathBuf;
+use std::process::Command;
+
+use deno_core::anyhow::bail;
+use deno_core::anyhow::Context;
+use deno_core::error::AnyError;
+
+fn unzip_with_shell(
+  archive_path: &Path,
+  archive_data: &[u8],
+  dest_path: &Path,
+) -> Result<(), AnyError> {
+  fs::write(archive_path, archive_data)?;
+  let unpack_status = if cfg!(windows) {
+    Command::new("tar.exe")
+      .arg("xf")
+      .arg(archive_path)
+      .arg("-C")
+      .arg(dest_path)
+      .spawn()
+      .map_err(|err| {
+        if err.kind() == std::io::ErrorKind::NotFound {
+          std::io::Error::new(
+            std::io::ErrorKind::NotFound,
+            "`tar.exe` was not found in your PATH",
+          )
+        } else {
+          err
+        }
+      })?
+      .wait()?
+  } else {
+    Command::new("unzip")
+      .current_dir(dest_path)
+      .arg(archive_path)
+      .spawn()
+      .map_err(|err| {
+        if err.kind() == std::io::ErrorKind::NotFound {
+          std::io::Error::new(
+            std::io::ErrorKind::NotFound,
+            "`unzip` was not found in your PATH, please install `unzip`",
+          )
+        } else {
+          err
+        }
+      })?
+      .wait()?
+  };
+
+  if !unpack_status.success() {
+    bail!("Failed to unpack archive.");
+  }
+
+  Ok(())
+}
+
+fn unzip(
+  archive_name: &str,
+  archive_data: &[u8],
+  dest_path: &Path,
+) -> Result<(), AnyError> {
+  let mut archive = zip::ZipArchive::new(std::io::Cursor::new(archive_data))?;
+  archive
+    .extract(dest_path)
+    .with_context(|| format!("failed to extract archive: {archive_name}"))?;
+
+  Ok(())
+}
+
+pub struct UnpackArgs<'a> {
+  pub exe_name: &'a str,
+  pub archive_name: &'a str,
+  pub archive_data: &'a [u8],
+  pub is_windows: bool,
+  pub dest_path: &'a Path,
+}
+
+pub fn unpack_into_dir(args: UnpackArgs) -> Result<PathBuf, AnyError> {
+  let UnpackArgs {
+    exe_name,
+    archive_name,
+    archive_data,
+    is_windows,
+    dest_path,
+  } = args;
+  let exe_ext = if is_windows { "exe" } else { "" };
+  let archive_path = dest_path.join(exe_name).with_extension("zip");
+  let exe_path = dest_path.join(exe_name).with_extension(exe_ext);
+  assert!(!exe_path.exists());
+
+  let archive_ext = Path::new(archive_name)
+    .extension()
+    .and_then(|ext| ext.to_str())
+    .unwrap();
+  match archive_ext {
+    "zip" => match unzip(archive_name, archive_data, dest_path) {
+      Ok(()) if !exe_path.exists() => {
+        log::warn!("unpacking via the zip crate didn't produce the executable");
+        // No error but didn't produce exe, fallback to shelling out
+        unzip_with_shell(&archive_path, archive_data, dest_path)?;
+      }
+      Ok(_) => {}
+      Err(e) => {
+        log::warn!("unpacking via zip crate failed: {e}");
+        // Fallback to shelling out
+        unzip_with_shell(&archive_path, archive_data, dest_path)?;
+      }
+    },
+    ext => bail!("Unsupported archive type: '{ext}'"),
+  }
+
+  assert!(exe_path.exists());
+  Ok(exe_path)
+}
diff --git a/cli/util/mod.rs b/cli/util/mod.rs
index b7eef95d3..b9071c496 100644
--- a/cli/util/mod.rs
+++ b/cli/util/mod.rs
@@ -1,6 +1,7 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 // Note: Only add code in this folder that has no application specific logic
+pub mod archive;
 pub mod checksum;
 pub mod console;
 pub mod diff;