From 2ea866f4188543b6c24640ca57ce89957eb0a756 Mon Sep 17 00:00:00 2001
From: Weihang Lo <me@weihanglo.tw>
Date: Sat, 29 Jul 2023 13:06:55 +0100
Subject: [PATCH] fix: normalize relative git submodule urls with `ssh://`

Git only assumes a URL is a relative path if it starts with `./` or `../`.
See <https://git-scm.com/docs/git-submodule>. To fetch the current repo,
we need to construct the full submodule URL.

At this moment it comes with some limitations:

* GitHub only normalizes HTTPS protocol with relative path `..`.
  (`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
* `url` crate cannot parse SCP-like URLs.
  (`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)

For the first one, we use `url` crate to normalize. For the second one,
we append the relative path directly to parent URL. That says, the only
failing case downs to one: SCP-like parent remote URLs with relative
submodule URLs on GitHub or git services not normalizing them.
---
 src/cargo/sources/git/utils.rs | 44 ++++++++++++++++++++++++++++------
 tests/testsuite/git.rs         |  6 ++---
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs
index b763ba61757e..4019306e794a 100644
--- a/src/cargo/sources/git/utils.rs
+++ b/src/cargo/sources/git/utils.rs
@@ -441,16 +441,46 @@ impl<'a> GitCheckout<'a> {
             }
 
             // Git only assumes a URL is a relative path if it starts with `./` or `../`.
-            // See [`git submodule add`] documentation.
+            // See <https://git-scm.com/docs/git-submodule>. To fetch the current repo,
+            // we need to construct the full submodule URL.
             //
-            // [`git submodule add`]: https://git-scm.com/docs/git-submodule
+            // At this moment it comes with some limitations:
+            //
+            // * GitHub only normalizes HTTPS protocol with relative path `..`.
+            //   (`ssh://git@github.com/rust-lang/cargo.git/relative/..` is invalid)
+            // * `url` crate cannot parse SCP-like URLs.
+            //   (`git@github.com:rust-lang/cargo.git` is not a valid WHATWG URL)
+            //
+            // For the first one, we use `url` crate to normalize. For the second one,
+            // we append the relative path directly to parent URL. That says, the only
+            // failing case downs to one: SCP-like parent remote URLs with relative
+            // submodule URLs on GitHub or git services not normalizing them.
+            //
+            // See rust-lang/cargo#12404 and rust-lang/cargo#12295.
             let child_remote_url = if ["./", "../"].iter().any(|p| child_url_str.starts_with(p)) {
-                let mut new_remote_url = parent_remote_url.to_string();
-                if !new_remote_url.ends_with('/') {
-                    new_remote_url.push('/');
+                match Url::parse(parent_remote_url) {
+                    Ok(mut parent_url) => {
+                        let path = parent_url.path();
+                        if !path.ends_with('/') {
+                            parent_url.set_path(&format!("{path}/"));
+                        }
+                        let new_remote_url = parent_url.join(child_url_str).with_context(|| {
+                            format!(
+                                "failed to parse relative child submodule url `{child_url_str}` \
+                                using parent base url `{parent_url}`"
+                            )
+                        })?;
+                        Cow::from(new_remote_url.to_string())
+                    }
+                    Err(_) => {
+                        let mut new_remote_url = parent_remote_url.to_string();
+                        if !new_remote_url.ends_with('/') {
+                            new_remote_url.push('/');
+                        }
+                        new_remote_url.push_str(child_url_str);
+                        Cow::from(new_remote_url)
+                    }
                 }
-                new_remote_url.push_str(child_url_str);
-                Cow::from(new_remote_url)
             } else {
                 Cow::from(child_url_str)
             };
diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs
index 09250e7ce11d..f60ee978a36e 100644
--- a/tests/testsuite/git.rs
+++ b/tests/testsuite/git.rs
@@ -3633,7 +3633,7 @@ fn different_user_relative_submodules() {
             .file("Cargo.toml", &basic_lib_manifest("dep1"))
             .file("src/lib.rs", "")
     });
-    let _user2_git_project2 = git::new("user2/dep2", |project| {
+    let user2_git_project2 = git::new("user2/dep2", |project| {
         project
             .file("Cargo.toml", &basic_lib_manifest("dep1"))
             .file("src/lib.rs", "")
@@ -3673,14 +3673,14 @@ fn different_user_relative_submodules() {
             "\
 [UPDATING] git repository `{}`
 [UPDATING] git submodule `{}`
-[UPDATING] git submodule `{}/../dep2`
+[UPDATING] git submodule `{}`
 [COMPILING] dep1 v0.5.0 ({}#[..])
 [COMPILING] foo v0.5.0 ([CWD])
 [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
 ",
             path2url(&user1_git_project.root()),
             path2url(&user2_git_project.root()),
-            path2url(&user2_git_project.root()),
+            path2url(&user2_git_project2.root()),
             path2url(&user1_git_project.root()),
         ))
         .run();