diff --git a/crates/update/src/release.rs b/crates/update/src/release.rs index ca5419e4..3fbf785a 100644 --- a/crates/update/src/release.rs +++ b/crates/update/src/release.rs @@ -247,15 +247,32 @@ pub fn tag_from_redirect(location: &str, repo: &str) -> Option { (!rest.is_empty() && rest.len() <= 64 && rest.bytes().all(|b| b.is_ascii_alphanumeric() || b"._-+".contains(&b))).then(|| rest.to_string()) } -/// Reads `sha256sum` output: ` ` (or ` *`) per line. +/// Reads `sha256sum` output: ` ` (or ` *`) per line, one entry per name. +/// A line repeated as is counts once; a name listed with two different hashes is dropped, so the +/// file has no checksum and isn't installed, whichever line comes first. pub fn parse_sha256sums(text: &str) -> Vec<(String, String)> { - text.lines() - .filter_map(|line| { - let (hash, rest) = line.trim().split_once(char::is_whitespace)?; - let name = rest.trim_start().trim_start_matches('*'); - (is_hex64(hash) && valid_file_name(name)).then(|| (hash.to_ascii_lowercase(), name.to_string())) - }) - .collect() + let mut sums: Vec<(String, String)> = Vec::new(); + let mut conflicting: Vec = Vec::new(); + for line in text.lines() { + let Some((hash, rest)) = line.trim().split_once(char::is_whitespace) else { continue }; + let name = rest.trim_start().trim_start_matches('*'); + if !is_hex64(hash) || !valid_file_name(name) { + continue; + } + let hash = hash.to_ascii_lowercase(); + match sums.iter().find(|(_, n)| n == name) { + None => sums.push((hash, name.to_string())), + Some((h, _)) if *h == hash => {} + Some(_) => { + if !conflicting.iter().any(|n| n == name) { + log::warn!("SHA256SUMS.txt lists {name} with different checksums; ignoring it"); + conflicting.push(name.to_string()); + } + } + } + } + sums.retain(|(_, n)| !conflicting.contains(n)); + sums } /// A bare file name we'd be willing to save: no paths, no control characters. @@ -428,6 +445,26 @@ mod tests { assert_eq!(r.version, Some(Version::new(0, 5, 0))); } + #[test] + fn a_name_listed_twice_gives_one_asset_or_none() { + let other = "b".repeat(64); + // The same line twice (and the same hash in upper case) is one entry. + let sums = parse_sha256sums(&format!("{HASH} a.zip\n{HASH} *a.zip\n{} a.zip\n", HASH.to_ascii_uppercase())); + assert_eq!(sums, [(HASH.to_string(), "a.zip".to_string())]); + let r = release_from_sums(REPO, "v0.5.0", &sums); + assert_eq!(r.assets.iter().filter(|a| a.name == "a.zip").count(), 1); + // Two hashes for one name: the name is dropped in either line order, and the other + // files keep theirs. + for text in [format!("{HASH} a.zip\n{other} a.zip\n{HASH} b.zip\n"), format!("{other} a.zip\n{HASH} b.zip\n{HASH} a.zip\n")] { + let sums = parse_sha256sums(&text); + assert_eq!(sums, [(HASH.to_string(), "b.zip".to_string())], "{text}"); + let r = release_from_sums(REPO, "v0.5.0", &sums); + assert!(r.assets.iter().all(|a| a.name != "a.zip"), "{text}"); + } + // A third, matching line doesn't bring the name back. + assert!(parse_sha256sums(&format!("{HASH} a.zip\n{other} a.zip\n{HASH} a.zip\n")).is_empty()); + } + #[test] fn file_names_cannot_escape() { for bad in ["", "..", "a/b", "a\\b", ".hidden", "c:x", "x\u{0}"] {