From 7cd88900d3e16186074e275cf03ae3737579f111 Mon Sep 17 00:00:00 2001 From: "brian m. carlson" Date: Fri, 14 Feb 2025 01:46:53 +0000 Subject: [PATCH] util: provide a better error message for invalid SSH URLs It's very common for users to attempt to use the pseudo-URLs that GitHub or other providers provide in the form `git@github.com:rust-lang/rust.git` as a source in Cargo.toml, which are the default format accepted by OpenSSH. Unfortunately, these are not actually URLs, and unsurprisingly, the `url` crate doesn't accept them. However, our error message is unhelpful and looks like this: invalid url `git@github.com:rust-lang/rust.git`: relative URL without a base This is technically true, but we can do better. The user actually wants to write a real SSH URL, so if the non-URL starts with `git@`, let's rewrite it into a real URL for them to help them and include that in the error message. `git@` is the prefix used by all major forges, as well as the default configuration for do-it-yourself implementations like Gitolite. While other options are possible, they are much less common, and this is an extremely easy and cheap heuristic that does not necessitate complicated parsing, but which we can change in the future should that be necessary. This also avoids the problem where users try to turn the pseudo-URL into a real URL by just prepending `ssh://`, which causes an error message about the invalid port number due to the colon which they have not changed. Since they can just copy and paste the proposed answer, they'll be less likely to attempt this invalid approach. --- src/cargo/util/into_url.rs | 13 ++++++++++++- tests/testsuite/git.rs | 5 +++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/cargo/util/into_url.rs b/src/cargo/util/into_url.rs index 26f365ee8..fa48cc31f 100644 --- a/src/cargo/util/into_url.rs +++ b/src/cargo/util/into_url.rs @@ -12,7 +12,18 @@ pub trait IntoUrl { impl<'a> IntoUrl for &'a str { fn into_url(self) -> CargoResult { - Url::parse(self).map_err(|s| anyhow::format_err!("invalid url `{}`: {}", self, s)) + Url::parse(self).map_err(|s| { + if self.starts_with("git@") { + anyhow::format_err!( + "invalid url `{}`: {}; try using `{}` instead", + self, + s, + format_args!("ssh://{}", self.replacen(':', "/", 1)) + ) + } else { + anyhow::format_err!("invalid url `{}`: {}", self, s) + } + }) } } diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index c63d355c4..8d00ecf18 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -528,6 +528,7 @@ this is dep1 this is dep2 #[cargo_test] fn cargo_compile_with_short_ssh_git() { let url = "git@github.com:a/dep"; + let well_formed_url = "ssh://git@github.com/a/dep"; let p = project() .file( @@ -565,9 +566,9 @@ fn cargo_compile_with_short_ssh_git() { [ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml` Caused by: - invalid url `{}`: relative URL without a base + invalid url `{}`: relative URL without a base; try using `{}` instead ", - url + url, well_formed_url )) .run(); }