mirror of
https://github.com/storytold/craft-launcher.git
synced 2026-10-15 06:46:47 -04:00
Keep feed repo renames inside storytold, and never install without a checksum (#208)
Two download-trust gaps from #90. A repo rename in the release feed only had to look like owner/name, so a compromised feed could send an app's downloads, or the launcher's own self-update, to another account's repository with matching links and checksums, and every check would vouch for them. A rename now has to stay under `catalog::GITHUB_OWNER` (storytold) at both ends, or it is dropped and the feed's entry stops matching, so GitHub decides as before. A release with neither a per-file digest nor a SHA256SUMS.txt line was still installed, checked by size only, with a note after the fact; a same-length altered file would have passed. `install::download` now refuses it before fetching anything, with an error that says why. With a checksum always in hand, the size-only path goes: `http::download` takes the SHA-256 as `&str`, and `Verified` and `keep_download` are removed. Tests: a rename within storytold is followed, one out of it is dropped in either direction, and a release without a checksum fetches nothing and never reaches the installer. Fixes #90 Co-authored-by: YangXZ2008 <[email protected]>
This commit is contained in:
1 parent
31a5e426e7
commit
80f70a1a33
6 files changed
+102
-111
No files matched your search
@@ -192,6 +192,9 @@ pub const SUITE: [CraftApp; 12] = [
|
||||
|
||||
/// ArtCraft's website.
|
||||
pub const WEBSITE_URL: &str = "https://getartcraft.com";
|
||||
/// The GitHub account every Craft App's repository lives under: the only one the launcher
|
||||
/// installs from, whatever the release feed says.
|
||||
pub const GITHUB_OWNER: &str = "storytold";
|
||||
/// ArtCraft on GitHub: every Craft App's source.
|
||||
pub const GITHUB_URL: &str = "https://github.com/storytold";
|
||||
/// ArtCraft's social accounts: (service, URL). Shown as small icons, less prominent than Discord.
|
||||
|
||||
+36
-10
@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
|
||||
use serde::Deserialize;
|
||||
use serde::de::{DeserializeOwned, Deserializer};
|
||||
|
||||
use crate::catalog::CraftApp;
|
||||
use crate::catalog::{CraftApp, GITHUB_OWNER};
|
||||
use crate::release::{self, Asset, Release};
|
||||
use crate::time::parse_rfc3339;
|
||||
use crate::version;
|
||||
@@ -36,6 +36,7 @@ pub struct Feed {
|
||||
/// When the least recently fetched app was fetched (Unix seconds).
|
||||
pub last_fetched: Option<i64>,
|
||||
pub app_renames: Vec<AppRename>,
|
||||
/// Repositories that moved, both under [`GITHUB_OWNER`]: a rename anywhere else is dropped.
|
||||
pub repo_renames: Vec<RepoRename>,
|
||||
pub announcements: Vec<Notice>,
|
||||
pub alerts: Vec<Notice>,
|
||||
@@ -140,7 +141,8 @@ impl Feed {
|
||||
self
|
||||
}
|
||||
|
||||
/// The repo to ask GitHub about for `app`, following the feed's repo renames.
|
||||
/// The repo to ask GitHub about for `app`, following the feed's repo renames. Always under
|
||||
/// [`GITHUB_OWNER`]: the feed can move an app within ArtCraft's account, never out of it.
|
||||
pub fn repo_for(&self, app: &CraftApp) -> String {
|
||||
let mut repo = app.repo.to_string();
|
||||
// Follow a chain of moves (a → b → c), without looping forever.
|
||||
@@ -364,10 +366,12 @@ struct RawRepoRename {
|
||||
}
|
||||
|
||||
impl RawRepoRename {
|
||||
/// `None` unless both names are `owner/name` under [`GITHUB_OWNER`]. A feed that could send
|
||||
/// an app's downloads to someone else's repository would make its checksums worthless (#90).
|
||||
fn into_rename(self) -> Option<RepoRename> {
|
||||
let ok = |r: &str| {
|
||||
let mut parts = r.split('/');
|
||||
matches!((parts.next(), parts.next(), parts.next()), (Some(a), Some(b), None) if !a.is_empty() && !b.is_empty()
|
||||
matches!((parts.next(), parts.next(), parts.next()), (Some(a), Some(b), None) if a.eq_ignore_ascii_case(GITHUB_OWNER) && !b.is_empty()
|
||||
&& r.bytes().all(|c| c.is_ascii_alphanumeric() || b"-_./".contains(&c)))
|
||||
};
|
||||
(ok(&self.old_name) && ok(&self.new_name)).then(|| RepoRename {
|
||||
@@ -524,17 +528,39 @@ mod tests {
|
||||
let moved = sample()
|
||||
.replace(
|
||||
"\"repo_renames\": []",
|
||||
r#""repo_renames": [{"old_name": "storytold/photocraft", "new_name": "art/photocraft", "renamed_at": null}]"#,
|
||||
r#""repo_renames": [{"old_name": "Storytold/photocraft", "new_name": "storytold/photocraft-app", "renamed_at": null}]"#,
|
||||
)
|
||||
.replace(
|
||||
"\"name\": \"PhotoCraft\", \"repo\": \"storytold/photocraft\"",
|
||||
"\"name\": \"PhotoCraft\", \"repo\": \"storytold/photocraft-app\"",
|
||||
)
|
||||
.replace("github.com/storytold/photocraft/releases", "github.com/storytold/photocraft-app/releases");
|
||||
let feed = Feed::parse(&moved).unwrap();
|
||||
assert_eq!(feed.repo_for(app("photocraft")), "storytold/photocraft-app");
|
||||
let FeedAnswer::Fresh(Some(r)) = feed.answer(app("photocraft"), NOW) else { panic!() };
|
||||
assert!(r.assets[0].url.starts_with("https://github.com/storytold/photocraft-app/releases/download/"));
|
||||
// The same entry without the rename: the repo doesn't match, so GitHub decides.
|
||||
let unannounced = moved.replace(r#"{"old_name": "Storytold/photocraft", "new_name": "storytold/photocraft-app", "renamed_at": null}"#, "");
|
||||
let feed = Feed::parse(&unannounced).unwrap();
|
||||
assert_eq!(feed.answer(app("photocraft"), NOW), FeedAnswer::AskGitHub);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_rename_out_of_storytold_is_dropped() {
|
||||
// Even a feed that vouches for the new repository, with matching links and checksums,
|
||||
// can't send an app's downloads to another account (#90).
|
||||
let moved = sample()
|
||||
.replace(
|
||||
"\"repo_renames\": []",
|
||||
r#""repo_renames": [{"old_name": "storytold/photocraft", "new_name": "art/photocraft", "renamed_at": null},
|
||||
{"old_name": "art/photocraft", "new_name": "storytold/photocraft", "renamed_at": null},
|
||||
{"old_name": "storytold/photocraft", "new_name": "storytold.evil.example/photocraft", "renamed_at": null}]"#,
|
||||
)
|
||||
.replace("\"name\": \"PhotoCraft\", \"repo\": \"storytold/photocraft\"", "\"name\": \"PhotoCraft\", \"repo\": \"art/photocraft\"")
|
||||
.replace("github.com/storytold/photocraft/releases", "github.com/art/photocraft/releases");
|
||||
let feed = Feed::parse(&moved).unwrap();
|
||||
assert_eq!(feed.repo_for(app("photocraft")), "art/photocraft");
|
||||
let FeedAnswer::Fresh(Some(r)) = feed.answer(app("photocraft"), NOW) else { panic!() };
|
||||
assert!(r.assets[0].url.starts_with("https://github.com/art/photocraft/releases/download/"));
|
||||
// The same entry without the rename: the repo doesn't match, so GitHub decides.
|
||||
let unannounced = moved.replace(r#"{"old_name": "storytold/photocraft", "new_name": "art/photocraft", "renamed_at": null}"#, "");
|
||||
let feed = Feed::parse(&unannounced).unwrap();
|
||||
assert!(feed.repo_renames.is_empty(), "{:?}", feed.repo_renames);
|
||||
assert_eq!(feed.repo_for(app("photocraft")), "storytold/photocraft");
|
||||
assert_eq!(feed.answer(app("photocraft"), NOW), FeedAnswer::AskGitHub);
|
||||
}
|
||||
|
||||
|
||||
@@ -476,7 +476,7 @@ fn run_install(
|
||||
shared.emit(Event::Progress { app: app.id.to_string(), phase: p });
|
||||
};
|
||||
let mut job = Job { http, paths, stop, phase: &mut phase };
|
||||
let (new_record, verified) = install::install(app, &release, &asset, kind, existing, record, &mut job)?;
|
||||
let new_record = install::install(app, &release, &asset, kind, existing, record, &mut job)?;
|
||||
{
|
||||
let mut m = shared.manifest.lock().unwrap_or_else(PoisonError::into_inner);
|
||||
let changed = match new_record {
|
||||
@@ -491,11 +491,7 @@ fn run_install(
|
||||
log::warn!("couldn't save the install record: {e}");
|
||||
}
|
||||
}
|
||||
let note = match verified {
|
||||
http::Verified::Checksum => "",
|
||||
http::Verified::SizeOnly => " (this release publishes no checksum, so only its size was checked)",
|
||||
};
|
||||
Ok(format!("{}{note}", installed_text(app, existing.is_some(), &release)))
|
||||
Ok(installed_text(app, existing.is_some(), &release))
|
||||
}
|
||||
|
||||
/// What a finished install tells the user.
|
||||
|
||||
+20
-55
@@ -178,24 +178,15 @@ impl Http {
|
||||
/// its partial file; a file that fails its checksum is fetched once more from scratch before
|
||||
/// giving up. When the connection stays down, the download ends paused (`PAUSED`), never
|
||||
/// losing what it has. `stop` pauses or cancels it between reads.
|
||||
pub fn download(
|
||||
&self,
|
||||
url: &str,
|
||||
dest: &Path,
|
||||
size: u64,
|
||||
sha256: Option<&str>,
|
||||
stop: &Stop,
|
||||
progress: &mut dyn FnMut(Transfer),
|
||||
) -> Result<Verified, String> {
|
||||
pub fn download(&self, url: &str, dest: &Path, size: u64, sha256: &str, stop: &Stop, progress: &mut dyn FnMut(Transfer)) -> Result<(), String> {
|
||||
if let Some(dir) = dest.parent() {
|
||||
std::fs::create_dir_all(dir).map_err(|e| format!("couldn't create {} ({e})", dir.display()))?;
|
||||
}
|
||||
let verified = if sha256.is_some() { Verified::Checksum } else { Verified::SizeOnly };
|
||||
// A finished copy from an earlier attempt that never got installed. Marking it as just
|
||||
// used, before checking it, keeps `clean_downloads` away from it while it's installed.
|
||||
if dest.is_file() && touch(dest) && matches_published(dest, size, sha256) {
|
||||
if dest.is_file() && touch(dest) && matches_published(dest, sha256) {
|
||||
progress(Transfer::Bytes { done: size, total: size });
|
||||
return Ok(verified);
|
||||
return Ok(());
|
||||
}
|
||||
// The partial file is named after the release file and its size; if it still turns out
|
||||
// to belong to something else, the checksum catches it and the download starts over.
|
||||
@@ -210,9 +201,9 @@ impl Http {
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
if matches_published(&part, size, sha256) {
|
||||
if matches_published(&part, sha256) {
|
||||
std::fs::rename(&part, dest).map_err(|e| format!("couldn't save the download ({e})"))?;
|
||||
return Ok(verified);
|
||||
return Ok(());
|
||||
}
|
||||
let _ = std::fs::remove_file(&part);
|
||||
if fresh_start_used {
|
||||
@@ -359,15 +350,6 @@ pub enum Transfer {
|
||||
},
|
||||
}
|
||||
|
||||
/// How a finished download was checked.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum Verified {
|
||||
/// Against the release's published SHA-256.
|
||||
Checksum,
|
||||
/// The release publishes no checksum: only its size could be checked.
|
||||
SizeOnly,
|
||||
}
|
||||
|
||||
/// Asks a running download to pause (keeping its partial file) or cancel (deleting it).
|
||||
#[derive(Debug, Default)]
|
||||
pub struct Stop(AtomicU8);
|
||||
@@ -395,12 +377,9 @@ impl Stop {
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether a file on disk is the published one: its checksum when there is one, else its size.
|
||||
fn matches_published(path: &Path, size: u64, sha256: Option<&str>) -> bool {
|
||||
match sha256 {
|
||||
Some(sha) => sha256_file(path).is_ok_and(|h| h.eq_ignore_ascii_case(sha)),
|
||||
None => size > 0 && std::fs::metadata(path).is_ok_and(|m| m.len() == size),
|
||||
}
|
||||
/// Whether a file on disk is the published one: its SHA-256 matches.
|
||||
fn matches_published(path: &Path, sha256: &str) -> bool {
|
||||
sha256_file(path).is_ok_and(|h| h.eq_ignore_ascii_case(sha256))
|
||||
}
|
||||
|
||||
/// How much of a download is already on disk (a paused or interrupted one).
|
||||
@@ -564,18 +543,18 @@ pub(crate) mod tests {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join("file.bin");
|
||||
let http = Http::new().unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
http.download(&url, &dest, len, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert!(finished(&dest, len) && !finished(&dest, 0));
|
||||
// Old enough to be cleaned up, were it not about to be used.
|
||||
let old = std::time::SystemTime::now() - Duration::from_secs(40 * 86_400);
|
||||
std::fs::File::options().write(true).open(&dest).unwrap().set_modified(old).unwrap();
|
||||
let mut last = (0, 0);
|
||||
let again = http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |t| {
|
||||
let again = http.download(&url, &dest, len, &sha, &Stop::default(), &mut |t| {
|
||||
if let Transfer::Bytes { done, total } = t {
|
||||
last = (done, total);
|
||||
}
|
||||
});
|
||||
assert_eq!(again, Ok(Verified::Checksum));
|
||||
assert_eq!(again, Ok(()));
|
||||
assert_eq!(last, (len, len));
|
||||
assert_eq!(requests.lock().unwrap().len(), 1);
|
||||
// A cleanup while the installer runs leaves the reused file alone.
|
||||
@@ -593,10 +572,10 @@ pub(crate) mod tests {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join("file.bin");
|
||||
let http = Http::new().unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
http.download(&url, &dest, len, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
// The same size, so only the checksum can tell.
|
||||
std::fs::write(&dest, vec![b'x'; body.len()]).unwrap();
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
http.download(&url, &dest, len, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
assert_eq!(requests.lock().unwrap().len(), 2);
|
||||
}
|
||||
@@ -689,15 +668,12 @@ pub(crate) mod tests {
|
||||
let dest = dir.path().join("file.bin");
|
||||
let http = Http::new().unwrap();
|
||||
let mut last = (0, 0);
|
||||
let result = http.download(&url, &dest, body.len() as u64, Some(&sha), &Stop::default(), &mut |t| {
|
||||
let result = http.download(&url, &dest, body.len() as u64, &sha, &Stop::default(), &mut |t| {
|
||||
if let Transfer::Bytes { done, total } = t {
|
||||
last = (done, total);
|
||||
}
|
||||
});
|
||||
let result = result.map(|v| {
|
||||
assert_eq!(v, Verified::Checksum);
|
||||
std::fs::read(&dest).unwrap()
|
||||
});
|
||||
let result = result.map(|()| std::fs::read(&dest).unwrap());
|
||||
if result.is_ok() {
|
||||
assert_eq!(last, (body.len() as u64, body.len() as u64));
|
||||
}
|
||||
@@ -733,17 +709,6 @@ pub(crate) mod tests {
|
||||
assert!(result.unwrap_err().contains("checksum"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn without_a_checksum_the_size_is_still_checked() {
|
||||
let (body, _) = body();
|
||||
let (url, _) = serve(body.clone(), vec![Reply::Full]);
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join("file.bin");
|
||||
let v = Http::new().unwrap().download(&url, &dest, body.len() as u64, None, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(v, Verified::SizeOnly);
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pausing_keeps_the_partial_file_and_resuming_continues_it() {
|
||||
let (body, sha) = body();
|
||||
@@ -755,7 +720,7 @@ pub(crate) mod tests {
|
||||
let stop = Stop::default();
|
||||
// Pause as soon as the first bytes arrive.
|
||||
let err = http
|
||||
.download(&url, &dest, len, Some(&sha), &stop, &mut |t| {
|
||||
.download(&url, &dest, len, &sha, &stop, &mut |t| {
|
||||
if matches!(t, Transfer::Bytes { done, .. } if done > 0) {
|
||||
stop.pause();
|
||||
}
|
||||
@@ -764,7 +729,7 @@ pub(crate) mod tests {
|
||||
assert_eq!(err, PAUSED);
|
||||
let kept = partial_bytes(&dest, len).unwrap();
|
||||
assert!(kept > 0 && kept < len, "{kept}");
|
||||
http.download(&url, &dest, len, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
http.download(&url, &dest, len, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
assert_eq!(ranges.lock().unwrap()[1], Some(kept.to_string()));
|
||||
}
|
||||
@@ -779,7 +744,7 @@ pub(crate) mod tests {
|
||||
let stop = Stop::default();
|
||||
let err = Http::new()
|
||||
.unwrap()
|
||||
.download(&url, &dest, len, Some(&sha), &stop, &mut |t| {
|
||||
.download(&url, &dest, len, &sha, &stop, &mut |t| {
|
||||
if matches!(t, Transfer::Bytes { done, .. } if done > 0) {
|
||||
stop.cancel();
|
||||
}
|
||||
@@ -804,7 +769,7 @@ pub(crate) mod tests {
|
||||
let dest = dir.path().join("file.bin");
|
||||
std::fs::write(part_path(&dest, body.len() as u64), &body[..500_000]).unwrap();
|
||||
assert_eq!(partial_bytes(&dest, body.len() as u64), Some(500_000));
|
||||
Http::new().unwrap().download(&url, &dest, body.len() as u64, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
Http::new().unwrap().download(&url, &dest, body.len() as u64, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(std::fs::read(&dest).unwrap(), body);
|
||||
assert_eq!(*ranges.lock().unwrap(), [Some("500000".to_string())]);
|
||||
assert!(!part_path(&dest, body.len() as u64).exists());
|
||||
@@ -825,7 +790,7 @@ pub(crate) mod tests {
|
||||
let sha = asset.sha256.clone().or_else(|| sums.iter().find(|(_, n)| *n == asset.name).map(|(h, _)| h.clone())).unwrap();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let dest = dir.path().join(&asset.name);
|
||||
http.download(&asset.url, &dest, asset.size, Some(&sha), &Stop::default(), &mut |_| {}).unwrap();
|
||||
http.download(&asset.url, &dest, asset.size, &sha, &Stop::default(), &mut |_| {}).unwrap();
|
||||
assert_eq!(std::fs::metadata(&dest).unwrap().len(), asset.size);
|
||||
}
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ use artcraft_launcher_core::protocol::{CANCELLED, INSTALL_CANCELLED, JobPhase};
|
||||
use artcraft_launcher_core::release::{self, Asset, Release};
|
||||
use artcraft_launcher_core::{CraftApp, InstallSource, Installed, PackageKind};
|
||||
|
||||
use crate::http::{Http, Stop, Transfer, Verified};
|
||||
use crate::http::{Http, Stop, Transfer};
|
||||
use crate::manifest::Record;
|
||||
use crate::paths::Paths;
|
||||
use crate::process;
|
||||
@@ -45,7 +45,7 @@ pub fn install(
|
||||
existing: Option<&Installed>,
|
||||
previous: Option<&Record>,
|
||||
job: &mut Job<'_>,
|
||||
) -> Result<(Option<Record>, Verified), String> {
|
||||
) -> Result<Option<Record>, String> {
|
||||
let _ = previous;
|
||||
if let Some(inst) = existing
|
||||
&& inst.source.launcher_can_update()
|
||||
@@ -54,7 +54,7 @@ pub fn install(
|
||||
return Err(format!("{} is running. Quit it, then try again.", app.name));
|
||||
}
|
||||
log::info!("{}: starting {} to {} ({})", app.name, if existing.is_some() { "update" } else { "install" }, release.version_label(), kind.label());
|
||||
let (file, verified) = download(release, asset, job)?;
|
||||
let file = download(release, asset, job)?;
|
||||
log::info!("{}: download ready ({}), running installer...", app.name, file.display());
|
||||
// Past this point the install can't be interrupted halfway.
|
||||
(job.phase)(JobPhase::Installing);
|
||||
@@ -81,18 +81,13 @@ pub fn install(
|
||||
Err(e) if e == CANCELLED => log::info!("{}: installation cancelled", app.name),
|
||||
Err(e) => log::error!("{}: installation failed: {e}", app.name),
|
||||
}
|
||||
if !keep_download(result.is_ok(), verified) {
|
||||
// A failed install keeps the download: the next try checks it against its published checksum
|
||||
// again and skips the transfer.
|
||||
if result.is_ok() {
|
||||
let _ = std::fs::remove_file(&file);
|
||||
}
|
||||
// The system's own prompt was dismissed: the download itself wasn't cancelled.
|
||||
result.map(|r| (r, verified)).map_err(|e| if e == CANCELLED { INSTALL_CANCELLED.to_string() } else { e })
|
||||
}
|
||||
|
||||
/// Whether the download stays on disk once the install step has ended. A failed install keeps it:
|
||||
/// the next try checks it against its published checksum again and skips the transfer. Without a
|
||||
/// checksum there's nothing to check it against later, so it goes.
|
||||
fn keep_download(installed: bool, verified: Verified) -> bool {
|
||||
!installed && verified == Verified::Checksum
|
||||
result.map_err(|e| if e == CANCELLED { INSTALL_CANCELLED.to_string() } else { e })
|
||||
}
|
||||
|
||||
/// Where `asset` is downloaded to (and its partial file kept while unfinished).
|
||||
@@ -150,10 +145,10 @@ pub fn available_kinds() -> Vec<PackageKind> {
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
/// Fetches `asset` into the download folder, checked against its published SHA-256: GitHub's
|
||||
/// per-file digest, else the release's `SHA256SUMS.txt`. A release with neither still installs,
|
||||
/// checked by size only, and says so.
|
||||
pub(crate) fn download(release: &Release, asset: &Asset, job: &mut Job<'_>) -> Result<(PathBuf, Verified), String> {
|
||||
/// Fetches `asset` into the download folder, checked against its published SHA-256: the feed's
|
||||
/// or GitHub's per-file digest, else the release's `SHA256SUMS.txt`. A release with neither is
|
||||
/// refused before anything is fetched: a file nothing vouches for is never installed (#90).
|
||||
pub(crate) fn download(release: &Release, asset: &Asset, job: &mut Job<'_>) -> Result<PathBuf, String> {
|
||||
if !release::valid_file_name(&asset.name) {
|
||||
return Err("the release has an unexpected file name".into());
|
||||
}
|
||||
@@ -161,17 +156,20 @@ pub(crate) fn download(release: &Release, asset: &Asset, job: &mut Job<'_>) -> R
|
||||
Some(s) => Some(s.clone()),
|
||||
None => published_checksum(release, asset, job.http)?,
|
||||
};
|
||||
if sha.is_none() {
|
||||
log::warn!("{}: no published checksum; checking the size only", asset.name);
|
||||
}
|
||||
let Some(sha) = sha else {
|
||||
return Err(format!(
|
||||
"the release publishes no checksum for {}, so it can't be verified and wasn't installed. Try again later, or report it on GitHub.",
|
||||
asset.name
|
||||
));
|
||||
};
|
||||
let dest = download_path(job.paths, asset);
|
||||
let phase = &mut *job.phase;
|
||||
let verified = job.http.download(&asset.url, &dest, asset.size, sha.as_deref(), job.stop, &mut |t| match t {
|
||||
job.http.download(&asset.url, &dest, asset.size, &sha, job.stop, &mut |t| match t {
|
||||
Transfer::Bytes { done, total } => phase(JobPhase::Downloading { done, total }),
|
||||
Transfer::Reconnecting { done, total } => phase(JobPhase::Reconnecting { done, total }),
|
||||
})?;
|
||||
(job.phase)(JobPhase::Verifying);
|
||||
Ok((dest, verified))
|
||||
Ok(dest)
|
||||
}
|
||||
|
||||
/// The file's line in the release's `SHA256SUMS.txt`. `Ok(None)` only when the release really
|
||||
@@ -1100,18 +1098,6 @@ mod tests {
|
||||
assert_eq!(std::fs::read_to_string(dest.join("v")).unwrap(), "new");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_a_failed_install_of_a_checksummed_download_keeps_it() {
|
||||
for (installed, verified, keep) in [
|
||||
(false, Verified::Checksum, true),
|
||||
(false, Verified::SizeOnly, false),
|
||||
(true, Verified::Checksum, false),
|
||||
(true, Verified::SizeOnly, false),
|
||||
] {
|
||||
assert_eq!(keep_download(installed, verified), keep, "installed: {installed}, {verified:?}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_install_keeps_the_download_for_the_next_try() {
|
||||
use crate::http::tests::{Reply, body, serve};
|
||||
@@ -1135,14 +1121,29 @@ mod tests {
|
||||
}
|
||||
assert_eq!(requests.lock().unwrap().len(), 1, "the second try reuses the kept file");
|
||||
assert_eq!(phases.iter().filter(|p| **p == JobPhase::Installing).count(), 2);
|
||||
}
|
||||
|
||||
// Checked by size only: not kept.
|
||||
let unchecked = Asset { name: "wordcraft-0.3.1-test.pkg".into(), sha256: None, ..asset };
|
||||
let mut phase = |_: JobPhase| {};
|
||||
#[test]
|
||||
fn a_release_without_a_checksum_is_refused_before_anything_is_fetched() {
|
||||
use crate::http::tests::{Reply, body, serve};
|
||||
let app = CraftApp::by_id("wordcraft").unwrap();
|
||||
let (body, _) = body();
|
||||
let (url, requests) = serve(body.clone(), vec![Reply::Full]);
|
||||
// Neither a per-file digest nor a SHA256SUMS.txt in the release.
|
||||
let asset = Asset { name: "wordcraft-0.3.1-test.pkg".into(), size: body.len() as u64, url, sha256: None };
|
||||
let release = Release { assets: vec![asset.clone()], ..Release::default() };
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let paths = Paths::under(dir.path());
|
||||
let http = Http::new().unwrap();
|
||||
let kind = if cfg!(windows) { PackageKind::Dmg } else { PackageKind::Msi };
|
||||
let mut phases = Vec::new();
|
||||
let mut phase = |p: JobPhase| phases.push(p);
|
||||
let mut job = Job { http: &http, paths: &paths, stop: &Stop::default(), phase: &mut phase };
|
||||
install(app, &release, &unchecked, kind, None, None, &mut job).unwrap_err();
|
||||
assert!(!download_path(&paths, &unchecked).exists());
|
||||
assert_eq!(requests.lock().unwrap().len(), 2);
|
||||
let err = install(app, &release, &asset, kind, None, None, &mut job).unwrap_err();
|
||||
assert!(err.contains("publishes no checksum"), "{err}");
|
||||
assert!(!download_path(&paths, &asset).exists());
|
||||
assert!(requests.lock().unwrap().is_empty(), "nothing was fetched");
|
||||
assert!(!phases.contains(&JobPhase::Installing));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
|
||||
@@ -169,7 +169,7 @@ pub fn update(
|
||||
.ok_or_else(|| format!("ArtCraft Launcher {} has no {} for {}", release.version_label(), install.kind().label(), platform.label()))?;
|
||||
let asset = asset.clone();
|
||||
let mut job = Job { http, paths, stop, phase };
|
||||
let (file, _) = install::download(release, &asset, &mut job)?;
|
||||
let file = install::download(release, &asset, &mut job)?;
|
||||
// The restart ends this process: nothing else may be under way.
|
||||
last()?;
|
||||
phase(JobPhase::Installing);
|
||||
|
||||
Reference in new issue
Block a user