From f72b35acfa878eabd3551fd2dde9d3c2b4d99d2e Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Wed, 16 Sep 2026 02:44:45 +0200 Subject: [PATCH] Handle malformed remote and edge-case data instead of panicking - distro_info: malformed CSV rows are skipped with a warning, dates that fail to parse become None, and all plain HTTP requests go through a shared reqwest client with connect/total timeouts - package_info: the Sources stanza iterator is iterative (a crafted index with many blank stanzas overflowed the stack), stanzas missing a Version are skipped, and failed series/pocket probes are summarized in the final 'not found' error instead of being silently dropped - pull: no double unwrap on the remote-derived artifact filename, an empty series list is an error, and streaming downloads get a per-request timeout - deb/cross: dpkg-architecture output parsing skips unexpected lines and its exit status is checked, as is dpkg --add-architecture - changelog: version increments parse as u64 with checked arithmetic (1.0-20250123123456 used to panic on the u32 parse) --- src/changelog.rs | 91 ++++++++++++----- src/deb/cross.rs | 127 +++++++++++++++++++---- src/distro_info.rs | 143 ++++++++++++++++++++++---- src/package_info.rs | 238 ++++++++++++++++++++++++++++++-------------- src/pull.rs | 32 ++++-- 5 files changed, 489 insertions(+), 142 deletions(-) diff --git a/src/changelog.rs b/src/changelog.rs index 9eb13db..6edc364 100644 --- a/src/changelog.rs +++ b/src/changelog.rs @@ -40,7 +40,7 @@ pub fn generate_entry( version.to_string() } else { // TODO: Pass these flags from CLI - compute_new_version(&old_version, false, false, false) + compute_new_version(&old_version, false, false, false)? }; let (maintainer_name, maintainer_email) = get_maintainer_info()?; @@ -68,7 +68,7 @@ fn compute_new_version( is_ubuntu: bool, is_rebuild: bool, is_nmu: bool, -) -> String { +) -> Result> { if is_ubuntu { return increment_suffix(old_version, "ubuntu"); } @@ -86,7 +86,7 @@ fn compute_new_version( } /// Increment a version number by 1, for a given suffix -fn increment_suffix(version: &str, suffix: &str) -> String { +fn increment_suffix(version: &str, suffix: &str) -> Result> { // If suffix is empty, we just look for trailing digits // If suffix is not empty, we look for suffix followed by digits @@ -100,19 +100,33 @@ fn increment_suffix(version: &str, suffix: &str) -> String { if let Some(caps) = re.captures(version) { let num_str = caps.get(1).unwrap().as_str(); - let num: u32 = num_str.parse().unwrap(); + // Parse as u64 so that large trailing numbers (e.g. date-based + // versions like '1.0-20250123123456', which do not fit in a u32) + // still increment normally + let num: u64 = num_str.parse().map_err(|_| { + format!( + "Cannot increment version '{version}': trailing number '{num_str}' \ + is too large to be incremented. Specify a version explicitly instead." + ) + })?; let range = caps.get(1).unwrap().range(); + let new_num = num.checked_add(1).ok_or_else(|| { + format!( + "Cannot increment version '{version}': trailing number {num} \ + is too large to be incremented. Specify a version explicitly instead." + ) + })?; let mut new_ver = version.to_string(); - new_ver.replace_range(range, &(num + 1).to_string()); - return new_ver; + new_ver.replace_range(range, &new_num.to_string()); + return Ok(new_ver); } // If pattern not found, append suffix + "1" // But if suffix is empty, we default to appending "-1" (standard Debian revision start) if suffix.is_empty() { - format!("{}-1", version) + Ok(format!("{}-1", version)) } else { - format!("{}{}{}", version, suffix, 1) + Ok(format!("{}{}{}", version, suffix, 1)) } } @@ -381,70 +395,101 @@ mod tests { fn test_compute_new_version() { // Debian upload assert_eq!( - compute_new_version("15.2.0-8", false, false, false), + compute_new_version("15.2.0-8", false, false, false).unwrap(), "15.2.0-9" ); assert_eq!( - compute_new_version("15.2.0-9", false, false, false), + compute_new_version("15.2.0-9", false, false, false).unwrap(), "15.2.0-10" ); // Ubuntu upload assert_eq!( - compute_new_version("15.2.0-9", true, false, false), + compute_new_version("15.2.0-9", true, false, false).unwrap(), "15.2.0-9ubuntu1" ); assert_eq!( - compute_new_version("15.2.0-9ubuntu1", true, false, false), + compute_new_version("15.2.0-9ubuntu1", true, false, false).unwrap(), "15.2.0-9ubuntu2" ); // No change rebuild assert_eq!( - compute_new_version("15.2.0-9", false, true, false), + compute_new_version("15.2.0-9", false, true, false).unwrap(), "15.2.0-9build1" ); assert_eq!( - compute_new_version("15.2.0-9build1", false, true, false), + compute_new_version("15.2.0-9build1", false, true, false).unwrap(), "15.2.0-9build2" ); // Rebuild of Ubuntu version assert_eq!( - compute_new_version("15.2.0-9ubuntu1", false, true, false), + compute_new_version("15.2.0-9ubuntu1", false, true, false).unwrap(), "15.2.0-9ubuntu1build1" ); // NMU // Native - assert_eq!(compute_new_version("1.0", false, false, true), "1.0+nmu1"); assert_eq!( - compute_new_version("1.0+nmu1", false, false, true), + compute_new_version("1.0", false, false, true).unwrap(), + "1.0+nmu1" + ); + assert_eq!( + compute_new_version("1.0+nmu1", false, false, true).unwrap(), "1.0+nmu2" ); // Non-native - assert_eq!(compute_new_version("1.0-1", false, false, true), "1.0-1.1"); assert_eq!( - compute_new_version("1.0-1.1", false, false, true), + compute_new_version("1.0-1", false, false, true).unwrap(), + "1.0-1.1" + ); + assert_eq!( + compute_new_version("1.0-1.1", false, false, true).unwrap(), "1.0-1.2" ); // NMU of NMU? assert_eq!( - compute_new_version("1.0-1.2", false, false, true), + compute_new_version("1.0-1.2", false, false, true).unwrap(), "1.0-1.3" ); // Native package uploads - assert_eq!(compute_new_version("1.0", false, false, false), "1.1"); - assert_eq!(compute_new_version("1.0.5", false, false, false), "1.0.6"); assert_eq!( - compute_new_version("20241126", false, false, false), + compute_new_version("1.0", false, false, false).unwrap(), + "1.1" + ); + assert_eq!( + compute_new_version("1.0.5", false, false, false).unwrap(), + "1.0.6" + ); + assert_eq!( + compute_new_version("20241126", false, false, false).unwrap(), "20241127" ); } + #[test] + fn test_compute_new_version_large_trailing_number() { + // Date-based versions with a trailing number larger than u32::MAX + // must increment normally (they fit in a u64) + assert_eq!( + compute_new_version("1.0-20250123123456", false, false, false).unwrap(), + "1.0-20250123123457" + ); + + // A number that does not even fit in a u64 yields a clear error + // instead of panicking + let err = compute_new_version("1.0-99999999999999999999999999", false, false, false); + assert!(err.is_err()); + + // u64::MAX itself cannot be incremented + let err = compute_new_version("1.0-18446744073709551615", false, false, false); + assert!(err.is_err()); + } + #[test] fn test_get_maintainer_info() { // Test with env vars diff --git a/src/deb/cross.rs b/src/deb/cross.rs index b1848af..004b2bd 100644 --- a/src/deb/cross.rs +++ b/src/deb/cross.rs @@ -1,24 +1,19 @@ use crate::context::Context; +use log::debug; use std::collections::HashMap; use std::error::Error; use std::sync::Arc; -/// Set environment variables for cross-compilation -pub fn setup_environment( - env: &mut HashMap, - arch: &str, - ctx: Arc, -) -> Result<(), Box> { - let dpkg_architecture = String::from_utf8( - ctx.command("dpkg-architecture") - .arg("-a") - .arg(arch) - .output()? - .stdout, - )?; +/// Parse 'dpkg-architecture' output (KEY=value lines) into a set of +/// environment variables. Unexpected lines (e.g. warnings on stderr leaking +/// into stdout) are skipped instead of causing a failure. +fn parse_dpkg_architecture_output(output: &str, env: &mut HashMap) { let env_var_regex = regex::Regex::new(r"(?.*)=(?.*)").unwrap(); - for l in dpkg_architecture.lines() { - let capture = env_var_regex.captures(l).unwrap(); + for l in output.lines() { + let Some(capture) = env_var_regex.captures(l) else { + debug!("Skipping unexpected dpkg-architecture output line: '{l}'"); + continue; + }; let key = capture.name("key").unwrap().as_str().to_string(); let value = capture.name("value").unwrap().as_str().to_string(); @@ -28,6 +23,45 @@ pub fn setup_environment( env.insert("CROSS_COMPILE".to_string(), format!("{value}-")); } } +} + +/// Set environment variables for cross-compilation +pub fn setup_environment( + env: &mut HashMap, + arch: &str, + ctx: Arc, +) -> Result<(), Box> { + let output = ctx + .command("dpkg-architecture") + .arg("-a") + .arg(arch) + .output() + .map_err(|e| { + format!( + "Failed to run 'dpkg-architecture -a {arch}': {e}. \ + Is 'dpkg-dev' installed?" + ) + })?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!( + "'dpkg-architecture -a {}' failed with status: {}.{}", + arch, + output.status, + if stderr.trim().is_empty() { + String::new() + } else { + format!("\ndpkg-architecture output:\n{}", stderr.trim()) + } + ) + .into()); + } + + let dpkg_architecture = String::from_utf8(output.stdout) + .map_err(|e| format!("Invalid UTF-8 in dpkg-architecture output: {e}"))?; + parse_dpkg_architecture_output(&dpkg_architecture, env); + env.insert("DEB_BUILD_PROFILES".to_string(), "cross".to_string()); Ok(()) @@ -44,10 +78,19 @@ pub fn ensure_repositories( let local_arch = crate::get_current_arch(); // Add target ('host') architecture - ctx.command("dpkg") + let status = ctx + .command("dpkg") .arg("--add-architecture") .arg(arch) - .status()?; + .status() + .map_err(|e| format!("Failed to run 'dpkg --add-architecture {arch}': {e}"))?; + if !status.success() { + return Err(format!( + "'dpkg --add-architecture {}' failed with status: {}", + arch, status + ) + .into()); + } // Check if we are on Ubuntu let os_release = String::from_utf8(ctx.command("cat").arg("/etc/os-release").output()?.stdout)?; @@ -142,3 +185,53 @@ pub fn ensure_repositories( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_dpkg_architecture_output() { + let output = "DEB_BUILD_ARCH=amd64\n\ + DEB_HOST_ARCH=arm64\n\ + DEB_HOST_GNU_TYPE=aarch64-linux-gnu\n"; + + let mut env = HashMap::new(); + parse_dpkg_architecture_output(output, &mut env); + + assert_eq!(env.get("DEB_BUILD_ARCH").map(String::as_str), Some("amd64")); + assert_eq!(env.get("DEB_HOST_ARCH").map(String::as_str), Some("arm64")); + assert_eq!( + env.get("DEB_HOST_GNU_TYPE").map(String::as_str), + Some("aarch64-linux-gnu") + ); + // Derived variable for the GNU type + assert_eq!( + env.get("CROSS_COMPILE").map(String::as_str), + Some("aarch64-linux-gnu-") + ); + } + + #[test] + fn test_parse_dpkg_architecture_output_skips_unexpected_lines() { + // Unexpected lines (warnings on stdout, empty lines) must be skipped + // instead of panicking + let output = "dpkg-architecture: warning: something odd happened\n\ + \n\ + DEB_HOST_GNU_TYPE=arm-linux-gnueabihf\n\ + not an environment variable assignment\n"; + + let mut env = HashMap::new(); + parse_dpkg_architecture_output(output, &mut env); + + assert_eq!( + env.get("DEB_HOST_GNU_TYPE").map(String::as_str), + Some("arm-linux-gnueabihf") + ); + assert_eq!( + env.get("CROSS_COMPILE").map(String::as_str), + Some("arm-linux-gnueabihf-") + ); + assert_eq!(env.len(), 2); + } +} diff --git a/src/distro_info.rs b/src/distro_info.rs index b5c1ac5..845e490 100644 --- a/src/distro_info.rs +++ b/src/distro_info.rs @@ -3,6 +3,7 @@ use lazy_static::lazy_static; use serde::Deserialize; use std::error::Error; use std::path::Path; +use std::time::Duration; #[derive(Debug, Clone)] /// Information about a specific distribution series @@ -13,8 +14,8 @@ pub struct SeriesInformation { pub codename: String, /// Series version as numbers pub version: Option, - /// Series creation date - pub created: NaiveDate, + /// Series creation date (absent if missing or invalid in the CSV data) + pub created: Option, /// Series release date pub release: Option, /// Series end-of-life date @@ -42,7 +43,45 @@ struct Data { const DATA_YAML: &str = include_str!("../distro_info.yml"); lazy_static! { - static ref DATA: Data = serde_yaml::from_str(DATA_YAML).unwrap(); + // The YAML is include_str!'d at compile time and statically valid; if it + // ever failed to parse it would be a build-time bug that cannot be + // recovered from at runtime, so panicking here is acceptable. + static ref DATA: Data = serde_yaml::from_str(DATA_YAML) + .expect("built-in distro_info.yml data is statically valid and must parse"); + + // Shared HTTP client used for all outgoing plain requests: timeouts keep + // a hanging remote (connect or transfer) from stalling pkh indefinitely. + static ref HTTP_CLIENT: reqwest::Client = reqwest::Client::builder() + .connect_timeout(Duration::from_secs(10)) + .timeout(Duration::from_secs(30)) + .build() + .expect("building the shared HTTP client with static options cannot fail"); +} + +/// Shared HTTP client with a connect timeout (10s) and a total request +/// timeout (30s), to be used for all outgoing plain HTTP(S) requests +pub(crate) fn http_client() -> &'static reqwest::Client { + &HTTP_CLIENT +} + +/// Parse an optional '%Y-%m-%d' date from a CSV cell, warning instead of +/// panicking on invalid remote data +fn parse_optional_date(value: Option<&str>, series: &str, field: &str) -> Option { + value.and_then( + |date_str| match NaiveDate::parse_from_str(date_str, "%Y-%m-%d") { + Ok(date) => Some(date), + Err(e) => { + log::warn!( + "Invalid '{}' date '{}' for series '{}': {}. Ignoring the date.", + field, + date_str, + series, + e + ); + None + } + }, + ) } fn parse_series_csv(content: &str) -> Result, Box> { @@ -79,24 +118,39 @@ fn parse_series_csv(content: &str) -> Result, Box record, + Err(e) => { + log::warn!("Skipping malformed series CSV row: {}", e); + continue; + } + }; + + // Rows missing essential identification fields are skipped: they + // cannot be used nor reported meaningfully. Dates, on the other + // hand, are all optional in the model, so a bad date keeps the row. + let Some(series) = record.get(series_idx).filter(|s| !s.is_empty()) else { + log::warn!( + "Skipping series CSV row without a 'series' value: {:?}", + record + ); + continue; + }; + let Some(codename) = record.get(codename_idx).filter(|s| !s.is_empty()) else { + log::warn!( + "Skipping series CSV row for series '{}' without a 'codename' value", + series + ); + continue; + }; let version = record.get(version_idx).map(|s| s.to_string()); - let created = record - .get(created_idx) - .map(|date_str| NaiveDate::parse_from_str(date_str, "%Y-%m-%d").unwrap()) - .unwrap(); - let release = record - .get(release_idx) - .map(|date_str| NaiveDate::parse_from_str(date_str, "%Y-%m-%d").unwrap()); - let eol = record - .get(eol_idx) - .map(|date_str| NaiveDate::parse_from_str(date_str, "%Y-%m-%d").unwrap()); + let created = parse_optional_date(record.get(created_idx), series, "created"); + let release = parse_optional_date(record.get(release_idx), series, "release"); + let eol = parse_optional_date(record.get(eol_idx), series, "eol"); series_info_list.push(SeriesInformation { - series, - codename, + series: series.to_string(), + codename: codename.to_string(), version, created, release, @@ -134,7 +188,9 @@ pub async fn get_ordered_series(dist: &str) -> Result, Bo ) })? } else { - reqwest::get(series_info.network.as_str()) + http_client() + .get(series_info.network.as_str()) + .send() .await? .text() .await? @@ -324,7 +380,7 @@ pub async fn get_components( let url = get_release_url(base_url, series, pocket); log::debug!("Fetching Release file from: {}", url); - let content = reqwest::get(&url).await?.text().await?; + let content = http_client().get(&url).send().await?.text().await?; for line in content.lines() { if line.starts_with("Components:") @@ -359,7 +415,9 @@ pub async fn get_debian_series_number(series: &str) -> Result, Bo ) })? } else { - reqwest::get(series_info.network.as_str()) + http_client() + .get(series_info.network.as_str()) + .send() .await? .text() .await? @@ -395,6 +453,49 @@ pub async fn get_debian_series_number(series: &str) -> Result, Bo mod tests { use super::*; + #[test] + fn test_parse_series_csv_malformed_rows() { + // A short row (missing 'codename') is skipped, a row with an invalid + // 'created' date is kept without a date, and invalid 'release'/'eol' + // dates become None: none of this may panic on remote data + let csv_data = "series,codename,version,created,release,eol\n\ + noble,Noble N,24.04,2023-10-26,2024-04-25,2029-04-25\n\ + lonely\n\ + badbad,Bad B,1.0,not-a-date,2020-01-01,also-bad\n\ + sid,sid,unstable,1999-01-01,,\n"; + + let series = parse_series_csv(csv_data).unwrap(); + + // Rows are returned most recent first (the parser reverses the list), + // with the malformed 'lonely' row skipped entirely + let names: Vec<&str> = series.iter().map(|s| s.series.as_str()).collect(); + assert_eq!(names, vec!["sid", "badbad", "noble"]); + + let noble = &series[2]; + assert_eq!(noble.codename, "Noble N"); + assert_eq!(noble.version.as_deref(), Some("24.04")); + assert_eq!( + noble.created, + Some(NaiveDate::from_ymd_opt(2023, 10, 26).unwrap()) + ); + assert_eq!( + noble.release, + Some(NaiveDate::from_ymd_opt(2024, 4, 25).unwrap()) + ); + assert_eq!( + noble.eol, + Some(NaiveDate::from_ymd_opt(2029, 4, 25).unwrap()) + ); + + let badbad = &series[1]; + assert_eq!(badbad.created, None); + assert_eq!( + badbad.release, + Some(NaiveDate::from_ymd_opt(2020, 1, 1).unwrap()) + ); + assert_eq!(badbad.eol, None); + } + #[test] fn test_get_dist_pockets_order() { // Without an explicit pocket, packages are searched in this order: diff --git a/src/package_info.rs b/src/package_info.rs index 37cc99d..03cfce4 100644 --- a/src/package_info.rs +++ b/src/package_info.rs @@ -190,82 +190,98 @@ impl Iterator for DebianSources { type Item = PackageStanza; fn next(&mut self) -> Option { - let stanza = self.splitted_sources.next()?; + // Iterate over stanzas in a loop: package-less (blank) stanzas are + // skipped without recursion, so that a crafted index with many + // consecutive blank stanzas cannot blow the stack + loop { + let stanza = self.splitted_sources.next()?; - // Parse stanza into a hashmap of strings, the fields - let mut fields: HashMap = HashMap::new(); - let mut current_key = String::new(); + // Parse stanza into a hashmap of strings, the fields + let mut fields: HashMap = HashMap::new(); + let mut current_key = String::new(); - for line in stanza.lines() { - if line.is_empty() { + for line in stanza.lines() { + if line.is_empty() { + continue; + } + + if line.starts_with(' ') || line.starts_with('\t') { + // Continuation line + if let Some(val) = fields.get_mut(¤t_key) { + val.push('\n'); + val.push_str(line.trim()); + } + } else if let Some((key, value)) = line.split_once(':') { + current_key = key.trim().to_string(); + fields.insert(current_key.clone(), value.trim().to_string()); + } + } + + let Some(package) = fields.get("Package") else { + // Skip empty stanza continue; - } + }; + let package = package.to_string(); - if line.starts_with(' ') || line.starts_with('\t') { - // Continuation line - if let Some(val) = fields.get_mut(¤t_key) { - val.push('\n'); - val.push_str(line.trim()); - } - } else if let Some((key, value)) = line.split_once(':') { - current_key = key.trim().to_string(); - fields.insert(current_key.clone(), value.trim().to_string()); - } - } + // A stanza without a version is malformed remote data: skip it + // rather than panicking + let Some(version) = fields.get("Version") else { + debug!( + "Skipping malformed stanza for package '{}' without a 'Version' field", + package + ); + continue; + }; + let version = version.to_string(); - let pkg = fields.get("Package"); - if pkg.is_none() { - // Skip empty stanza - return self.next(); - } - - // Parse package files. - // Prefer the strongest available checksum field: Checksums-Sha256, - // then Checksums-Sha512, then the legacy 'Files' (MD5) field. - // Some archives (e.g. the Ubuntu development series) no longer ship - // Checksums-Sha256, so falling back is required to keep working. - let mut files = Vec::new(); - let (checksum_field, algo) = if fields.contains_key("Checksums-Sha256") { - ("Checksums-Sha256", ChecksumAlgo::Sha256) - } else if fields.contains_key("Checksums-Sha512") { - ("Checksums-Sha512", ChecksumAlgo::Sha512) - } else { - ("Files", ChecksumAlgo::Md5) - }; - if let Some(checksums) = fields.get(checksum_field) { - for line in checksums.lines() { - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 3 { - files.push(FileEntry { - checksum: parts[0].to_string(), - size: parts[1].parse().unwrap_or(0), - name: parts[2].to_string(), - checksum_algo: algo, - }); + // Parse package files. + // Prefer the strongest available checksum field: Checksums-Sha256, + // then Checksums-Sha512, then the legacy 'Files' (MD5) field. + // Some archives (e.g. the Ubuntu development series) no longer ship + // Checksums-Sha256, so falling back is required to keep working. + let mut files = Vec::new(); + let (checksum_field, algo) = if fields.contains_key("Checksums-Sha256") { + ("Checksums-Sha256", ChecksumAlgo::Sha256) + } else if fields.contains_key("Checksums-Sha512") { + ("Checksums-Sha512", ChecksumAlgo::Sha512) + } else { + ("Files", ChecksumAlgo::Md5) + }; + if let Some(checksums) = fields.get(checksum_field) { + for line in checksums.lines() { + let parts: Vec<&str> = line.split_whitespace().collect(); + if parts.len() >= 3 { + files.push(FileEntry { + checksum: parts[0].to_string(), + size: parts[1].parse().unwrap_or(0), + name: parts[2].to_string(), + checksum_algo: algo, + }); + } } } + + // Parse Vcs-Git field: it may contain just a URL, or URL followed by -b + // e.g., "https://salsa.debian.org/science-team/paraview.git -b debian/latest" + let vcs_git = fields.get("Vcs-Git").map(|vcs| { + // Split on whitespace and take the first part (the URL) + // The URL should not contain spaces, so this is safe + vcs.split_whitespace().next().unwrap_or(vcs).to_string() + }); + + return Some(PackageStanza { + package, + version, + directory: fields.get("Directory").cloned().unwrap_or_default(), + format: fields + .get("Format") + .cloned() + .unwrap_or_else(|| "1.0".to_string()), + vcs_git, + vcs_browser: fields.get("Vcs-Browser").cloned(), + files, + }); } - - // Parse Vcs-Git field: it may contain just a URL, or URL followed by -b - // e.g., "https://salsa.debian.org/science-team/paraview.git -b debian/latest" - let vcs_git = fields.get("Vcs-Git").map(|vcs| { - // Split on whitespace and take the first part (the URL) - // The URL should not contain spaces, so this is safe - vcs.split_whitespace().next().unwrap_or(vcs).to_string() - }); - - Some(PackageStanza { - package: fields.get("Package").unwrap().to_string(), - version: fields.get("Version").unwrap().to_string(), - directory: fields.get("Directory").cloned().unwrap_or_default(), - format: fields - .get("Format") - .cloned() - .unwrap_or_else(|| "1.0".to_string()), - vcs_git, - vcs_browser: fields.get("Vcs-Browser").cloned(), - files, - }) } } @@ -361,21 +377,28 @@ async fn get( } debug!("Found components: {:?}", components); + // Collect the failures of individual fetch attempts so that, if the + // package is not found, the final error explains what actually went + // wrong instead of misleadingly claiming a plain 'not found' + let mut fetch_errors: Vec = Vec::new(); + for component in components { let url = crate::distro_info::get_sources_url(&base_url, series, pocket, &component); debug!("Fetching sources from: {}", url); - let response = match reqwest::get(&url).await { + let response = match crate::distro_info::http_client().get(&url).send().await { Ok(resp) => resp, Err(e) => { debug!("Failed to fetch {}: {}", url, e); + fetch_errors.push(format!("{suite}/{component}: {}", e)); continue; } }; if !response.status().is_success() { debug!("Failed to fetch {}: status {}", url, response.status()); + fetch_errors.push(format!("{suite}/{component}: HTTP {}", response.status())); continue; } @@ -417,9 +440,14 @@ async fn get( } } + let details = if fetch_errors.is_empty() { + String::new() + } else { + format!(" (last errors: {})", fetch_errors.join("; ")) + }; Err(format!( - "Package '{}' not found in {}/{}", - package_name, dist, series + "Package '{}' not found in {}/{}{}", + package_name, dist, series, details ) .into()) } @@ -436,6 +464,11 @@ async fn find_package( ) -> Result> { let series_list = crate::distro_info::get_ordered_series_name(dist).await?; + // Collect the failures of the individual series/pocket probes so that, + // if nothing is found, the final error summarizes what went wrong + // (e.g. network errors, HTTP statuses) instead of a bare 'not found' + let mut attempt_errors: Vec = Vec::new(); + for (i, series) in series_list.iter().enumerate() { if let Some(cb) = progress { cb("", &format!("Checking {}...", series), i, series_list.len()); @@ -486,13 +519,32 @@ async fn find_package( if e.downcast_ref::().is_some() { return Err(e); } + // Remember the failure for the final error message, and + // keep probing the other series/pockets + let suite = if p.is_empty() { + series.clone() + } else { + format!("{series}-{p}") + }; + attempt_errors.push(format!("{}: {}", suite, e)); continue; } } } } - Err(format!("Package '{}' not found.", package_name).into()) + // Keep only the last few attempts so the message stays readable + if attempt_errors.len() > 5 { + let drain_to = attempt_errors.len() - 5; + attempt_errors.drain(..drain_to); + } + let details = if attempt_errors.is_empty() { + String::new() + } else { + format!(" (last errors: {})", attempt_errors.join("; ")) + }; + + Err(format!("Package '{}' not found.{}", package_name, details).into()) } /// Fetch the 'Release' file at the root of a flat repository, and return its suite name @@ -502,7 +554,7 @@ async fn find_package( /// read from the 'Codename' field, falling back to 'Suite'. async fn get_flat_repo_series(base_url: &str) -> Result> { let url = format!("{}/Release", base_url.trim_end_matches('/')); - let response = reqwest::get(&url).await?; + let response = crate::distro_info::http_client().get(&url).send().await?; if !response.status().is_success() { return Err(format!( "No Release file at '{}' (HTTP {}) - is '{}' the suite URL of a flat repository? \ @@ -540,7 +592,7 @@ async fn get_flat_repo_sources( let mut errors = Vec::new(); for name in ["Sources.xz", "Sources.gz", "Sources"] { let url = format!("{base}/{name}"); - match reqwest::get(&url).await { + match crate::distro_info::http_client().get(&url).send().await { Ok(response) if response.status().is_success() => { let data = response.bytes().await?.to_vec(); @@ -860,6 +912,44 @@ Directory: pool/main/h/hello assert_eq!(info.version, "1.0"); } + #[test] + fn test_parse_sources_many_blank_stanzas() { + // A crafted index with many consecutive package-less stanzas must be + // iterated without recursion: 100k blank stanzas would overflow the + // stack with the old recursive 'return self.next()' implementation + let mut data = String::new(); + for _ in 0..100_000 { + data.push_str("Not-Really-Package: x\n\n"); + } + data.push_str("Package: hello\nVersion: 1.0\n"); + + let info = parse_sources(data.as_bytes(), "hello", None) + .unwrap() + .unwrap(); + assert_eq!(info.package, "hello"); + assert_eq!(info.version, "1.0"); + } + + #[test] + fn test_parse_sources_stanza_without_version() { + // A stanza with a 'Package' but no 'Version' field is malformed + // remote data: it must be skipped rather than panic + let data = "Package: noversion\nDirectory: pool/main/n/noversion\n\n\ + Package: hello\nVersion: 1.0\n"; + + let info = parse_sources(data.as_bytes(), "hello", None) + .unwrap() + .unwrap(); + assert_eq!(info.package, "hello"); + assert_eq!(info.version, "1.0"); + + assert!( + parse_sources(data.as_bytes(), "noversion", None) + .unwrap() + .is_none() + ); + } + #[tokio::test] async fn test_find_package_fallback() { // python2.7 is in bullseye but not above diff --git a/src/pull.rs b/src/pull.rs index 205c21d..627e397 100644 --- a/src/pull.rs +++ b/src/pull.rs @@ -310,8 +310,14 @@ async fn download_file_checksum( target_dir: &Path, progress: ProgressCallback<'_>, ) -> Result<(), Box> { - // Download with reqwest - let response = reqwest::get(url).await?; + // Download with the shared client (connect timeout). Large orig tarballs + // can legitimately take longer than the client's default total timeout, + // so use a generous per-request timeout for streaming downloads + let response = crate::distro_info::http_client() + .get(url) + .timeout(std::time::Duration::from_secs(30 * 60)) + .send() + .await?; if !response.status().is_success() { return Err(format!("Failed to download '{}' : {}", url, response.status()).into()); } @@ -322,7 +328,12 @@ async fn download_file_checksum( let mut index = 0; // Target file: extract file name from URL - let filename = Path::new(url).file_name().unwrap().to_str().unwrap(); + let filename = Path::new(url) + .file_name() + .and_then(|f| f.to_str()) + .ok_or_else(|| { + format!("Could not determine a file name from URL '{url}' to download the package file") + })?; let path = target_dir.join(filename); let mut file = File::create(path)?; @@ -650,10 +661,17 @@ pub async fn pull( // we target the development branch, i.e. the default branch // Only use Ubuntu-specific branch naming if the VCS is from Launchpad let is_launchpad_vcs = url.contains("launchpad.net"); - let branch_name = if crate::distro_info::get_ordered_series_name(package_info.dist.as_str()) - .await?[0] - != *series - { + let series_list = + crate::distro_info::get_ordered_series_name(package_info.dist.as_str()).await?; + let latest_series = series_list.first().ok_or_else(|| { + format!( + "No series information available for distribution '{}', \ + cannot determine its development series to select the git branch. \ + The 'distro-info' package provides this data.", + package_info.dist + ) + })?; + let branch_name = if latest_series != series { if package_info.dist == "ubuntu" && is_launchpad_vcs { Some(format!("{}/{}", package_info.dist, series)) } else {