Fixup missing percent encodings (#87)

EmbarkStudios · Jun 19, 2024 · 3e5d62c · 3e5d62c
1 parent 54b5fea
commit 3e5d62c
Show file tree

Hide file tree

Showing 7 changed files with 484 additions and 427 deletions.
diff --git a/src/builder.rs b/src/builder.rs
@@ -730,6 +730,10 @@ impl Builder {
             }
         }
 
+        // We need to establish if the package ids are in the opaque or stable format as
+        // it changes how git sources are compared
+        let is_opaque = resolved.nodes[0].id.repr.splitn(3, ' ').count() == 3;
+
         let mut nodes: Vec<_> = resolved
             .nodes
             .into_iter()
@@ -1232,9 +1236,14 @@ impl Builder {
                                 let source_matches = dep.source.as_deref().map_or(true, |dsrc| {
                                     let psrc = rdep.pkg.source();
                                     if let Some((dgit, pgit)) = dsrc.strip_prefix("git+").zip(psrc.strip_prefix("git+")) {
-                                        // Git sources can have the full revision spec at the end, which is not part of
+                                        // The opaque git sources can have the full revision spec at the end, which is not part of
                                         // source declaration
-                                        let dgit = dgit.rfind('#').map_or(dgit, |end| &dgit[..end]);
+                                        let dgit = if is_opaque {
+                                            dgit.rfind('#').map_or(dgit, |end| &dgit[..end])
+                                        } else {
+                                            dgit
+                                        };
+
                                         dgit == pgit
                                     } else {
                                         dsrc == psrc

diff --git a/src/lib.rs b/src/lib.rs
@@ -113,9 +113,6 @@ impl From<cargo_metadata::PackageId> for Kid {
                         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();
@@ -133,10 +130,23 @@ impl From<cargo_metadata::PackageId> for Kid {
                                     panic!("invalid percent encoding in '{}', '{}' should be exactly 3 digits long", &repr[end..], &encoded[pi..]);
                                 };
 
+                                // https://en.wikipedia.org/wiki/Percent-encoding
+                                // Reserved characters after percent-encoding
+                                //
+                                // ␣   !   "   #   $   %   &   '   (   )   *   +   ,   /   :   ;   =   ?   @   [   ]
+                                // %20 %21 %22 %23 %24 %25 %26 %27 %28 %29 %2A %2B %2C %2F %3A %3B %3D %3F %40 %5B %5D
+                                //
+                                // Common characters after percent-encoding (ASCII or UTF-8 based)
+                                //
+                                // -   .   <   >   \   ^   _   `   {   |   }   ~
+                                // %2D %2E %3C %3E %5C %5E %5F %60 %7B %7C %7D %7E
+                                //
+                                // Note that `£` and `€` can also be percent encoded, but are _completely_ different and
+                                // I don't feel like supporting it until someone actually complains
+
                                 let c = match encoding {
                                     // By far the most likely one
                                     "2F" | "2f" => '/',
-                                    "20" => ' ',
                                     "21" => '!',
                                     "22" => '"',
                                     "23" => '#',
@@ -149,13 +159,28 @@ impl From<cargo_metadata::PackageId> for Kid {
                                     "2A" | "2a" => '*',
                                     "2B" | "2b" => '+',
                                     "2C" | "2c" => ',',
-                                    "3A" | "3a" => ':',
+                                    "2D" | "2d" => '-',
+                                    "2E" | "2e" => '.',
                                     "3B" | "3b" => ';',
+                                    "3C" | "3c" => '<',
                                     "3D" | "3d" => '=',
-                                    "3F" | "3f" => '?',
+                                    "3E" | "3e" => '>',
                                     "40" => '@',
-                                    "5B" | "5b" => '[',
                                     "5D" | "5d" => ']',
+                                    "5F" | "5f" => '_',
+                                    "60" => '`',
+                                    "7B" | "7b" => '{',
+                                    "7C" | "7c" => '|',
+                                    "7D" | "7d" => '}',
+                                    // These are invalid in branches/tags, but meh
+                                    // https://git-scm.com/docs/git-check-ref-format
+                                    "20" => ' ',
+                                    "3A" | "3a" => ':',
+                                    "3F" | "3f" => '?',
+                                    "5B" | "5b" => '[',
+                                    "5C" | "5c" => '\\',
+                                    "5E" | "5e" => '^',
+                                    "7E" | "7e" => '~',
                                     _ => panic!(
                                         "unknown percent encoding '%{encoding}' in '{}'",
                                         &repr[end..]

diff --git a/tests/pid-opaque.json b/tests/pid-opaque.json
diff --git a/tests/pid-stable.json b/tests/pid-stable.json
diff --git a/tests/pid/Cargo.lock b/tests/pid/Cargo.lock
diff --git a/tests/pid/Cargo.toml b/tests/pid/Cargo.toml
@@ -21,6 +21,7 @@ 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" }
+sanity = { git = "https://github.com/EmbarkStudios/krates", branch = "}{please_don't-ever_do/this!#$%&()", package = "krates" }
 
 # 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