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

Use the `zip` crate instead

Fixes #23988.

Cargo.lock

@@ -204,6 +204,15 @@ version = "1.0.82"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f538837af36e6f6a9be0faa67f9a314f8119e4e4b5867c6ab40ed60360142519"
 
+[[package]]
+name = "arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7d5a26814d8dcb93b0e5a0ff3c6d80a8843bafb21b39e8e18a6f05471870e110"
+dependencies = [
+ "derive_arbitrary",
+]
+
 [[package]]
 name = "arrayvec"
 version = "0.7.4"
@@ -919,9 +928,9 @@ checksum = "ccaeedb56da03b09f598226e25e80088cb4cd25f316e6e4df7d695f0feeb1403"
 
 [[package]]
 name = "crc32fast"
-version = "1.4.0"
+version = "1.4.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b3855a8a784b474f333699ef2bbca9db2c4a1f6d9088a90a2d25b1eb53111eaa"
+checksum = "a97769d94ddab943e4510d138150169a2758b5ef3eb191a9ee688de3e23ef7b3"
 dependencies = [
  "cfg-if",
 ]
@@ -965,9 +974,9 @@ dependencies = [
 
 [[package]]
 name = "crossbeam-utils"
-version = "0.8.19"
+version = "0.8.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "248e3bacc7dc6baa3b21e405ee045c3047101a49145e7e9eca583ab4c2ca5345"
+checksum = "22ec99545bb0ed0ea7bb9b8e1e9122ea386ff8a48c0922e43f36d45ab09e0e80"
 
 [[package]]
 name = "crypto-bigint"
@@ -1221,6 +1230,7 @@ dependencies = [
  "winapi",
  "winres",
  "zeromq",
+ "zip",
  "zstd",
 ]
 
@@ -2221,6 +2231,17 @@ dependencies = [
  "powerfmt",
 ]
 
+[[package]]
+name = "derive_arbitrary"
+version = "1.3.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.72",
+]
+
 [[package]]
 name = "derive_builder"
 version = "0.12.0"
@@ -2899,9 +2920,9 @@ dependencies = [
 
 [[package]]
 name = "flate2"
-version = "1.0.28"
+version = "1.0.31"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "46303f565772937ffe1d394a4fac6f411c6013172fadde9dcdb1e147a086940e"
+checksum = "7f211bbe8e69bbd0cfdea405084f128ae8b4aaa6b0b522fc8f2b009084797920"
 dependencies = [
  "crc32fast",
  "libz-sys",
@@ -8515,6 +8536,22 @@ dependencies = [
  "uuid",
 ]
 
+[[package]]
+name = "zip"
+version = "2.1.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "40dd8c92efc296286ce1fbd16657c5dbefff44f1b4ca01cc5f517d8b7b3d3e2e"
+dependencies = [
+ "arbitrary",
+ "crc32fast",
+ "crossbeam-utils",
+ "displaydoc",
+ "flate2",
+ "indexmap",
+ "memchr",
+ "thiserror",
+]
+
 [[package]]
 name = "zstd"
 version = "0.12.4"

Cargo.toml

@@ -112,7 +112,7 @@ fast-socks5 = "0.9.6"
 faster-hex = "0.9"
 fastwebsockets = { version = "0.8", features = ["upgrade", "unstable-split"] }
 filetime = "0.2.16"
-flate2 = { version = "1.0.26", default-features = false }
+flate2 = { version = "1.0.30", default-features = false }
 fs3 = "0.5.0"
 futures = "0.3.21"
 glob = "0.3.1"

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]

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)

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)?;
 

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)
+}

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;