mirror of
https://github.com/storytold/craft-libs.git
synced 2026-11-12 21:42:13 +00:00
craft-update: tests for the review's cases (other host, two links, running app, bad checksum)
FredApps asked on #388 for tests of two links for one asset, a link to another host, a failing commit, a newer installed copy and a running app. The failing commit and the newer copy were already tested on main (portable.rs, install_refuses_to_downgrade_when_target_release_is_older). - release.rs: a_download_link_must_be_its_releases_own also refuses the right path on another host, over http, and behind look-alike hosts. New two_links_for_one_asset_keep_only_the_releases_own. - install.rs: install_refuses_links_off_the_releases_own_before_downloading covers other hosts and a SHA256SUMS.txt linked elsewhere (the sums_asset branch of check_download_link). install_refuses_while_the_installed_copy_is_running uses the test binary on Windows and process.rs's RunningProbe on Unix, where pgrep -f matches the command line. a_download_that_fails_its_checksum_replaces_nothing: the installed copy keeps its bytes, the install never starts, nothing is committed or kept. - process.rs: RunningProbe is pub(crate) so install.rs's test can use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c4530bb26e
commit
3ad90741b7
3 files changed
+139
-3
No files matched your search
@@ -2844,6 +2844,110 @@ mod tests {
|
|||||||
assert!(bind_artifact_version(app, &release, &other_escape).is_err());
|
assert!(bind_artifact_version(app, &release, &other_escape).is_err());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// craft-launcher#764: `install_at` refuses, before downloading, an asset whose link is the
|
||||||
|
/// right path on another host, and a release whose `SHA256SUMS.txt` (what the download is
|
||||||
|
/// checked against) is linked elsewhere. The stop is cancelled, so if the check stopped working
|
||||||
|
/// the install would end at the download with `CANCELLED` instead of fetching anything.
|
||||||
|
#[test]
|
||||||
|
fn install_refuses_links_off_the_releases_own_before_downloading() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let paths = Paths::under(dir.path());
|
||||||
|
let http = Http::new(crate::TEST_AGENT).unwrap();
|
||||||
|
let locations = InstallLocations::default();
|
||||||
|
let app = App::by_id("photocraft").unwrap();
|
||||||
|
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
|
||||||
|
let own = release::release_from_sums(app.repo, "v0.5.0", &[("a".repeat(64), name.to_string())]);
|
||||||
|
let refuse = |release: &Release, asset: &Asset| {
|
||||||
|
let stop = Stop::default();
|
||||||
|
stop.cancel();
|
||||||
|
let phase = &mut |_| ();
|
||||||
|
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase, install_locations: &locations, replace: false };
|
||||||
|
let result = install_at(app, release, asset, PackageKind::AppImage, None, None, &mut job, None);
|
||||||
|
assert!(result.as_ref().is_err_and(|e| e.contains("isn't the one its release publishes")), "{}: {result:?}", asset.url);
|
||||||
|
};
|
||||||
|
|
||||||
|
for url in [
|
||||||
|
format!("https://evil.example/storytold/photocraft/releases/download/v0.5.0/{name}"),
|
||||||
|
format!("http://github.com/storytold/photocraft/releases/download/v0.5.0/{name}"),
|
||||||
|
format!("https://github.com.evil.example/storytold/photocraft/releases/download/v0.5.0/{name}"),
|
||||||
|
] {
|
||||||
|
refuse(&own, &Asset { url, ..own.assets[0].clone() });
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut sums_elsewhere = own.clone();
|
||||||
|
let sums = sums_elsewhere.assets.iter_mut().find(|a| a.name == "SHA256SUMS.txt").unwrap();
|
||||||
|
sums.url = "https://evil.example/storytold/photocraft/releases/download/v0.5.0/SHA256SUMS.txt".into();
|
||||||
|
refuse(&sums_elsewhere, &sums_elsewhere.assets[0]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A download that fails its checksum (twice, as a resumed file gets one fresh try) replaces
|
||||||
|
/// nothing: the installed copy keeps its bytes, the install never starts, the record isn't
|
||||||
|
/// committed, and the bad file isn't kept for the next try.
|
||||||
|
#[test]
|
||||||
|
fn a_download_that_fails_its_checksum_replaces_nothing() {
|
||||||
|
use crate::http::tests::{Reply, body, serve};
|
||||||
|
let app = App::by_id("photocraft").unwrap();
|
||||||
|
let (body, sha) = body();
|
||||||
|
let (url, requests) = serve(body.clone(), vec![Reply::Corrupt, Reply::Corrupt]);
|
||||||
|
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
|
||||||
|
let asset = Asset { name: name.into(), size: body.len() as u64, url, sha256: Some(sha) };
|
||||||
|
let release = Release { version: version::parse("0.5.0"), tag: "v0.5.0".into(), assets: vec![asset.clone()], ..Release::default() };
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let paths = Paths::under(dir.path());
|
||||||
|
let http = Http::new(crate::TEST_AGENT).unwrap();
|
||||||
|
let old = dir.path().join("Apps").join("PhotoCraft.AppImage");
|
||||||
|
std::fs::create_dir_all(old.parent().unwrap()).unwrap();
|
||||||
|
std::fs::write(&old, b"the installed 0.4.0").unwrap();
|
||||||
|
let installed =
|
||||||
|
Installed { version: version::parse("0.4.0"), source: InstallSource::Launcher, location: old.display().to_string(), package: None };
|
||||||
|
let previous = record("0.4.0", PackageKind::AppImage, old.clone(), vec![], false);
|
||||||
|
let mut phases = Vec::new();
|
||||||
|
let mut phase = |p: JobPhase| phases.push(p);
|
||||||
|
let locations = InstallLocations::default();
|
||||||
|
let stop = Stop::default();
|
||||||
|
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase: &mut phase, install_locations: &locations, replace: false };
|
||||||
|
let mut committed = false;
|
||||||
|
let mut commit = |_: &Record| {
|
||||||
|
committed = true;
|
||||||
|
Ok(())
|
||||||
|
};
|
||||||
|
let result = install_at(app, &release, &asset, PackageKind::AppImage, Some(&installed), Some(&previous), &mut job, Some(&mut commit));
|
||||||
|
assert!(result.as_ref().is_err_and(|e| e.contains("checksum")), "{result:?}");
|
||||||
|
assert_eq!(std::fs::read(&old).unwrap(), b"the installed 0.4.0");
|
||||||
|
assert!(!phases.contains(&JobPhase::Installing), "{phases:?}");
|
||||||
|
assert!(!committed);
|
||||||
|
assert!(!download_path(&paths, &asset).exists());
|
||||||
|
assert_eq!(requests.lock().unwrap().len(), 2);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A running installed copy isn't replaced; the stop is cancelled in case the check stops
|
||||||
|
/// working. Windows finds a process by its file name, so the running test binary stands in for
|
||||||
|
/// the copy. Elsewhere `pgrep -f` matches the command line, so a probe runs with the copy's path.
|
||||||
|
#[test]
|
||||||
|
fn install_refuses_while_the_installed_copy_is_running() {
|
||||||
|
let dir = tempfile::tempdir().unwrap();
|
||||||
|
let paths = Paths::under(dir.path());
|
||||||
|
let http = Http::new(crate::TEST_AGENT).unwrap();
|
||||||
|
let phase = &mut |_| ();
|
||||||
|
let locations = InstallLocations::default();
|
||||||
|
let app = App::by_id("photocraft").unwrap();
|
||||||
|
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
|
||||||
|
let release = release::release_from_sums(app.repo, "v0.5.0", &[("a".repeat(64), name.to_string())]);
|
||||||
|
#[cfg(windows)]
|
||||||
|
let location = std::env::current_exe().unwrap().display().to_string();
|
||||||
|
#[cfg(unix)]
|
||||||
|
let location = dir.path().join("PhotoCraft.AppImage").display().to_string();
|
||||||
|
#[cfg(unix)]
|
||||||
|
let _probe = crate::process::tests::RunningProbe::start(&location);
|
||||||
|
assert!(process::is_running(Path::new(&location)), "{location} should be seen as running");
|
||||||
|
let installed = Installed { version: version::parse("0.4.0"), source: InstallSource::Launcher, location, package: None };
|
||||||
|
let stop = Stop::default();
|
||||||
|
stop.cancel();
|
||||||
|
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase, install_locations: &locations, replace: false };
|
||||||
|
let result = install_at(app, &release, &release.assets[0], PackageKind::AppImage, Some(&installed), None, &mut job, None);
|
||||||
|
assert!(result.as_ref().is_err_and(|e| e.contains("is running")), "{result:?}");
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(target_os = "linux")]
|
#[cfg(target_os = "linux")]
|
||||||
#[test]
|
#[test]
|
||||||
fn package_kinds_order_matches_launcher_format() {
|
fn package_kinds_order_matches_launcher_format() {
|
||||||
|
|||||||
@@ -351,16 +351,17 @@ fn pgrep_pattern(path: &str) -> String {
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
#[cfg(unix)]
|
#[cfg(unix)]
|
||||||
mod tests {
|
pub(crate) mod tests {
|
||||||
use std::io::{BufRead, BufReader};
|
use std::io::{BufRead, BufReader};
|
||||||
use std::process::Child;
|
use std::process::Child;
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
struct RunningProbe(Child);
|
/// A process whose command line is `path`, as `pgrep -f` sees it, until dropped.
|
||||||
|
pub(crate) struct RunningProbe(Child);
|
||||||
|
|
||||||
impl RunningProbe {
|
impl RunningProbe {
|
||||||
fn start(path: &str) -> Self {
|
pub(crate) fn start(path: &str) -> Self {
|
||||||
let child = command("/bin/sh")
|
let child = command("/bin/sh")
|
||||||
.args(["-c", "printf 'ready\\n'; read line", path])
|
.args(["-c", "printf 'ready\\n'; read line", path])
|
||||||
.stdin(Stdio::piped())
|
.stdin(Stdio::piped())
|
||||||
|
|||||||
@@ -370,6 +370,11 @@ mod tests {
|
|||||||
"https://github.com/storytold/photocraft/releases/download/v0.4.0/photocraft-0.5.0-macos-universal.dmg",
|
"https://github.com/storytold/photocraft/releases/download/v0.4.0/photocraft-0.5.0-macos-universal.dmg",
|
||||||
"https://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.4.0-macos-universal.dmg",
|
"https://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.4.0-macos-universal.dmg",
|
||||||
"https://github.com/storytold/photocraft/releases/download/%2e%2e/%2E%2E/%2e%2e/%2e%2e/other/repo/releases/download/v1/photocraft-0.5.0-macos-universal.dmg",
|
"https://github.com/storytold/photocraft/releases/download/%2e%2e/%2E%2E/%2e%2e/%2e%2e/other/repo/releases/download/v1/photocraft-0.5.0-macos-universal.dmg",
|
||||||
|
// The right path on another host, over plain http, or behind a look-alike host.
|
||||||
|
"https://evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
|
||||||
|
"http://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
|
||||||
|
"https://github.com.evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
|
||||||
|
"https://github.com@evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
|
||||||
] {
|
] {
|
||||||
assert!(parse_github_release(&api_json(url), REPO).unwrap().assets.is_empty(), "{url}");
|
assert!(parse_github_release(&api_json(url), REPO).unwrap().assets.is_empty(), "{url}");
|
||||||
}
|
}
|
||||||
@@ -383,6 +388,32 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Two entries for one asset name, one with the release's own link and one elsewhere: only the
|
||||||
|
/// release's own survives, in either order, so a later lookup by name can't pick the other.
|
||||||
|
/// Built from `SHA256SUMS.txt`, every entry for a repeated name gets that same single link.
|
||||||
|
#[test]
|
||||||
|
fn two_links_for_one_asset_keep_only_the_releases_own() {
|
||||||
|
let name = "photocraft-0.5.0-macos-universal.dmg";
|
||||||
|
let own = format!("https://github.com/storytold/photocraft/releases/download/v0.5.0/{name}");
|
||||||
|
let asset = |url: &str| format!(r#"{{"name":"{name}","size":123,"browser_download_url":"{url}","digest":"sha256:{HASH}"}}"#);
|
||||||
|
let elsewhere = "https://evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg";
|
||||||
|
for (first, second) in [(elsewhere, own.as_str()), (own.as_str(), elsewhere)] {
|
||||||
|
let json = format!(
|
||||||
|
r#"{{"tag_name":"v0.5.0","name":"PhotoCraft v0.5.0","html_url":"https://github.com/storytold/photocraft/releases/tag/v0.5.0",
|
||||||
|
"published_at":"2026-10-08T14:52:50Z","prerelease":false,"body":"Notes","assets":[{},{}]}}"#,
|
||||||
|
asset(first),
|
||||||
|
asset(second)
|
||||||
|
);
|
||||||
|
let r = parse_github_release(&json, REPO).unwrap();
|
||||||
|
assert_eq!(r.assets.len(), 1, "{first} {second}");
|
||||||
|
assert_eq!(r.assets[0].url, own);
|
||||||
|
}
|
||||||
|
|
||||||
|
let r = release_from_sums(REPO, "v0.5.0", &[("a".repeat(64), name.to_string()), ("b".repeat(64), name.to_string())]);
|
||||||
|
let links: Vec<&str> = r.assets.iter().filter(|a| a.name == name).map(|a| a.url.as_str()).collect();
|
||||||
|
assert!(!links.is_empty() && links.iter().all(|url| *url == own), "{links:?}");
|
||||||
|
}
|
||||||
|
|
||||||
/// A `+` in the tag or name (semver build metadata) is `%2B` in GitHub's download links, but a
|
/// A `+` in the tag or name (semver build metadata) is `%2B` in GitHub's download links, but a
|
||||||
/// plain `+` in the asset's name and in the `releases/latest` redirect, as on bell-sw/Liberica's
|
/// plain `+` in the asset's name and in the `releases/latest` redirect, as on bell-sw/Liberica's
|
||||||
/// releases. The API's assets, and those built from `SHA256SUMS.txt`, get the same link.
|
/// releases. The API's assets, and those built from `SHA256SUMS.txt`, get the same link.
|
||||||
|
|||||||
Reference in new issue
Block a user