node 19.1 breaks `url.parse`

[ljharb]

Version

v19.1.0

Platform

Darwin Jordans-MacBook-Pro-2.local 20.6.0 Darwin Kernel Version 20.6.0: Thu Sep 29 20:15:11 PDT 2022; root:xnu-7195.141.42~1/RELEASE_X86_64 x86_64

Subsystem

url

What steps will reproduce the bug?

url.parse('https://git@github.com:inspect-js/is-array-buffer.git')

How often does it reproduce? Is there a required condition?

node v19.1.0 always reproduces it; any earlier version does not.

What is the expected behavior?

Url {
  protocol: 'https:',
  slashes: true,
  auth: 'git',
  host: 'github.com',
  port: null,
  hostname: 'github.com',
  hash: null,
  search: null,
  query: null,
  pathname: '/:inspect-js/is-array-buffer.git',
  path: '/:inspect-js/is-array-buffer.git',
  href: 'https://git@github.com/:inspect-js/is-array-buffer.git'
}

What do you see instead?

Uncaught TypeError [ERR_INVALID_URL]: Invalid URL
    at __node_internal_captureLargerStackTrace (node:internal/errors:484:5)
    at new NodeError (node:internal/errors:393:5)
    at getHostname (node:url:521:15)
    at Url.parse (node:url:390:14)
    at Object.urlParse [as parse] (node:url:147:13) {
  input: 'https://git@github.com:inspect-js/is-array-buffer.git',
  code: 'ERR_INVALID_URL'
}

Additional information

Specifically, this breaks https://npmjs.com/auto-changelog via https://npmjs.com/parse-github-url in node 19.1+.

[ljharb]

This may be related to #45116 or #45011; cc @Trott

[Trott]

I saw @aduh95 had marked something as baking-for-lts and dont-land-on-v18.x out of suspicion that it broke a url.parse() usage in the ecosystem but now I'm not finding that anywhere.

Normally, the thing to do would be to find the culprit commit and revert it (and likely re-introduce it as a breaking change in a major release). However it's likely this is going to be a revert that will increase security liability, so another possibility is to create a carve-out for these kinds of URLs.

[Trott]

Confirmed that the culprit is #45012.

[Trott]

I saw @aduh95 had marked something as baking-for-lts and dont-land-on-v18.x out of suspicion that it broke a url.parse() usage in the ecosystem but now I'm not finding that anywhere.

I was confused. I was thinking of #45176.

[Trott]

Unfortunately, I'm not seeing a non-terrible way to do a carve-out. Reluctantly, I think the approach I'd take is:

1. Revert this.
2. Re-introduce it as a warning rather than an error.
3. Re-introduce the error as a semver-major and Get The Word Out™.

I'll open a revert now.