build: parse .dsc checksum fields through one shared, validating parser

The source-build pipeline and the binNMU metadata path had drifted into
two inline parsers with different acceptance rules: binary.rs filled
names from any line with a third column but partials only from
exactly-three-column lines, so a 4+ column Checksums line made
&partials[name] panic by map index. Both paths now share one parser
that accepts the modern 3-column and the legacy 5-column Files layout,
rejects anything else with an error naming the field and line, and all
remaining lookups go through .get() with a clear error instead of
indexing. Legacy 5-column Files md5s were previously attributed to the
section token instead of the file name.

src/build/binary.rs

@@ -18,6 +18,8 @@ use crate::debian::{
     ChecksumEntry, ControlInfo, FileChecksums, FilesList, parse_changelog_entry_from_str,
 };
 
+use super::parse_checksum_field;
+
 /// Digests of one artifact.
 #[derive(Debug, Clone, Default)]
 struct ArtifactHashes {
@@ -393,41 +395,29 @@ fn include_dsc_artifacts(
         .next()
         .ok_or_else(|| format!("'{dsc_name}' is empty"))?;
 
+    // Names and partial checksums are filled from the very same validated
+    // lines, so a listed name can never miss its checksum entry.
+    // Distribution order follows the Checksums fields (Checksums-Sha1 then
+    // Checksums-Sha256), like dpkg-genchanges; the `Files` field only
+    // supplements the md5 digests.
     let mut names: Vec<String> = Vec::new();
-    let mut partials: BTreeMap<String, PartialDscChecksums> = BTreeMap::new();
+    let mut partials: BTreeMap<String, super::PartialChecksum> = BTreeMap::new();
-    for field in ["Checksums-Sha1", "Checksums-Sha256"] {
+    for field in ["Checksums-Sha1", "Checksums-Sha256", "Files"] {
-        if let Some(value) = para.get(field) {
+        let Some(value) = para.get(field) else {
-            for line in value.lines() {
-                let tokens: Vec<&str> = line.split_whitespace().collect();
-                if tokens.len() != 3 {
             continue;
+        };
+        for cl in parse_checksum_field(field, value)
+            .map_err(|e| format!("cannot parse '{dsc_name}': {e}"))?
+        {
+            let slot = partials.entry(cl.name.clone()).or_default();
+            match field {
+                "Checksums-Sha1" => slot.sha1 = Some(cl.digest),
+                "Checksums-Sha256" => slot.sha256 = Some(cl.digest),
+                _ => slot.md5 = Some(cl.digest),
             }
-                let slot = partials.entry(tokens[2].to_string()).or_default();
+            slot.size = Some(cl.size);
-                if field == "Checksums-Sha1" {
+            if field != "Files" && !names.contains(&cl.name) {
-                    slot.sha1 = Some(tokens[0].to_string());
+                names.push(cl.name);
-                } else {
-                    slot.sha256 = Some(tokens[0].to_string());
-                }
-                slot.size = tokens[1].parse().ok().or(slot.size);
-            }
-        }
-    }
-    if let Some(files_value) = para.get("Files") {
-        for line in files_value.lines() {
-            let tokens: Vec<&str> = line.split_whitespace().collect();
-            if tokens.len() >= 3 {
-                let slot = partials.entry(tokens[2].to_string()).or_default();
-                slot.md5 = Some(tokens[0].to_string());
-                slot.size = tokens[1].parse().ok().or(slot.size);
-            }
-        }
-    }
-    for field in ["Checksums-Sha1", "Checksums-Sha256"] {
-        if let Some(value) = para.get(field) {
-            for line in value.lines() {
-                if let Some(name) = line.split_whitespace().nth(2) {
-                    names.push(name.to_string());
-                }
             }
         }
     }
@@ -451,7 +441,9 @@ fn include_dsc_artifacts(
         if name == dsc_name {
             continue;
         }
-        let p = &partials[name];
+        let p = partials
+            .get(name)
+            .ok_or_else(|| format!("file '{name}' listed in '{dsc_name}' has no checksum entry"))?;
         checksums.insert_entry(
             name,
             ChecksumEntry {
@@ -465,11 +457,105 @@ fn include_dsc_artifacts(
     Ok(())
 }
 
-/// Partially-known checksums taken from a `.dsc` checksum field.
+#[cfg(test)]
-#[derive(Debug, Default)]
+mod tests {
-struct PartialDscChecksums {
+    use super::*;
-    size: Option<u64>,
+
-    md5: Option<String>,
+    /// A minimal previous-version `.dsc` with a 3-column Checksums-Sha1
-    sha1: Option<String>,
+    /// field, a 4-column Checksums-Sha256 line and a 3-column `Files`.
-    sha256: Option<String>,
+    /// Regression: the old code filled `names` from any line with a third
+    /// column but `partials` only from exactly-3-column lines, so the
+    /// "bogus" name landed in `names` alone and `&partials["bogus"]`
+    /// panicked. It must produce a build error instead.
+    #[test]
+    fn dsc_four_column_checksum_line_errors_instead_of_panicking() {
+        let dsc_name = "hello_1.0-1.dsc";
+        let dsc = "\
+Format: 3.0 (native)
+Source: hello
+Binary: hello
+Architecture: any
+Version: 1.0-1
+Maintainer: A B <a@b.c>
+Checksums-Sha1:
+ aaa111 12 hello_1.0.orig.tar.xz
+Checksums-Sha256:
+ bbb222 12 bogus hello_1.0-1.debian.tar.xz
+Files:
+ ddd333 12 hello_1.0.orig.tar.xz
+";
+        let base = tempfile::tempdir().expect("tempdir");
+        std::fs::write(base.path().join(dsc_name), dsc).expect("write dsc");
+
+        let ctx = Arc::new(
+            crate::context::Context::new(crate::context::ContextConfig::Local).expect("context"),
+        );
+        let mut checksums = FileChecksums::new();
+        let err = include_dsc_artifacts(&ctx, base.path(), dsc_name, &mut checksums)
+            .expect_err("malformed Checksums-Sha256 line must fail the build");
+        let err = err.to_string();
+        assert!(err.contains("Checksums-Sha256"), "{err}");
+        assert!(err.contains("hello_1.0-1.debian.tar.xz"), "{err}");
+    }
+
+    /// Happy path: tarball entries are assembled from the Checksums fields
+    /// (sha1/sha256) and merged with the legacy 5-column `Files` md5, in
+    /// Checksums-Sha1 order, with the `.dsc` itself hashed fresh first.
+    #[test]
+    fn include_dsc_artifacts_merges_legacy_files_layout() {
+        let dsc_name = "hello_1.0-1.dsc";
+        let tarball = "hello_1.0.orig.tar.xz";
+        let dsc = "\
+Format: 3.0 (quilt)
+Source: hello
+Binary: hello
+Architecture: any
+Version: 1.0-1
+Maintainer: A B <a@b.c>
+Checksums-Sha1:
+ aaa111 12 hello_1.0.orig.tar.xz
+Checksums-Sha256:
+ bbb222 12 hello_1.0.orig.tar.xz
+Files:
+ ddd333 12 devel optional hello_1.0.orig.tar.xz
+";
+        let base = tempfile::tempdir().expect("tempdir");
+        std::fs::write(base.path().join(dsc_name), dsc).expect("write dsc");
+        std::fs::write(base.path().join(tarball), "tarball bytes").expect("write tarball");
+
+        let ctx = Arc::new(
+            crate::context::Context::new(crate::context::ContextConfig::Local).expect("context"),
+        );
+        let mut checksums = FileChecksums::new();
+        include_dsc_artifacts(&ctx, base.path(), dsc_name, &mut checksums)
+            .expect("valid dsc must parse");
+
+        let collected: Vec<(String, crate::debian::ChecksumEntry)> = checksums
+            .iter()
+            .map(|(k, e)| (k.clone(), e.clone()))
+            .collect();
+        assert_eq!(
+            collected
+                .iter()
+                .map(|(k, _)| k.as_str())
+                .collect::<Vec<_>>(),
+            vec![dsc_name, tarball],
+            ".dsc first, then Checksums-Sha1 order"
+        );
+
+        // The .dsc is hashed fresh from disk.
+        let dsc_entry = &collected[0].1;
+        assert_eq!(dsc_entry.size, dsc.len() as u64);
+        assert_eq!(dsc_entry.md5.len(), 32);
+        assert_eq!(dsc_entry.sha1.len(), 40);
+        assert_eq!(dsc_entry.sha256.len(), 64);
+
+        // The tarball reuses the .dsc-recorded digests, including the
+        // legacy 5-column `Files` md5 (section/priority skipped).
+        let tar_entry = &collected[1].1;
+        assert_eq!(tar_entry.size, 12);
+        assert_eq!(tar_entry.md5, "ddd333");
+        assert_eq!(tar_entry.sha1, "aaa111");
+        assert_eq!(tar_entry.sha256, "bbb222");
+    }
 }

src/build/mod.rs

@@ -384,33 +384,26 @@ pub fn run_source_build(
     let mut tarball_paths = Vec::new();
     let mut dsc_file_names: Vec<String> = Vec::new();
     let mut dsc_files: HashMap<String, PartialChecksum> = HashMap::new();
-    for field in ["Checksums-Sha1", "Checksums-Sha256"] {
+    // Distribution order follows the Checksums fields (Checksums-Sha1 then
-        if let Some(value) = dsc_para.get(field) {
+    // Checksums-Sha256), like dpkg-genchanges; the `Files` field only
-            for line in value.lines() {
+    // supplements the md5 digests.
-                let tokens: Vec<&str> = line.split_whitespace().collect();
+    for field in ["Checksums-Sha1", "Checksums-Sha256", "Files"] {
-                if tokens.len() != 3 {
+        let Some(value) = dsc_para.get(field) else {
             continue;
+        };
+        for cl in parse_checksum_field(field, value)
+            .map_err(|e| format!("cannot parse '{}': {e}", ref_dsc_path.display()))?
+        {
+            if !dsc_files.contains_key(&cl.name) {
+                dsc_file_names.push(cl.name.clone());
             }
-                if !dsc_files.contains_key(tokens[2]) {
+            let slot = dsc_files.entry(cl.name.clone()).or_default();
-                    dsc_file_names.push(tokens[2].to_string());
-                }
-                let slot = dsc_files.entry(tokens[2].to_string()).or_default();
             match field {
-                    "Checksums-Sha1" => slot.sha1 = Some(tokens[0].to_string()),
+                "Checksums-Sha1" => slot.sha1 = Some(cl.digest),
-                    _ => slot.sha256 = Some(tokens[0].to_string()),
+                "Checksums-Sha256" => slot.sha256 = Some(cl.digest),
-                }
+                _ => slot.md5 = Some(cl.digest),
-                slot.size = tokens[1].parse().ok().or(slot.size);
-            }
-        }
-    }
-    if let Some(files_value) = dsc_para.get("Files") {
-        for line in files_value.lines() {
-            let tokens: Vec<&str> = line.split_whitespace().collect();
-            if tokens.len() >= 3 {
-                let slot = dsc_files.entry(tokens[2].to_string()).or_default();
-                slot.md5 = Some(tokens[0].to_string());
-                slot.size = tokens[1].parse().ok().or(slot.size);
             }
+            slot.size = Some(cl.size);
         }
     }
     for name in &dsc_file_names {
@@ -426,7 +419,12 @@ pub fn run_source_build(
             )
             .into());
         }
-        let partial = &dsc_files[name];
+        let partial = dsc_files.get(name).ok_or_else(|| {
+            format!(
+                "file '{name}' listed in '{}' has no checksum entry",
+                ref_dsc_path.display()
+            )
+        })?;
         checksums.insert_entry(
             name,
             ChecksumEntry {
@@ -546,6 +544,63 @@ struct PartialChecksum {
     sha256: Option<String>,
 }
 
+/// One validated line of a `Checksums-Sha1` / `Checksums-Sha256` / `Files`
+/// field body.
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct ChecksumLine {
+    /// Digest as written in the first column.
+    digest: String,
+    /// File size in bytes.
+    size: u64,
+    /// File name (last column of the line).
+    name: String,
+}
+
+/// Parse the body of a `Checksums-Sha1` / `Checksums-Sha256` / `Files` field
+/// (one file per line) into validated entries. Shared by the source-build
+/// pipeline and the binary-only metadata generation so both accept exactly
+/// the same lines.
+///
+/// Both layouts are accepted, detected by column count:
+/// - 3 columns: `<digest> <size> <name>` (modern `Checksums-*` fields and
+///   the `Files` field of freshly built `.dsc`/`.changes`),
+/// - 5 columns: `<digest> <size> <section> <priority> <name>` (legacy
+///   `Files` fields, where the name is the last token).
+///
+/// Any other line (notably 4 columns) or a non-numeric size is a malformed
+/// line and yields an error naming `field` and the offending line.
+fn parse_checksum_field(field: &str, value: &str) -> Result<Vec<ChecksumLine>, String> {
+    let mut entries = Vec::new();
+    for line in value.lines() {
+        if line.trim().is_empty() {
+            continue;
+        }
+        let tokens: Vec<&str> = line.split_whitespace().collect();
+        let (digest, size, name) = match tokens.as_slice() {
+            [digest, size, name] => (*digest, *size, *name),
+            // Legacy 5-column `Files` layout: digest size section priority name.
+            [digest, size, _section, _priority, name] => (*digest, *size, *name),
+            _ => {
+                return Err(format!(
+                    "malformed '{field}' line (expected 'checksum size name' \
+                     or legacy 'checksum size section priority name', got {} \
+                     columns): '{line}'",
+                    tokens.len()
+                ));
+            }
+        };
+        let size: u64 = size.parse().map_err(|_| {
+            format!("malformed '{field}' line (size '{size}' is not a number): '{line}'")
+        })?;
+        entries.push(ChecksumLine {
+            digest: digest.to_string(),
+            size,
+            name: name.to_string(),
+        });
+    }
+    Ok(entries)
+}
+
 /// Run a build command in `cwd` with extra environment variables layered on
 /// top of the inherited environment.
 ///
@@ -633,6 +688,73 @@ mod tests {
         let v = DebianVersion::parse("1.0-2").unwrap();
         assert_eq!(v.no_epoch(), "1.0-2");
     }
+
+    #[test]
+    fn checksum_field_parses_three_column_lines() {
+        let entries = parse_checksum_field(
+            "Checksums-Sha256",
+            " aaa111 12 hello_1.0.orig.tar.xz\n bbb222 3 hello_1.0-1.debian.tar.xz",
+        )
+        .expect("valid 3-column field");
+        assert_eq!(
+            entries,
+            vec![
+                ChecksumLine {
+                    digest: "aaa111".into(),
+                    size: 12,
+                    name: "hello_1.0.orig.tar.xz".into(),
+                },
+                ChecksumLine {
+                    digest: "bbb222".into(),
+                    size: 3,
+                    name: "hello_1.0-1.debian.tar.xz".into(),
+                },
+            ]
+        );
+    }
+
+    #[test]
+    fn checksum_field_parses_legacy_five_column_files() {
+        // Old archive .dsc/.changes carry `Files` as
+        // md5 size section priority name.
+        let entries = parse_checksum_field(
+            "Files",
+            " d111 100 editors optional hello_1.0.orig.tar.gz\n \
+             d222 55 web optional hello_1.0-1.diff.gz",
+        )
+        .expect("valid legacy 5-column field");
+        assert_eq!(entries.len(), 2);
+        assert_eq!(entries[0].digest, "d111");
+        assert_eq!(entries[0].size, 100);
+        assert_eq!(entries[0].name, "hello_1.0.orig.tar.gz");
+        assert_eq!(entries[1].name, "hello_1.0-1.diff.gz");
+    }
+
+    #[test]
+    fn checksum_field_rejects_four_column_line() {
+        let err = parse_checksum_field(
+            "Checksums-Sha256",
+            " aaa111 12 hello_1.0.orig.tar.xz\n ccc333 12 bogus hello_1.0-1.debian.tar.xz",
+        )
+        .expect_err("4-column line must be rejected");
+        assert!(err.contains("Checksums-Sha256"), "{err}");
+        assert!(err.contains("hello_1.0-1.debian.tar.xz"), "{err}");
+    }
+
+    #[test]
+    fn checksum_field_rejects_non_numeric_size() {
+        let err = parse_checksum_field("Files", " d111 twelve hello.tar.xz")
+            .expect_err("non-numeric size must be rejected");
+        assert!(err.contains("twelve"), "{err}");
+    }
+
+    #[test]
+    fn checksum_field_skips_blank_lines() {
+        let entries = parse_checksum_field("Files", "\n d111 100 hello.tar.xz\n\n")
+            .expect("blank lines are ignored");
+        assert_eq!(entries.len(), 1);
+        assert_eq!(entries[0].name, "hello.tar.xz");
+    }
 }
 
 /// Differential tests: build synthetic (or real archive) source packages