From 397c34ca1512e592d08bf7b3c7e86b65d42ee08f Mon Sep 17 00:00:00 2001 From: Leo K Date: Wed, 7 Jul 2021 20:59:39 +0200 Subject: [PATCH] fix(cli/tools/upgrade): check if passed version is valid (#11296) --- cli/tests/integration/upgrade_tests.rs | 61 +++++++++++++++++++++++--- cli/tools/standalone.rs | 7 +-- cli/tools/upgrade.rs | 32 +++++++------- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/cli/tests/integration/upgrade_tests.rs b/cli/tests/integration/upgrade_tests.rs index 4edd7a418e..60a84e673c 100644 --- a/cli/tests/integration/upgrade_tests.rs +++ b/cli/tests/integration/upgrade_tests.rs @@ -1,6 +1,6 @@ // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license. -use std::process::Command; +use std::process::{Command, Stdio}; use tempfile::TempDir; use test_util as util; @@ -63,7 +63,7 @@ fn upgrade_with_version_in_tmpdir() { .arg("upgrade") .arg("--force") .arg("--version") - .arg("0.42.0") + .arg("1.11.5") .spawn() .unwrap() .wait() @@ -73,7 +73,7 @@ fn upgrade_with_version_in_tmpdir() { Command::new(&exe_path).arg("-V").output().unwrap().stdout, ) .unwrap(); - assert!(upgraded_deno_version.contains("0.42.0")); + assert!(upgraded_deno_version.contains("1.11.5")); let _mtime2 = std::fs::metadata(&exe_path).unwrap().modified().unwrap(); // TODO(ry) assert!(mtime1 < mtime2); } @@ -121,7 +121,7 @@ fn upgrade_with_out_in_tmpdir() { let status = Command::new(&exe_path) .arg("upgrade") .arg("--version") - .arg("1.0.2") + .arg("1.11.5") .arg("--output") .arg(&new_exe_path.to_str().unwrap()) .spawn() @@ -141,5 +141,56 @@ fn upgrade_with_out_in_tmpdir() { .stdout, ) .unwrap(); - assert!(v.contains("1.0.2")); + assert!(v.contains("1.11.5")); +} + +// Warning: this test requires internet access. +// TODO(#7412): reenable. test is flaky +#[test] +#[ignore] +fn upgrade_invalid_stable_version() { + let temp_dir = TempDir::new().unwrap(); + let exe_path = temp_dir.path().join("deno"); + let _ = std::fs::copy(util::deno_exe_path(), &exe_path).unwrap(); + assert!(exe_path.exists()); + let output = Command::new(&exe_path) + .arg("upgrade") + .arg("--version") + .arg("foobar") + .stderr(Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!( + "error: Invalid semver passed\n", + util::strip_ansi_codes(&String::from_utf8(output.stderr).unwrap()) + ); +} + +// Warning: this test requires internet access. +// TODO(#7412): reenable. test is flaky +#[test] +#[ignore] +fn upgrade_invalid_canary_version() { + let temp_dir = TempDir::new().unwrap(); + let exe_path = temp_dir.path().join("deno"); + let _ = std::fs::copy(util::deno_exe_path(), &exe_path).unwrap(); + assert!(exe_path.exists()); + let output = Command::new(&exe_path) + .arg("upgrade") + .arg("--canary") + .arg("--version") + .arg("foobar") + .stderr(Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!( + "error: Invalid commit hash passed\n", + util::strip_ansi_codes(&String::from_utf8(output.stderr).unwrap()) + ); } diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index 37f4649142..ab093eec1e 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -46,11 +46,8 @@ pub async fn get_base_binary( } let archive_data = tokio::fs::read(binary_path).await?; - let base_binary_path = crate::tools::upgrade::unpack( - archive_data, - "deno", - target.contains("windows"), - )?; + let base_binary_path = + crate::tools::upgrade::unpack(archive_data, target.contains("windows"))?; let base_binary = tokio::fs::read(base_binary_path).await?; Ok(base_binary) } diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index 75e84b9960..752929b94b 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -2,7 +2,7 @@ //! This module provides feature to upgrade deno executable -use deno_core::error::AnyError; +use deno_core::error::{bail, AnyError}; use deno_core::futures::StreamExt; use deno_runtime::deno_fetch::reqwest; use deno_runtime::deno_fetch::reqwest::Client; @@ -41,6 +41,14 @@ pub async fn upgrade_command( let install_version = match version { Some(passed_version) => { + if canary + && !regex::Regex::new("^[0-9a-f]{40}$")?.is_match(&passed_version) + { + bail!("Invalid commit hash passed"); + } else if !canary && semver_parse(&passed_version).is_err() { + bail!("Invalid semver passed"); + } + let current_is_passed = if canary { crate::version::GIT_COMMIT_HASH == passed_version } else if !crate::version::is_canary() { @@ -68,14 +76,8 @@ pub async fn upgrade_command( latest_hash.truncate(7); crate::version::GIT_COMMIT_HASH == latest_hash } else if !crate::version::is_canary() { - let current = semver_parse(&*crate::version::deno()).unwrap(); - let latest = match semver_parse(&latest_version) { - Ok(v) => v, - Err(_) => { - eprintln!("Invalid semver passed"); - std::process::exit(1) - } - }; + let current = semver_parse(&crate::version::deno()).unwrap(); + let latest = semver_parse(&latest_version).unwrap(); current >= latest } else { false @@ -88,7 +90,7 @@ pub async fn upgrade_command( ); return Ok(()); } else { - println!("Found latest version {}", &latest_version); + println!("Found latest version {}", latest_version); latest_version } } @@ -106,12 +108,12 @@ pub async fn upgrade_command( ) }; - let archive_data = download_package(client, &*download_url).await?; + let archive_data = download_package(client, &download_url).await?; println!("Deno is upgrading to version {}", &install_version); let old_exe_path = std::env::current_exe()?; - let new_exe_path = unpack(archive_data, "deno", cfg!(windows))?; + let new_exe_path = unpack(archive_data, cfg!(windows))?; let permissions = fs::metadata(&old_exe_path)?.permissions(); fs::set_permissions(&new_exe_path, permissions)?; check_exe(&new_exe_path)?; @@ -213,16 +215,16 @@ async fn download_package( pub fn unpack( archive_data: Vec, - exe_name: &str, is_windows: bool, ) -> Result { + const EXE_NAME: &str = "deno"; // We use into_path so that the tempdir is not automatically deleted. This is // useful for debugging upgrade, but also so this function can return a path // to the newly uncompressed file without fear of the tempdir being deleted. let temp_dir = TempDir::new()?.into_path(); let exe_ext = if is_windows { "exe" } else { "" }; - let archive_path = temp_dir.join(exe_name).with_extension("zip"); - let exe_path = temp_dir.join(exe_name).with_extension(exe_ext); + let archive_path = temp_dir.join(EXE_NAME).with_extension("zip"); + let exe_path = temp_dir.join(EXE_NAME).with_extension(exe_ext); assert!(!exe_path.exists()); let archive_ext = Path::new(&*ARCHIVE_NAME)