Fix stable format using url-encoding (#86)

* Update crates

* Fix lints

* Add test case

* Fix percent encoded urls

* Sanity check quinn not actually being used

Cargo.toml

@@ -44,7 +44,7 @@ similar-asserts = "1.1"
 # Used to deserialize test files into metadata we can load
 serde_json = "1.0"
 # index metadata retrieval
-tame-index = "0.10"
+tame-index = "0.12"
 
 [profile.dev.package.insta]
 opt-level = 3

deny.toml

@@ -23,6 +23,9 @@ allow = [
 
 [bans]
 multiple-versions = "deny"
+deny = [
+    { crate = "quinn", reason = "we don't need http3 support, this just ensures we aren't unneccessarily having http3 support actually (https://github.com/rust-lang/cargo/issues/10801) enabled" },
+]
 skip = []
 skip-tree = []
 

src/errors.rs

@@ -6,7 +6,7 @@ use std::fmt;
 pub enum Error {
     /// --no-deps was specified when acquiring metadata
     NoResolveGraph,
-    /// A cargo_metadata error occurred
+    /// A [`cargo_metadata::Error`] error occurred
     Metadata(CMErr),
     /// A package specification was invalid
     InvalidPkgSpec(&'static str),

src/lib.rs

@@ -85,9 +85,9 @@ impl Kid {
 #[allow(clippy::fallible_impl_from)]
 impl From<cargo_metadata::PackageId> for Kid {
     fn from(pid: cargo_metadata::PackageId) -> Self {
-        let repr = pid.repr;
+        let mut repr = pid.repr;
 
-        let gen = || {
+        let mut gen = || {
             let components = if repr.contains(' ') {
                 let name = (0, repr.find(' ')?);
                 let version = (name.1 + 1, repr[name.1 + 1..].find(' ')? + name.1 + 1);
@@ -101,13 +101,79 @@ impl From<cargo_metadata::PackageId> for Kid {
 
                 [name, version, source]
             } else {
-                let vmn = repr.rfind('#')?;
+                let mut vmn = repr.rfind('#')?;
                 let (name, version) = if let Some(split) = repr[vmn..].find('@') {
                     ((vmn + 1, vmn + split), (vmn + split + 1, repr.len()))
                 } else {
                     let begin = repr.rfind('/')? + 1;
                     let end = if repr.starts_with("git+") {
-                        repr[begin..].find('?').map_or(vmn, |q| q + begin)
+                        // Unfortunately the stable format percent encodes the source url in the metadata, and since
+                        // git branches/tags can contain various special characters, notably '/', we need to decode them
+                        // to be able to match against the non-encoded url used...everywhere else
+                        let end = repr[begin..].rfind('?').map_or(vmn, |q| q + begin);
+
+                        if repr[end..].contains('%') {
+                            // https://en.wikipedia.org/wiki/Percent-encoding
+                            // ␣   !   "   #   $   %   &   '   (   )   *   +   ,   /   :   ;   =   ?   @   [   ]
+                            // %20 %21 %22 %23 %24 %25 %26 %27 %28 %29 %2A %2B %2C %2F %3A %3B %3D %3F %40 %5B %5D
+                            let mut decoded = String::new();
+                            let mut encoded = &repr[end..];
+                            let before = encoded.len();
+
+                            loop {
+                                let Some(pi) = encoded.find('%') else {
+                                    decoded.push_str(encoded);
+                                    break;
+                                };
+
+                                decoded.push_str(&encoded[..pi]);
+
+                                let Some(encoding) = encoded.get(pi + 1..pi + 3) else {
+                                    // This _should_ never happen, but just in case
+                                    panic!("invalid percent encoding in '{}', '{}' should be exactly 3 digits long", &repr[end..], &encoded[pi..]);
+                                };
+
+                                let c = match encoding {
+                                    // By far the most likely one
+                                    "2F" | "2f" => '/',
+                                    "20" => ' ',
+                                    "21" => '!',
+                                    "22" => '"',
+                                    "23" => '#',
+                                    "24" => '$',
+                                    "25" => '%',
+                                    "26" => '&',
+                                    "27" => '\'',
+                                    "28" => '(',
+                                    "29" => ')',
+                                    "2A" | "2a" => '*',
+                                    "2B" | "2b" => '+',
+                                    "2C" | "2c" => ',',
+                                    "3A" | "3a" => ':',
+                                    "3B" | "3b" => ';',
+                                    "3D" | "3d" => '=',
+                                    "3F" | "3f" => '?',
+                                    "40" => '@',
+                                    "5B" | "5b" => '[',
+                                    "5D" | "5d" => ']',
+                                    _ => panic!(
+                                        "unknown percent encoding '%{encoding}' in '{}'",
+                                        &repr[end..]
+                                    ),
+                                };
+
+                                decoded.push(c);
+                                encoded = &encoded[pi + 3..];
+                            }
+
+                            repr.truncate(end);
+                            repr.push_str(&decoded);
+
+                            // move the version string back to account for the now shorter decoded repr
+                            vmn -= before - decoded.len();
+                        }
+
+                        end
                     } else {
                         vmn
                     };
@@ -238,7 +304,7 @@ pub enum Node<N> {
     Krate {
         /// The unique identifier for this node.
         id: Kid,
-        /// Associated user data with the node. Must be From<cargo_metadata::Package>
+        /// Associated user data with the node. Must be `From<cargo_metadata::Package>`
         krate: N,
         /// List of features enabled on the crate
         features: EnabledFeatures,
@@ -288,15 +354,15 @@ pub enum Edge {
     Dep {
         /// The dependency kind for the edge link
         kind: DepKind,
-        /// A possible cfg() or <target-triple> applied to this dependency
+        /// A possible `cfg()` or <target-triple> applied to this dependency
         cfg: Option<String>,
     },
     /// An edge from one feature to another
     Feature,
     DepFeature {
         /// The dependency kind for the edge link
         kind: DepKind,
-        /// A possible cfg() or <target-triple> applied to this dependency
+        /// A possible `cfg()` or <target-triple> applied to this dependency
         cfg: Option<String>,
     },
 }

tests/pid/Cargo.toml

@@ -18,6 +18,9 @@ tower-http = { package = "tower-http", version = "0.5.0", features = [
 objc2-latest = { package = "objc2", version = "*" }
 objc2-registry = { package = "objc2", version = "0.3.0-beta.3.patch-leaks.3" }
 objc2 = { git = "https://github.com/madsmtm/objc2", rev = "65de002" }
+# Repro for https://github.com/EmbarkStudios/krates/issues/85
+ohno = { git = "https://github.com/EmbarkStudios/krates", branch = "branch/test", package = "krates" }
+krates = { git = "https://github.com/EmbarkStudios/krates", tag = "0.16.10" }
 
 # Repro for https://github.com/EmbarkStudios/krates/issues/74, depending on 2
 # versions of the same crate, but only one of which is selected due to features