install: keep non-default ssh port in the ssh fallback clone URL

Closed
RO
1mo ago
+84-2

Fixes #36931

Problem

  • A git dependency with an explicit ssh port, git+ssh://git@host:52626/user/repo.git#v1.3.2, fails to install: the ssh clone attempt runs git clone ssh://git@host/52626/user/repo.git, with the port turned into a path segment.
  • Repository::try_ssh (src/install/repository.rs, the ssh:// branch) passes every ssh:// URL through hosted_git_info::correct_url, which replaces the last colon with a slash. That rewrite exists for scp-style input such as ssh://git@github.com:user/repo, but it runs unconditionally, so it also hits a port, a [::1]:2222 host, a colon inside the path, and, for a URL with no userinfo (ssh://host/user/repo.git), the colon of the scheme itself (git is handed ssh///host/user/repo.git).

Fix

  • try_ssh now returns an ssh:// URL unchanged when it parses as a URL, and only runs correct_url when it does not. The check is hosted_git_info::is_parseable_url, a whole-string WTF::URL parse, the same call hosted_git_info::parse_url already makes before it falls back to correct_url.
  • This is the order npm's hosted-git-info uses: parseUrl tries new URL(giturl) first and only calls correctUrl when that throws. ssh://host:52626/path parses, so npm never rewrites it; ssh://host:user/repo does not parse (non-numeric port), so it is still rewritten. try_ssh was the one caller of correct_url that skipped the parse step.
  • Checking "does it parse" rather than "is the colon followed by a digit" matters: scp-style paths that start with a digit (git@github.com:1Password/op-js.git) do not parse as URLs and must still be rewritten, which npm does. A digit heuristic inside correct_url (the approach in #38887) stops rewriting those and also breaks HostedGitInfo::from_url for them, verified on a build with that change: hostedGitInfo.parseUrl("git@github.com:1Password/op-js.git") throws InvalidGitUrl, and for git+ssh://git@github.com:1Password/op-js.git the ssh fallback runs git clone ssh://git@github.com:1Password/op-js.git, which git rejects. Both work on main.
  • The check has to parse the whole string. The first revision used bun_url::origin_from_slice, which parses only the leading ASCII prefix, so ssh://git@höst:user/repo.git was judged valid on ssh://g and no longer corrected; the höst test row below fails on that revision and passes on this one.
  • Verified:
    • test/cli/install/bun-install.test.ts, "ssh fallback clone URL for ...": shadows git with a PATH shim that records its argv and asserts the exact ssh:// URL for eight dependency specs. On a build without the fix the five pass-through rows fail (port, IPv6 + port, port + non-ASCII path, colon in the path, no userinfo; e.g. ssh://git@localhost/52626/user/repo.git, ssh///localhost/user/repo.git) and the three rewrite rows pass; with the fix all eight pass.
    • test/cli/install/hosted-git-info/cases.ts and parse-url.test.ts: cases for the port URL surviving parse_url and for digit-leading scp paths still being corrected. They pass on main; with #38887's correct_url change applied, all five fail, as does the digit-leading bun-install.test.ts row.
    • bun bd test test/cli/install/hosted-git-info/: 662 pass. cargo clippy -p bun_install and cargo fmt are clean.

Background

  • scp-style git URL: user@host:path, what git clone git@github.com:foo/bar takes. The colon separates host and path, not host and port. hosted-git-info (the npm library src/install/hosted_git_info.rs ports) turns these into parseable URLs by replacing that colon with a slash; this is correctUrl / correct_url.
  • try_https / try_ssh: when cloning a git dependency, PackageManagerTask first tries an https form of the URL and, if that clone fails, an ssh form. try_ssh builds the ssh form; for input that is already ssh:// its only job is the scp-style correction. git+ssh: dependencies are tagged as plain git dependencies without going through hosted_git_info::parse_url (dependency.rs), so try_ssh is the only place the issue's URL is inspected.
  • JscUrl::from_utf8 (bun_url::whatwg::URL): the WHATWG URL parser from WebKit; it returns nothing for input it rejects. For a non-special scheme such as ssh:, a non-numeric port (:user/repo) is a rejection and a numeric one (:52626) is not. OwnedJscUrl is the RAII wrapper that frees the C++ object.
  • HostedGitInfo::from_url returns None for known hosts with an explicit port (git+ssh://user@bitbucket.org:2222/foo/bar) because the host lookup compares host:port against the provider domain. This is unrelated to the bug (such a dependency is treated as a plain git URL and cloned with the port intact, which is what the issue asks for), so this PR leaves it alone; it is why #38887's cases.ts entries do not pass even with that PR's change applied.
First revision

The first revision gated the correction on bun_url::origin_from_slice(url).is_some(). origin_from_slice hands only the bytes before the first non-ASCII one to the parser (its existing caller passes an already-serialized href), so for ssh://git@höst:user/repo.git it parsed ssh://g, reported it valid, and the scp-style colon was no longer rewritten, a change from main for that input. The current revision parses the full string and adds that input as a test row.

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/install/bun-install.test.ts