diff --git a/download/src/lib.rs b/download/src/lib.rs index 35c68ed6b3..5cbc244720 100644 --- a/download/src/lib.rs +++ b/download/src/lib.rs @@ -42,6 +42,7 @@ pub fn download_to_path_with_backend( callback: Option<&dyn Fn(Event<'_>) -> Result<()>>, ) -> Result<()> { use std::cell::RefCell; + use std::fs::remove_file; use std::fs::OpenOptions; use std::io::{Read, Seek, SeekFrom, Write}; @@ -114,7 +115,10 @@ pub fn download_to_path_with_backend( Ok(()) }() .map_err(|e| { - // TODO is there any point clearing up here? What kind of errors will leave us with an unusable partial? + // TODO: We currently clear up the cached download on any error, should we restrict it to a subset? + remove_file(path) + .chain_err(|| "cleaning up cached downloads") + .unwrap(); e }) } diff --git a/src/dist/download.rs b/src/dist/download.rs index 2f33b98a83..be6cb72ce0 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -68,26 +68,39 @@ impl<'a> DownloadCfg<'a> { + ".partial", ); + let partial_file_existed = partial_file_path.exists(); + let mut hasher = Sha256::new(); - utils::download_file_with_resume( + if let Err(e) = utils::download_file_with_resume( &url, &partial_file_path, Some(&mut hasher), true, &|n| (self.notify_handler)(n.into()), - )?; + ) { + if partial_file_existed { + return Err(e).chain_err(|| ErrorKind::BrokenPartialFile); + } else { + return Err(e); + } + }; let actual_hash = format!("{:x}", hasher.result()); if hash != actual_hash { // Incorrect hash - Err(ErrorKind::ChecksumFailed { - url: url.to_string(), - expected: hash.to_string(), - calculated: actual_hash, + if partial_file_existed { + self.clean(&[hash.to_string() + &".partial".to_string()])?; + Err(ErrorKind::BrokenPartialFile.into()) + } else { + Err(ErrorKind::ChecksumFailed { + url: url.to_string(), + expected: hash.to_string(), + calculated: actual_hash, + } + .into()) } - .into()) } else { (self.notify_handler)(Notification::ChecksumValid(&url.to_string())); diff --git a/src/errors.rs b/src/errors.rs index 7e0f00c88e..5df21d375b 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -362,6 +362,9 @@ error_chain! { description("invalid PGP key"), display("unable to read the PGP key '{}'", v.display()) } + BrokenPartialFile { + description("partially downloaded file may have been damaged and was removed, please try again") + } } } diff --git a/tests/dist.rs b/tests/dist.rs index 9dbde03922..90274f79f5 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -25,6 +25,8 @@ use std::str::FromStr; use std::sync::Arc; use url::Url; +const SHA256_HASH_LEN: usize = 64; + // Creates a mock dist server populated with some test data pub fn create_mock_dist_server( path: &Path, @@ -1984,3 +1986,68 @@ fn checks_files_hashes_before_reuse() { assert!(noticed_bad_checksum.get()); }) } + +#[test] +fn handle_corrupt_partial_downloads() { + setup(None, false, &|url, + toolchain, + prefix, + download_cfg, + temp_cfg| { + // write a corrupt partial out + let path = url.to_file_path().unwrap(); + let target_hash = utils::read_file( + "target hash", + &path.join("dist/2016-02-02/rustc-nightly-x86_64-apple-darwin.tar.gz.sha256"), + ) + .unwrap()[..SHA256_HASH_LEN] + .to_owned(); + + utils::ensure_dir_exists( + "download dir", + &download_cfg.download_dir, + &|_: Notification<'_>| {}, + ) + .unwrap(); + let partial_path = download_cfg + .download_dir + .join(format!("{}.partial", target_hash)); + utils_raw::write_file( + &partial_path, + "file will be resumed from here and not match hash", + ) + .unwrap(); + + let err = update_from_dist( + url, + toolchain, + prefix, + &[], + &[], + download_cfg, + temp_cfg, + false, + ) + .unwrap_err(); + + match err.kind() { + ErrorKind::ComponentDownloadFailed(_) => (), + e => panic!("Unexpected error: {:?}", e), + } + + update_from_dist( + url, + toolchain, + prefix, + &[], + &[], + &download_cfg, + temp_cfg, + false, + ) + .unwrap(); + + assert!(utils::path_exists(&prefix.path().join("bin/rustc"))); + assert!(utils::path_exists(&prefix.path().join("lib/libstd.rlib"))); + }); +}