Fix registry download URL template expansion

Author: syphar

src/config.rs

@@ -1,10 +1,25 @@
 use crate::dirs::crate_prefix;
 use serde_derive::Deserialize;
 
+/// marker / template vars for generating download urls.
+const CRATE_MARKER: &str = "{crate}";
+const VERSION_MARKER: &str = "{version}";
+const PREFIX_MARKER: &str = "{prefix}";
+const LOWERPREFIX_MARKER: &str = "{lowerprefix}";
+const SHA256_CHECKSUM_MARKER: &str = "{sha256-checksum}";
+const DOWNLOAD_URL_MARKERS: [&str; 5] = [
+    CRATE_MARKER,
+    VERSION_MARKER,
+    PREFIX_MARKER,
+    LOWERPREFIX_MARKER,
+    SHA256_CHECKSUM_MARKER,
+];
+
 /// Global configuration of an index, reflecting the [contents of config.json](https://doc.rust-lang.org/cargo/reference/registries.html#index-format).
 #[derive(Clone, Debug, Deserialize)]
 pub struct IndexConfig {
-    /// Pattern for creating download URLs. Use [`IndexConfig::download_url`] instead.
+    /// Pattern for creating download URLs. Use [`IndexConfig::download_url`] or
+    /// [`IndexConfig::download_url_with_checksum`] instead.
     pub dl: String,
     /// Base URL for publishing, etc.
     pub api: Option<String>,
@@ -14,13 +29,27 @@ impl IndexConfig {
     /// Get the URL from where the specified package can be downloaded.
     /// This method assumes the particular version is present in the registry,
     /// and does not verify that it is.
+    ///
+    /// Returns `None` when the configured URL requires the
+    /// `{sha256-checksum}` placeholder. Use [`Self::download_url_with_checksum`]
+    /// when the checksum is available.
     #[must_use]
     pub fn download_url(&self, name: &str, version: &str) -> Option<String> {
-        if !self.dl.contains("{crate}")
-            && !self.dl.contains("{version}")
-            && !self.dl.contains("{prefix}")
-            && !self.dl.contains("{lowerprefix}")
-        {
+        self.download_url_inner(name, version, None)
+    }
+
+    /// Get the URL from where the specified package can be downloaded, including
+    /// the package checksum when required by the registry's URL template.
+    ///
+    /// This method assumes the particular version is present in the registry,
+    /// and does not verify that it is.
+    #[must_use]
+    pub fn download_url_with_checksum(&self, name: &str, version: &str, checksum: &[u8; 32]) -> Option<String> {
+        self.download_url_inner(name, version, Some(checksum))
+    }
+
+    fn download_url_inner(&self, name: &str, version: &str, checksum: Option<&[u8; 32]>) -> Option<String> {
+        if !DOWNLOAD_URL_MARKERS.iter().any(|marker| self.dl.contains(marker)) {
             let mut new = String::with_capacity(self.dl.len() + name.len() + version.len() + 10);
             new.push_str(&self.dl);
             new.push('/');
@@ -30,15 +59,99 @@ impl IndexConfig {
             new.push_str("/download");
             Some(new)
         } else {
-            let mut prefix = String::with_capacity(5);
-            crate_prefix(&mut prefix, name, '/')?;
+            let prefix = if self.dl.contains(PREFIX_MARKER) || self.dl.contains(LOWERPREFIX_MARKER) {
+                let mut prefix = String::with_capacity(5);
+                crate_prefix(&mut prefix, name, '/')?;
+                Some(prefix)
+            } else {
+                None
+            };
+            let lowerprefix = prefix.as_ref().map(|prefix| prefix.to_ascii_lowercase());
+            let checksum = checksum.map(hex::encode);
+            if self.dl.contains(SHA256_CHECKSUM_MARKER) && checksum.is_none() {
+                return None;
+            }
+
             Some(
                 self.dl
-                    .replace("{crate}", name)
-                    .replace("{version}", version)
-                    .replace("{prefix}", &prefix)
-                    .replace("{lowerprefix}", &prefix.to_ascii_lowercase()),
+                    .replace(CRATE_MARKER, name)
+                    .replace(VERSION_MARKER, version)
+                    .replace(PREFIX_MARKER, prefix.as_deref().unwrap_or_default())
+                    .replace(LOWERPREFIX_MARKER, lowerprefix.as_deref().unwrap_or_default())
+                    .replace(SHA256_CHECKSUM_MARKER, checksum.as_deref().unwrap_or_default()),
             )
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::IndexConfig;
+    use crate::Crate;
+
+    fn config(dl: &str) -> IndexConfig {
+        IndexConfig {
+            dl: dl.to_owned(),
+            api: None,
+        }
+    }
+
+    #[test]
+    fn appends_the_default_download_path() {
+        assert_eq!(
+            config("https://example.invalid/crates")
+                .download_url("demo", "1.2.3")
+                .as_deref(),
+            Some("https://example.invalid/crates/demo/1.2.3/download"),
+        );
+    }
+
+    #[test]
+    fn substitutes_all_documented_markers() {
+        let checksum = [0xab; 32];
+        let config = config("https://example.invalid/{crate}/{version}/{prefix}/{lowerprefix}/{sha256-checksum}");
+
+        assert_eq!(
+            config
+                .download_url_with_checksum("MyCrate", "1.2.3", &checksum)
+                .as_deref(),
+            Some(
+                "https://example.invalid/MyCrate/1.2.3/My/Cr/my/cr/\
+                 abababababababababababababababababababababababababababababababab"
+            ),
+        );
+    }
+
+    #[test]
+    fn checksum_marker_requires_a_checksum() {
+        assert_eq!(
+            config("https://example.invalid/{sha256-checksum}").download_url("demo", "1.2.3"),
+            None,
+        );
+    }
+
+    #[test]
+    fn templates_without_prefix_markers_do_not_build_one() {
+        assert_eq!(
+            config("https://example.invalid/{crate}/{version}")
+                .download_url("", "1.2.3")
+                .as_deref(),
+            Some("https://example.invalid//1.2.3"),
+        );
+    }
+
+    #[test]
+    fn version_download_url_supplies_its_checksum() {
+        let krate = Crate::from_slice(
+            br#"{"name":"demo","vers":"1.2.3","deps":[],"cksum":"abababababababababababababababababababababababababababababababab","features":{}}"#,
+        )
+        .unwrap();
+
+        assert_eq!(
+            krate.versions()[0]
+                .download_url(&config("https://example.invalid/{sha256-checksum}"))
+                .as_deref(),
+            Some("https://example.invalid/abababababababababababababababababababababababababababababababab"),
+        );
+    }
+}

src/dirs.rs

@@ -32,7 +32,17 @@ pub fn local_path_and_canonical_url_with_hash_kind(
     Ok((path, canonical_url))
 }
 
+/// Appends Cargo's case-preserving `{prefix}` value.
 pub(crate) fn crate_prefix(accumulator: &mut String, crate_name: &str, separator: char) -> Option<()> {
+    crate_prefix_inner(accumulator, crate_name, separator, false)
+}
+
+/// Appends Cargo's lowercased `{lowerprefix}` value.
+pub(crate) fn crate_lowerprefix(accumulator: &mut String, crate_name: &str, separator: char) -> Option<()> {
+    crate_prefix_inner(accumulator, crate_name, separator, true)
+}
+
+fn crate_prefix_inner(accumulator: &mut String, crate_name: &str, separator: char, lowercase: bool) -> Option<()> {
     match crate_name.len() {
         0 => return None,
         1 => accumulator.push('1'),
@@ -45,7 +55,7 @@ pub(crate) fn crate_prefix(accumulator: &mut String, crate_name: &str, separator
                     .as_bytes()
                     .get(0..1)?
                     .iter()
-                    .map(|c| c.to_ascii_lowercase() as char),
+                    .map(|c| (if lowercase { c.to_ascii_lowercase() } else { *c }) as char),
             );
         }
         _ => {
@@ -54,15 +64,15 @@ pub(crate) fn crate_prefix(accumulator: &mut String, crate_name: &str, separator
                     .as_bytes()
                     .get(0..2)?
                     .iter()
-                    .map(|c| c.to_ascii_lowercase() as char),
+                    .map(|c| (if lowercase { c.to_ascii_lowercase() } else { *c }) as char),
             );
             accumulator.push(separator);
             accumulator.extend(
                 crate_name
                     .as_bytes()
                     .get(2..4)?
                     .iter()
-                    .map(|c| c.to_ascii_lowercase() as char),
+                    .map(|c| (if lowercase { c.to_ascii_lowercase() } else { *c }) as char),
             );
         }
     };
@@ -72,13 +82,54 @@ pub(crate) fn crate_prefix(accumulator: &mut String, crate_name: &str, separator
 pub(crate) fn crate_name_to_relative_path(crate_name: &str, separator: Option<char>) -> Option<String> {
     let separator = separator.unwrap_or(std::path::MAIN_SEPARATOR);
     let mut rel_path = String::with_capacity(crate_name.len() + 6);
-    crate_prefix(&mut rel_path, crate_name, separator)?;
+    crate_lowerprefix(&mut rel_path, crate_name, separator)?;
     rel_path.push(separator);
     rel_path.extend(crate_name.as_bytes().iter().map(|c| c.to_ascii_lowercase() as char));
 
     Some(rel_path)
 }
 
+#[cfg(test)]
+mod tests {
+    use super::{crate_lowerprefix, crate_name_to_relative_path, crate_prefix};
+
+    #[test]
+    fn crate_prefixes_follow_cargo_layout_rules() {
+        for (name, prefix, lowerprefix) in [
+            ("a", "1", "1"),
+            ("ab", "2", "2"),
+            ("AbC", "3/A", "3/a"),
+            ("MyCrate", "My/Cr", "my/cr"),
+        ] {
+            let mut actual_prefix = String::new();
+            crate_prefix(&mut actual_prefix, name, '/').unwrap();
+            assert_eq!(actual_prefix, prefix);
+
+            let mut actual_lowerprefix = String::new();
+            crate_lowerprefix(&mut actual_lowerprefix, name, '/').unwrap();
+            assert_eq!(actual_lowerprefix, lowerprefix);
+        }
+    }
+
+    #[test]
+    fn empty_crate_names_have_no_prefix() {
+        let mut prefix = String::new();
+        assert!(crate_prefix(&mut prefix, "", '/').is_none());
+        assert!(prefix.is_empty());
+
+        assert!(crate_lowerprefix(&mut prefix, "", '/').is_none());
+        assert!(prefix.is_empty());
+    }
+
+    #[test]
+    fn local_index_paths_use_lowerprefix_and_lowercase_filenames() {
+        assert_eq!(
+            crate_name_to_relative_path("MyCrate", Some('/')).as_deref(),
+            Some("my/cr/mycrate"),
+        );
+    }
+}
+
 /// Matches https://github.com/rust-lang/cargo/blob/2928e32734b04925ee51e1ae88bea9a83d2fd451/crates/cargo-util-schemas/src/core/source_kind.rs#L5
 type SourceKind = u64;
 const SOURCE_KIND_REGISTRY: SourceKind = 2;

src/types.rs

@@ -135,7 +135,7 @@ impl Version {
     /// Where to find crate tarball
     #[must_use]
     pub fn download_url(&self, index: &IndexConfig) -> Option<String> {
-        index.download_url(&self.name, &self.vers)
+        index.download_url_with_checksum(&self.name, &self.vers, &self.cksum)
     }
 }
 

tests/sparse_index/mod.rs

@@ -71,6 +71,16 @@ mod with_sparse_http_feature {
         }
     }
 
+    #[test]
+    fn crate_url_uses_a_lowercase_index_path() {
+        let index = crates_io();
+
+        assert_eq!(
+            index.crate_url("MyCrate").as_deref(),
+            Some("https://index.crates.io/my/cr/mycrate"),
+        );
+    }
+
     mod parse_cache_response {
         use crate::sparse_index::with_sparse_http_feature::crates_io;
         use http::header;