vhaudiquet/pkh
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Handle malformed remote and edge-case data instead of panicking

Browse Source 
- 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)
This commit is contained in:
vhaudiquet
2026-09-16 02:44:45 +02:00
parent 6a5c5a7106
commit f72b35acfa
5 changed files with 489 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/package_info.rs
+164 -74
View File
@@ -190,82 +190,98 @@ impl Iterator for DebianSources {
type Item = PackageStanza;
fn next(&mut self) -> Option<Self::Item> {
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<String, String> = HashMap::new();
let mut current_key = String::new();
// Parse stanza into a hashmap of strings, the fields
let mut fields: HashMap<String, String> = 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(&current_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(&current_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 <branch>
// 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 <branch>
// 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<String> = 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<PackageInfo, Box<dyn Error>> {
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<String> = 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::<release::VerifyError>().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<String, Box<dyn Error>> {
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