From 820f9e51705fd45ea107246d0fcef741627ae4c0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 3 Nov 2026 15:46:38 +0100 Subject: [PATCH 1/7] refactor(download): extract `DownloadCfg::download_with_retry()` --- src/dist/download.rs | 41 +++++++++++++++++++++++++++++++++ src/dist/manifestation.rs | 48 +++++++-------------------------------- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index 6434e12406..ddfd7a8681 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -52,6 +52,45 @@ impl<'a> DownloadCfg<'a> { } } + /// Same as [`DownloadCfg::download()`], but also retries the download if it fails. + /// + /// Only certain errors are considered retriable, and the download will be retried up to + /// `retries` times. The error from the latest retry is returned if all retries are exhausted. + pub(crate) async fn download_with_retry( + &self, + url: &Url, + hash: &str, + status: &DownloadStatus, + mut retries: usize, + ) -> anyhow::Result { + loop { + let downloaded = self.partial_len(hash); + let future = self.download(url, hash, status); + let err = match future.await { + Err(err) => err, + Ok(file) => break Ok(file), + }; + + // retry only known retriable cases + match err.downcast_ref::() { + Some(RustupError::BrokenPartialFile) + | Some(RustupError::IncompletePartialFile) + | Some(RustupError::DownloadingFile { .. }) => {} + _ => return Err(err), + } + + let progress = self.partial_len(hash).saturating_sub(downloaded); + if progress < Self::MIN_PROGRESS { + if retries == 0 { + return Err(err); + } + retries -= 1; + } + + status.retrying(); + } + } + /// Downloads a file and validates its hash. Resumes interrupted downloads. /// Partial downloads are stored in `self.download_dir`, keyed by hash. If the /// target file already exists, then the hash is checked and it is returned @@ -331,6 +370,8 @@ impl<'a> DownloadCfg<'a> { _ => utils::parse_url(url), } } + + const MIN_PROGRESS: u64 = 1024 * 1024; } /// Tracks download progress and displays information about it to a terminal. diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index d10d202a41..3a2eaac19c 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -770,45 +770,15 @@ impl<'a> ComponentBinary<'a> { })) } - async fn download(self, mut retries: usize) -> anyhow::Result<(ComponentInstall, &'a str)> { + async fn download(self, retries: usize) -> anyhow::Result<(ComponentInstall, &'a str)> { let url = self.download_cfg.url(&self.binary.url)?; - let installer = loop { - let downloaded = self.download_cfg.partial_len(&self.binary.hash); - let future = self - .download_cfg - .download(&url, &self.binary.hash, &self.status); - let err = match future.await { - Ok(file) => break file, - Err(err) => err, - }; - - // retry only known retriable cases - match err.downcast_ref::() { - Some(RustupError::BrokenPartialFile) - | Some(RustupError::IncompletePartialFile) - | Some(RustupError::DownloadingFile { .. }) => {} - _ => { - return Err(err).with_context(|| { - RustupError::ComponentDownloadFailed(self.manifest.name(&self.component)) - }); - } - } - - let progress = self - .download_cfg - .partial_len(&self.binary.hash) - .saturating_sub(downloaded); - if progress < Self::MIN_PROGRESS { - if retries == 0 { - return Err(err).with_context(|| { - RustupError::ComponentDownloadFailed(self.manifest.name(&self.component)) - }); - } - retries -= 1; - } - - self.status.retrying(); - }; + let installer = self + .download_cfg + .download_with_retry(&url, &self.binary.hash, &self.status, retries) + .await + .with_context(|| { + RustupError::ComponentDownloadFailed(self.manifest.name(&self.component)) + })?; let install = ComponentInstall { status: self.status, @@ -825,8 +795,6 @@ impl<'a> ComponentBinary<'a> { Ok((install, &self.binary.hash)) } - - const MIN_PROGRESS: u64 = 1024 * 1024; } struct ComponentInstall { From 9364f6a924d739cf25289e33f6c4e20270086bea Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 3 Nov 2026 15:58:27 +0100 Subject: [PATCH 2/7] refactor(download): extract `DownloadCfg::max_retries()` --- src/dist/download.rs | 9 +++++++++ src/dist/manifestation.rs | 12 +++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index ddfd7a8681..b8d0e956eb 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -362,6 +362,15 @@ impl<'a> DownloadCfg<'a> { } } + pub(crate) fn max_retries(&self) -> usize { + const DEFAULT_MAX_RETRIES: usize = 3; + + match self.process.var("RUSTUP_MAX_RETRIES") { + Ok(s) if let Ok(n) = s.parse() => n, + Ok(_) | Err(_) => DEFAULT_MAX_RETRIES, + } + } + pub(crate) fn url(&self, url: &str) -> anyhow::Result { match &*self.tmp_cx.dist_server { server if server != DEFAULT_DIST_SERVER => utils::parse_url( diff --git a/src/dist/manifestation.rs b/src/dist/manifestation.rs index 3a2eaac19c..d19048f0f3 100644 --- a/src/dist/manifestation.rs +++ b/src/dist/manifestation.rs @@ -196,14 +196,6 @@ impl Manifestation { .concurrent_downloads() .unwrap_or(DEFAULT_CONCURRENT_DOWNLOADS); - const DEFAULT_MAX_RETRIES: usize = 3; - let max_retries: usize = download_cfg - .process - .var("RUSTUP_MAX_RETRIES") - .ok() - .and_then(|s| s.parse().ok()) - .unwrap_or(DEFAULT_MAX_RETRIES); - // Begin transaction let mut tx = Transaction::new( prefix.clone(), @@ -268,7 +260,9 @@ impl Manifestation { // Must live outside of `InstallEvents` because we can't write the type of future while stream.components.len() > 0 && stream.downloads.len() < concurrent_downloads { if let Some(bin) = stream.components.next() { - stream.downloads.push(bin.download(max_retries)); + stream + .downloads + .push(bin.download(download_cfg.max_retries())); } } From 7e8c2c2f7df1fae5c1804e97b703c4306f5365fd Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 3 Nov 2026 16:12:23 +0100 Subject: [PATCH 3/7] refactor(self-update): inline redundant `setup_path` --- src/cli/self_update.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 9e153decd0..5a8d025be5 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -1258,17 +1258,16 @@ async fn prepare_update(dl_cfg: &DownloadCfg<'_>) -> anyhow::Result Date: Wed, 4 Nov 2026 09:55:24 +0100 Subject: [PATCH 4/7] test(self-update): clarify naming of existing tests --- tests/suite/cli_self_upd.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index 82ea7a71c9..f06d3d5564 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -452,7 +452,7 @@ info: downloading self-update (new version: [TEST_VERSION]) } #[tokio::test] -async fn update_precise() { +async fn update_with_version_override_exact() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; cx.config .expect(["rustup-init", "-y", "--no-modify-path"]) @@ -774,7 +774,7 @@ info: checking for self-update (current version: [CURRENT_VERSION]) } #[tokio::test] -async fn rustup_self_updates_trivial() { +async fn update_on_rustup_update_trivial() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; cx.config .expect(["rustup", "set", "auto-self-update", "enable"]) @@ -796,7 +796,7 @@ async fn rustup_self_updates_trivial() { } #[tokio::test] -async fn rustup_self_updates_with_specified_toolchain() { +async fn update_on_rustup_update_with_specified_toolchain() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; cx.config .expect(["rustup", "set", "auto-self-update", "enable"]) @@ -821,7 +821,7 @@ async fn rustup_self_updates_with_specified_toolchain() { } #[tokio::test] -async fn rustup_no_self_update_with_specified_toolchain() { +async fn update_disabled_on_rustup_update_with_specified_toolchain() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; cx.config .expect(["rustup-init", "-y", "--no-modify-path"]) @@ -842,7 +842,7 @@ async fn rustup_no_self_update_with_specified_toolchain() { } #[tokio::test] -async fn rustup_self_update_exact() { +async fn update_on_rustup_update_exact() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; cx.config .expect(["rustup", "set", "auto-self-update", "enable"]) From a84090b5f8fb2613b7d5933e39e1d1898d086a6e Mon Sep 17 00:00:00 2001 From: rami3l Date: Tue, 3 Nov 2026 16:03:16 +0100 Subject: [PATCH 5/7] feat(self-update): use `DownloadCfg` for self-updates This feature adds a requirement for the release server to store `rustup-init[.EXE].sha256` files as well, which our official server already ships today. By falling back to the old logic only when `.sha256` returns 404, this design should be a good compromise between compatibility and convenience. --- src/cli/self_update.rs | 40 +++++++++++++++++++++++++++++++------ src/dist/download.rs | 4 ++-- src/download/mod.rs | 2 +- src/test/clitools.rs | 27 +++++++++++++++++++------ tests/suite/cli_self_upd.rs | 39 ++++++++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 15 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 5a8d025be5..8591f56a3f 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -63,7 +63,7 @@ use crate::{ ChannelToolchainName, DistOptions, PartialChannelToolchainName, Profile, TargetTuple, download::DownloadCfg, }, - download::DownloadOptions, + download::{DownloadError, DownloadOptions}, errors::RustupError, install::{InstallMethod, UpdateStatus}, process::Process, @@ -1259,12 +1259,40 @@ async fn prepare_update(dl_cfg: &DownloadCfg<'_>) -> anyhow::Result { + let name = "rustup"; + let status = dl_cfg.status_for(name, name.len()); + + let installer = dl_cfg + .download_with_retry(&download_url, &hash, &status, dl_cfg.max_retries()) + .await?; + + utils::rename( + name, + &installer, + &prepared_updater, + dl_cfg.permit_copy_rename, + )?; + } + // If the corresponding `.sha256` file is not found, skip hash check and use simple download + // as a fallback. + Err(e) + if matches!( + e.downcast_ref::(), + Some(DownloadError::HttpStatus(404) | DownloadError::FileNotFound) + ) => + { + warn!(url = %download_url, "`.sha256` checksum not found, skipping hash check"); + DownloadOptions::try_from(dl_cfg.process)? + .start(&download_url, &prepared_updater) + .download() + .await?; + } + Err(e) => return Err(e), + }; // Mark as executable utils::make_executable(&prepared_updater)?; diff --git a/src/dist/download.rs b/src/dist/download.rs index b8d0e956eb..ca5044a5d4 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -33,7 +33,7 @@ pub struct DownloadCfg<'a> { pub tmp_cx: Arc, pub download_dir: &'a PathBuf, pub(super) tracker: DownloadTracker, - pub(super) permit_copy_rename: bool, + pub permit_copy_rename: bool, pub process: &'a Process, } @@ -184,7 +184,7 @@ impl<'a> DownloadCfg<'a> { Ok(()) } - async fn download_hash(&self, url: &str) -> anyhow::Result { + pub(crate) async fn download_hash(&self, url: &str) -> anyhow::Result { let hash_url = utils::parse_url(&(url.to_owned() + ".sha256"))?; let hash_file = self.tmp_cx.new_file()?; DownloadOptions::try_from(self.process)? diff --git a/src/download/mod.rs b/src/download/mod.rs index 93ef2b6211..207811b075 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -439,7 +439,7 @@ fn native_tls_client(timeout: Duration) -> Result<&'static Client, DownloadError static CLIENT_NATIVE_TLS: OnceLock = OnceLock::new(); #[derive(Debug, Error)] -enum DownloadError { +pub enum DownloadError { #[error("http request returned an unsuccessful status code: {0}")] HttpStatus(u32), #[error("file not found")] diff --git a/src/test/clitools.rs b/src/test/clitools.rs index b5268216b2..b7a81d4a05 100644 --- a/src/test/clitools.rs +++ b/src/test/clitools.rs @@ -5,7 +5,7 @@ use std::{ cell::RefCell, collections::HashMap, env::{self, consts::EXE_SUFFIX}, - ffi::OsStr, + ffi::{OsStr, OsString}, fmt::Debug, fs, io::{self, Write}, @@ -26,7 +26,7 @@ use tempfile::TempDir; use url::Url; use super::{ - CHECKPOINT_ENV, CROSS_ARCH1, CROSS_ARCH2, MULTI_ARCH1, checkpoint_path, + CHECKPOINT_ENV, CROSS_ARCH1, CROSS_ARCH2, MULTI_ARCH1, calc_hash, checkpoint_path, dist::{MockDistServer, MockManifestVersion, Release, RlsStatus, change_channel_date}, mock::MockFile, }; @@ -59,7 +59,7 @@ pub struct Config { /// ~ pub homedir: PathBuf, /// Root for updates to rustup itself aka RUSTUP_UPDATE_ROOT - rustup_update_root: Option, + pub rustup_update_root: Option, /// This is cwd for the test pub workdir: RefCell, /// This is the test root for keeping stuff together @@ -952,9 +952,10 @@ impl SelfUpdateTestContext { let mut dest_file = fs::OpenOptions::new() .append(true) .create(true) - .open(dist_exe) + .open(&dist_exe) .unwrap(); writeln!(dest_file).unwrap(); + gen_hash_file(&dist_exe).unwrap(); Self { config: cx.config, @@ -1048,7 +1049,7 @@ impl CliTestContext { } }; - let deadline = Instant::now() + StdDuration::from_secs(10); + let deadline = Instant::now() + StdDuration::from_secs(30); loop { if matches!(fs::read_to_string(&marker), Ok(contents) if contents == checkpoint) { break parked; @@ -1098,9 +1099,10 @@ impl CliTestContext { let mut dest_file = fs::OpenOptions::new() .append(true) .create(true) - .open(dist_exe) + .open(&dist_exe) .unwrap(); writeln!(dest_file).unwrap(); + gen_hash_file(&dist_exe).unwrap(); self.config.rustup_update_root = Some(root_url); UpdateServerGuard { @@ -1224,6 +1226,19 @@ impl Drop for DistDirGuard<'_> { } } +fn gen_hash_file(src: &Path) -> io::Result<()> { + let hash = calc_hash(src); + let src_name = src + .file_name() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "input file has no name"))? + .display(); + let contents = format!("{hash} {src_name}\n"); + + let mut dst = OsString::from(src); + dst.push(".sha256"); + fs::write(Path::new(&dst), contents) +} + fn create_local_update_server(self_dist: &Path, exedir: &Path, version: &str) -> String { let trip = this_host_tuple(); let dist_dir = self_dist.join(format!("archive/{version}/{trip}")); diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index f06d3d5564..2039674da7 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -21,6 +21,7 @@ use rustup::{ cli::self_update::{ CHECKPOINT_SELF_UPDATE_PREPARED, Marker, SELF_UPDATE_DIRECTORY, updater_path, }, + for_host, test::{ CROSS_ARCH1, CliTestContext, Scenario, SelfUpdateTestContext, calc_hash, output_release_file, this_host_tuple, @@ -451,6 +452,44 @@ info: downloading self-update (new version: [TEST_VERSION]) .is_ok(); } +#[tokio::test] +async fn update_with_missing_hash_file_exact() { + let cx = SelfUpdateTestContext::new(TEST_VERSION).await; + cx.config + .expect(["rustup-init", "-y", "--no-modify-path"]) + .await + .is_ok(); + + // Remove the hash file so that the update falls back to simple download without hash check. + let update_root = cx + .config + .rustup_update_root + .as_ref() + .and_then(|url| url.strip_prefix("file://")) + .unwrap(); + fs::remove_file(for_host!( + "{update_root}/archive/{TEST_VERSION}/{}/rustup-init{EXE_SUFFIX}.sha256" + )) + .unwrap(); + + cx.config + .expect(["rustup", "self", "update"]) + .await + .extend_redactions([("[TEST_VERSION]", TEST_VERSION)]) + .with_stdout(snapbox::str![[r#" + rustup updated - [CURRENT_VERSION] (from [CURRENT_VERSION]) + + +"#]]) + .with_stderr(snapbox::str![[r#" +info: checking for self-update (current version: [CURRENT_VERSION]) +info: downloading self-update (new version: [TEST_VERSION]) +warn: `.sha256` checksum not found, skipping hash check url=[..] + +"#]]) + .is_ok(); +} + #[tokio::test] async fn update_with_version_override_exact() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; From 599895c9b3f6a8d1972c44e71426c73b57ccd25f Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 4 Nov 2026 10:50:56 +0100 Subject: [PATCH 6/7] test(self-update): demonstrate panic with bad hash file --- tests/suite/cli_self_upd.rs | 79 ++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index 2039674da7..b51ee7225d 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -2,7 +2,8 @@ use std::{ env::consts::EXE_SUFFIX, - fs, + fs::{self, File}, + io::Write, path::{Path, PathBuf}, process::Command, }; @@ -452,6 +453,82 @@ info: downloading self-update (new version: [TEST_VERSION]) .is_ok(); } +#[tokio::test] +async fn update_with_non_hex_hash_file_exact() { + let cx = SelfUpdateTestContext::new(TEST_VERSION).await; + cx.config + .expect(["rustup-init", "-y", "--no-modify-path"]) + .await + .is_ok(); + + // Write a bad hash file so that the check fails. + let update_root = cx + .config + .rustup_update_root + .as_ref() + .and_then(|url| url.strip_prefix("file://")) + .unwrap(); + let hash_file = File::create(for_host!( + "{update_root}/archive/{TEST_VERSION}/{}/rustup-init{EXE_SUFFIX}.sha256" + )) + .unwrap(); + write!(&hash_file, "really bad hash").unwrap(); + drop(hash_file); + + cx.config + .expect(["rustup", "self", "update"]) + .await + .extend_redactions([("[TEST_VERSION]", TEST_VERSION)]) + .with_stdout(snapbox::str![""]) + .with_stderr(snapbox::str![[r#" +info: checking for self-update (current version: [CURRENT_VERSION]) +info: downloading self-update (new version: [TEST_VERSION]) +... +end byte index 64 is out of bounds for string of length 15 +... +"#]]) + .is_err(); +} + +#[tokio::test] +async fn update_with_short_hash_file_exact() { + let cx = SelfUpdateTestContext::new(TEST_VERSION).await; + cx.config + .expect(["rustup-init", "-y", "--no-modify-path"]) + .await + .is_ok(); + + // Truncate the hash file so that the check fails. + let update_root = cx + .config + .rustup_update_root + .as_ref() + .and_then(|url| url.strip_prefix("file://")) + .unwrap(); + let hash_file = File::options() + .write(true) + .open(for_host!( + "{update_root}/archive/{TEST_VERSION}/{}/rustup-init{EXE_SUFFIX}.sha256" + )) + .unwrap(); + hash_file.set_len(15).unwrap(); + drop(hash_file); + + cx.config + .expect(["rustup", "self", "update"]) + .await + .extend_redactions([("[TEST_VERSION]", TEST_VERSION)]) + .with_stdout(snapbox::str![""]) + .with_stderr(snapbox::str![[r#" +info: checking for self-update (current version: [CURRENT_VERSION]) +info: downloading self-update (new version: [TEST_VERSION]) +... +end byte index 64 is out of bounds for string of length 15 +... +"#]]) + .is_err(); +} + #[tokio::test] async fn update_with_missing_hash_file_exact() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; From 9c377e7412d57fa7a807510d9270b8f7393a29e8 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 4 Nov 2026 10:44:55 +0100 Subject: [PATCH 7/7] fix(download): reinforce result handling in `DownloadCfg::download_hash()` --- src/dist/download.rs | 16 +++++++++++++++- tests/suite/cli_self_upd.rs | 10 ++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/dist/download.rs b/src/dist/download.rs index ca5044a5d4..d97db502cc 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -191,7 +191,21 @@ impl<'a> DownloadCfg<'a> { .start(&hash_url, &hash_file) .download() .await?; - utils::read_file("hash", &hash_file).map(|s| s[0..64].to_owned()) + + let mut contents = utils::read_file("hash", &hash_file)?; + // NOTE: According to , + // the first column of a `.sha256` file must be a SHA256 sum in hex characters. + let hash_len = 64; + if let Some(hash) = contents.split_whitespace().next() + && hash.len() == hash_len + && hash.chars().all(|c| c.is_ascii_hexdigit()) + { + contents.truncate(hash_len); + return Ok(contents); + } + Err(anyhow!( + "illegal hash file found at `{hash_url}`, expected a checksum in {hash_len} hex characters" + )) } pub(crate) async fn dl_v2_manifest( diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index b51ee7225d..20ab436055 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -483,9 +483,8 @@ async fn update_with_non_hex_hash_file_exact() { .with_stderr(snapbox::str![[r#" info: checking for self-update (current version: [CURRENT_VERSION]) info: downloading self-update (new version: [TEST_VERSION]) -... -end byte index 64 is out of bounds for string of length 15 -... +error: illegal hash file found at `[..]`, expected a checksum in 64 hex characters + "#]]) .is_err(); } @@ -522,9 +521,8 @@ async fn update_with_short_hash_file_exact() { .with_stderr(snapbox::str![[r#" info: checking for self-update (current version: [CURRENT_VERSION]) info: downloading self-update (new version: [TEST_VERSION]) -... -end byte index 64 is out of bounds for string of length 15 -... +error: illegal hash file found at `[..]`, expected a checksum in 64 hex characters + "#]]) .is_err(); }