util: provide a better error message for invalid SSH URLs#15185
Merged
weihanglo merged 1 commit intorust-lang:masterfrom Feb 14, 2025
Merged
util: provide a better error message for invalid SSH URLs#15185weihanglo merged 1 commit intorust-lang:masterfrom
weihanglo merged 1 commit intorust-lang:masterfrom
Conversation
Collaborator
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
0xPoe
approved these changes
Feb 14, 2025
Member
0xPoe
left a comment
There was a problem hiding this comment.
Thanks! ![]()
Looks good to me. It seems that converting git@ to ssh:// is always correct. I’ll ask another reviewer to take another look.
weihanglo
approved these changes
Feb 14, 2025
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.
cc9a840 to
7cd8890
Compare
weihanglo
approved these changes
Feb 14, 2025
Member
weihanglo
left a comment
There was a problem hiding this comment.
The suggestion is basically mapping from
[<user>@]<host>:/<path-to-git-repo>
to
ssh://[<user>@]<host>[:<port>]/<path-to-git-repo>,
so should be fairly correct. Anyway it is just a suggestion.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Feb 15, 2025
Update cargo 13 commits in 2928e32734b04925ee51e1ae88bea9a83d2fd451..ce948f4616e3d4277e30c75c8bb01e094910df39 2025-02-07 16:50:22 +0000 to 2025-02-14 20:32:07 +0000 - util: provide a better error message for invalid SSH URLs (rust-lang/cargo#15185) - Fix the description of the `"root"` field of the `cargo metadata`'s output (rust-lang/cargo#15182) - refactor: Consolidate creation of SourceId from manifest path (rust-lang/cargo#15172) - docs(embedded): Note the shebang deviation (rust-lang/cargo#15173) - refactor(embedded): Integrate cargo-script logic into main parser (rust-lang/cargo#15168) - feat: implement workspace feature unification (rust-lang/cargo#15157) - Fix race condition in panic_abort_tests (rust-lang/cargo#15169) - Update all dependencies (rust-lang/cargo#15166) - Update curl from 8.9.0 to 8.12.0 (rust-lang/cargo#15162) - Update annotate-snippets from 0.11.4 to 0.11.5 (rust-lang/cargo#15165) - Update deny.toml (rust-lang/cargo#15164) - Update rusqlite from 0.32.1 to 0.33.0 (rust-lang/cargo#15163) - fix: align first line of unordered list with following (rust-lang/cargo#15161)
RalfJung
pushed a commit
to RalfJung/miri
that referenced
this pull request
Feb 16, 2025
Update cargo 13 commits in 2928e32734b04925ee51e1ae88bea9a83d2fd451..ce948f4616e3d4277e30c75c8bb01e094910df39 2025-02-07 16:50:22 +0000 to 2025-02-14 20:32:07 +0000 - util: provide a better error message for invalid SSH URLs (rust-lang/cargo#15185) - Fix the description of the `"root"` field of the `cargo metadata`'s output (rust-lang/cargo#15182) - refactor: Consolidate creation of SourceId from manifest path (rust-lang/cargo#15172) - docs(embedded): Note the shebang deviation (rust-lang/cargo#15173) - refactor(embedded): Integrate cargo-script logic into main parser (rust-lang/cargo#15168) - feat: implement workspace feature unification (rust-lang/cargo#15157) - Fix race condition in panic_abort_tests (rust-lang/cargo#15169) - Update all dependencies (rust-lang/cargo#15166) - Update curl from 8.9.0 to 8.12.0 (rust-lang/cargo#15162) - Update annotate-snippets from 0.11.4 to 0.11.5 (rust-lang/cargo#15165) - Update deny.toml (rust-lang/cargo#15164) - Update rusqlite from 0.32.1 to 0.33.0 (rust-lang/cargo#15163) - fix: align first line of unordered list with following (rust-lang/cargo#15161)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR try to resolve?
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.gitas a source in Cargo.toml, which are the default format accepted by OpenSSH. Unfortunately, these are not actually URLs, and unsurprisingly, theurlcrate doesn't accept them.However, our error message is unhelpful and looks like this:
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.Fixes #13549
How should we test and review this PR?
cargo init pkg1cd pkg1Cargo.tomlto add the dependency linebytes = { git = "git@github.com:tokio-rs/bytes.git", tag = "v1.10.0" }.cargo buildcargo build.Additional information
N/A