src/action/base/fetch_and_unpack_nix_substituter.rs

@@ -133,7 +133,7 @@ impl FetchAndUnpackNixSubstituter {
                 self.require_sigs,
                 &self.trusted_keys,
                 substituter,
-                &output,
+                output,
                 &response
                     .bytes()
                     .await
@@ -304,14 +304,14 @@ impl Action for FetchAndUnpackNixSubstituter {
             // File format isn't documented anywhere but implementation is simple:
             // https://git.lix.systems/lix-project/lix/src/commit/d461cc1d7b2f489c3886f147166ba5b5e0e37541/src/libstore/store-api.cc#L846
             // Unwrapping because string can't fail methods in std::fmt::Write
-            write!(reginfo, "{}\n", output.full_path()).unwrap();
-            write!(reginfo, "sha256:{}\n", narinfo.nar_hash).unwrap();
-            write!(reginfo, "{}\n", narinfo.nar_size).unwrap();
+            writeln!(reginfo, "{}", output.full_path()).unwrap();
+            writeln!(reginfo, "sha256:{}", narinfo.nar_hash).unwrap();
+            writeln!(reginfo, "{}", narinfo.nar_size).unwrap();
             // Leave deriver empty, same as lix binary tarballs
             reginfo.push('\n');
-            write!(reginfo, "{}\n", narinfo.references.len()).unwrap();
+            writeln!(reginfo, "{}", narinfo.references.len()).unwrap();
             for reference in &narinfo.references {
-                write!(reginfo, "{}\n", reference.full_path()).unwrap();
+                writeln!(reginfo, "{}", reference.full_path()).unwrap();
             }
         }
 
@@ -348,7 +348,7 @@ fn parse_key(key: &str) -> Result<(String, VerifyingKey), SubstitutionError> {
 
     // seems to be the best way to handle keys both with and without padding
     let key_bytes = base64::engine::general_purpose::STANDARD_NO_PAD
-        .decode(key_base64.trim_end_matches("=").as_bytes())
+        .decode(key_base64.trim_end_matches('=').as_bytes())
         .map_err(|_| SubstitutionError::PublicKey)?;
 
     let key_array: [u8; ed25519_dalek::PUBLIC_KEY_LENGTH] = key_bytes
@@ -376,7 +376,7 @@ impl StorePath {
     /// The base32 hash part of a store path,
     /// e.g. `n50jk09x9hshwx1lh6k3qaiygc7yxbv9`
     pub fn digest(&self) -> &str {
-        &self.0.split_once('-').unwrap().0
+        self.0.split_once('-').unwrap().0
     }
 
     /// Full path of the output including STORE_DIR,
@@ -472,6 +472,17 @@ struct NarInfo {
 
 impl NarInfo {
     fn parse(substituter_url: &Url, contents: &bytes::Bytes) -> Result<Self, SubstitutionError> {
+        fn assign_unique_or_error<T>(
+            field: &mut Option<T>,
+            value: Result<T, SubstitutionError>,
+        ) -> Result<(), SubstitutionError> {
+            if field.replace(value?).is_some() {
+                Err(SubstitutionError::BadNarInfo)
+            } else {
+                Ok(())
+            }
+        }
+
         let mut store_path = None;
         let mut url = None;
         let mut compression = None;
@@ -481,6 +492,7 @@ impl NarInfo {
         let mut sig = None;
 
         for maybe_line in contents.lines() {
+            // Error if contents cannot be split into a list of line strings, probably if the NAR isn't valid utf-8
             let line = maybe_line.map_err(SubstitutionError::UndecodableNarInfo)?;
             let (tag, rest) = line
                 .split_once(':')
@@ -488,33 +500,46 @@ impl NarInfo {
             let value = rest.trim_start_matches(' ');
 
             match tag {
-                "StorePath" => store_path = StorePath::from_full_path(value),
-                "URL" => url = substituter_url.join(value).ok(),
-                "Compression" => compression = NarCompression::from_name(value),
-                "NarHash" => {
-                    nar_hash = {
-                        let (algorithm, digest) = value
-                            .split_once(':')
-                            .ok_or_else(|| SubstitutionError::BadNarInfo)?;
-                        if algorithm == "sha256" {
-                            Some(digest.to_string())
-                        } else {
-                            None
-                        }
-                    }
+                "StorePath" => {
+                    let path = StorePath::from_full_path(value)
+                        .ok_or_else(|| SubstitutionError::InvalidStorePath(value.into()));
+                    assign_unique_or_error(&mut store_path, path)?
                 },
-                "NarSize" => nar_size = u64::from_str_radix(value, 10).ok(),
+                "URL" => assign_unique_or_error(
+                    &mut url,
+                    substituter_url
+                        .join(value)
+                        .map_err(SubstitutionError::UrlParseError),
+                )?,
+                "Compression" => assign_unique_or_error(
+                    &mut compression,
+                    NarCompression::from_name(value).ok_or(SubstitutionError::BadNarInfo),
+                )?,
+                "NarHash" => {
+                    let (algorithm, digest) = value
+                        .split_once(':')
+                        .ok_or_else(|| SubstitutionError::BadNarInfo)?;
+                    if algorithm != "sha256" {
+                        Err(SubstitutionError::BadNarInfo)?
+                    }
+                    assign_unique_or_error(&mut nar_hash, Ok(digest.to_string()))?
+                },
+                "NarSize" => assign_unique_or_error(
+                    &mut nar_size,
+                    value.parse().map_err(|_| SubstitutionError::BadNarInfo),
+                )?,
                 "References" => {
-                    references = Some(if value.is_empty() {
+                    let refs = if value.is_empty() {
                         // split on empty strings still returns one value
-                        Vec::new()
+                        Ok(Vec::new())
                     } else {
                         value
                             .split(' ')
                             .map(StorePath::from_full_name)
                             .map(|value| value.ok_or_else(|| SubstitutionError::BadNarInfo))
-                            .collect::<Result<Vec<_>, _>>()?
-                    });
+                            .collect::<Result<Vec<_>, _>>()
+                    };
+                    assign_unique_or_error(&mut references, refs)?
                 },
                 "Sig" => {
                     let (signer, base64_signature) = value
@@ -524,8 +549,9 @@ impl NarInfo {
                         .decode(base64_signature)
                         .map_err(|_| SubstitutionError::BadNarInfo)?;
 
-                    sig = Some((signer.to_string(), signature))
+                    assign_unique_or_error(&mut sig, Ok((signer.to_string(), signature)))?
                 },
+                // Ignore any unmatched tags instead of erroring because there are some valid tags we are not parsing
                 _ => {},
             }
         }
@@ -602,12 +628,8 @@ impl NarInfo {
 static NIX32_CHARS: &[u8; 32] = b"0123456789abcdfghijklmnpqrsvwxyz";
 
 fn encode_nix32(input: &[u8]) -> String {
-    let length = if input.len() == 0 {
-        0
-    } else {
-        // ceil(input.len() * 8 / 5)
-        (input.len() * 8 - 1) / 5 + 1
-    };
+    // ceil(input.len() * 8 / 5)
+    let length = (input.len() * 8 + 4) / 5;
     let mut output = String::with_capacity(length);
 
     // nix32 hashes feel like they're a bug that stuck
@@ -639,11 +661,13 @@ fn encode_nix32(input: &[u8]) -> String {
     for char_no in 0..length {
         let bit_no = (length - char_no - 1) * 5;
         let byte_no = bit_no / 8;
-        let offset = bit_no % 8;
+        let bit_offset = bit_no % 8;
 
-        let next_byte = input.get(byte_no + 1).unwrap_or(&0);
-        let value = (input[byte_no] as u16 >> offset) | ((*next_byte as u16) << (8 - offset));
-        output.push(NIX32_CHARS[(value & 0x1f) as usize] as char);
+        let higher_order_byte = *input.get(byte_no + 1).unwrap_or(&0);
+        let lower_order_byte = input[byte_no];
+        let window = ((higher_order_byte as u16) << 8) | (lower_order_byte as u16);
+        let value = (window >> bit_offset) & 0b11111;
+        output.push(NIX32_CHARS[value as usize] as char);
     }
 
     output
@@ -674,6 +698,8 @@ pub enum SubstitutionError {
     NonexistantNarInfo(String),
     #[error("Invalid nix store path {0}")]
     InvalidStorePath(PathBuf),
+    #[error("Url in narinfo could not be parsed \"{0}\"")]
+    UrlParseError(url::ParseError),
 }
 
 impl From<SubstitutionError> for ActionErrorKind {