Cannot parse "scp-style" git URLs

[nipunn1313]

Short code to highlight issue:

extern crate url;

use url::Url;

fn main() {
    println!("1) {}", Url::parse("ssh://user@host.xz/path/to/repo.git/").unwrap());
    println!("2) {}", Url::parse("user@host.xz:path/to/repo.git/").unwrap());
}

The first line parses, but the second fails with
thread 'main' panicked at 'called Result::unwrap() on an Err value: RelativeUrlWithoutBase', ../src/libcore/result.rs:788

straight from git help clone we see that the second is a valid format

       The following syntaxes may be used with them:

       o   ssh://[user@]host.xz[:port]/path/to/repo.git/

       o   git://host.xz[:port]/path/to/repo.git/

       o   http[s]://host.xz[:port]/path/to/repo.git/

       o   ftp[s]://host.xz[:port]/path/to/repo.git/

       o   rsync://host.xz/path/to/repo.git/

       An alternative scp-like syntax may also be used with the ssh protocol:

       o   [user@]host.xz:path/to/repo.git/

See https://secure.phabricator.com/T11004 for an explanation why the "scp-like" syntax is not exactly the same as the ssh syntax. For me, switching to the ssh style syntax is insufficient on its own.

[SimonSapin]

straight from git help clone we see that the second is a valid format

It’s valid for git clone. That doesn’t make it a URL in either IETF or WHATWG’s definition of URL.

I’d recommend having your own code recognize what kind of input you have, only give some of them to rust-url, and parse the others yourself.

[nipunn1313]

Ah then perhaps this is an issue with cargo.
Cargo uses Url::parse in order to parse the git url in a field like this:

[dependencies.mydep]
git = "user@url.net:repo/mydep.git"

From what you're saying, Url::parse should not be expected to parse this git clone url type.

[SimonSapin]

Yes, that's what I'm saying.

[KiChjang]

@nipunn1313 For your reference: https://url.spec.whatwg.org/#syntax-url

[nipunn1313]

Thanks for the help. Closing this out since it doesn't make sense to implement this here.

[mxork]

I'd like you to reconsider opening this issue. I suspect there are enough scp-style not-URLs in the wild that the usefulness of adding a parser for them in url outweighs the cost of extending the api by a function.

Not supporting scp-style forces consumers of url to either:

1. Reimplement the same test_for_scp_style_url_and_convert function and remember to invoke it before passing the result to Url::parse.

   Or,

2. Not support scp-style at all.

The first choice is unergonomic for developers. The second results in an ecosystem of tools that return relative URL without a base errors on innocuously incorrect inputs. As referenced earlier in the thread, that ecosystem includes cargo, which has a years-old issue caused by this non-feature (though its dependency on git2-rs).

I want to close that issue, and I would prefer to do that in a way that potentially benefits all users of url.

The conversion between scp-style and true-url, as I understand it, is simply rewriting (or reinterpreting) the : terminating the host section to a /, and possibly pre-pending an ssh://.

My preferred way of solving this is to implement Url::parse_from_maybe_scp_style by simply performing the string rewrite and feeding the resut into the normal parser. It has clear semantics, and doesn't require modifying or duplicating any of parser. It's also clear that the user is opting in to non-standard behavior.

Feature-gating the method seems like overkill, but is an option.

I would also like to add a check for scp-style urls to the return path of RelativeUrlWithoutBase and hint that the caller may have passed an scp-style url.

[SimonSapin]

My response at #424 (comment) also applies here.