Skip to content

fix(arborist): only replace hostname for resolved URL - #8185

Merged
owlstronaut merged 3 commits into
npm:latestfrom
billy-briggs-dev:bugfix/only-replace-hostname
Mar 26, 2025
Merged

owlstronaut merged 3 commits into
npm:latestfrom
billy-briggs-dev:bugfix/only-replace-hostname

Conversation

@billy-briggs-dev

Copy link
Copy Markdown
Contributor

Related to #6110

There is an edge case where if an npm project is using a registry with a path the replace-registry-host is including the path from the registry url.

Current Behavior:

Expected Behavior:

This fix resolves this inconsistency by refactoring the logic to construct URL objects from the registry and resolved strings and maps the hostname of the new registry to the hostname of the resolved url instead of doing string parsing.

I'm welcome to any feedback on this solution!

@billy-briggs-dev
billy-briggs-dev requested a review from a team as a code owner March 26, 2025 19:13
@owlstronaut

Copy link
Copy Markdown

This is great, thanks for your contribution!

@owlstronaut
owlstronaut merged commit 885accd into npm:latest Mar 26, 2025
@github-actions github-actions Bot mentioned this pull request Mar 26, 2025
wraithgar pushed a commit that referenced this pull request Jun 10, 2025
…ls (#8356)

When custom hosted git url are used to install dependencies it's not
installing that dependency when the url is of
`git+ssh://git@customgit.com:a/b/c.git` format instead of
`git+ssh://git@customgit.com/a/b/c.git`.

#### Cause:
` new URL(resolved)` throws an error which is getting caught and in-turn
results in removal of that node.
This behaviour was working as expected before this
#8185 which replaced
`hgi.parseUrl(resoved)` call to `new URL(resolved)`

#### Fix:
keep the `hgi.parseUrl` call to correctly parse the git url as
mentioned/explained in this PR #5758

Fixes: #8331
msuozzo added a commit to gtystahl/oss-rebuild-fork that referenced this pull request Sep 24, 2026
Starting with npm 11.3.0 (arborist 9.0.2, npm/cli#8185) the install rewrites
lockfile and packument tarball URLs onto the configured registry by copying
only its hostname and port. The timewarp credentials carried in the registry
URL are dropped, every tarball request reaches timewarp unauthenticated, and
the install fails with "unsupported platform". Versions before 11.6.0 fail
earlier with a TLS error because the https scheme is kept as well.

Pass --replace-registry-host=never, the documented spelling of what the
previous commit's "true" value did by falling through to the hostname
string type. npm 12 refuses tarballs from a host other than the registry
unless allow-remote permits it, so pass --allow-remote=all alongside.
Both flags are accepted by every npm 11 release.

Derive the flags from the npm version inside the install tool rather than a
strategy field. This incurs no backwards compatibility concerns as all
applicable builds were failed and, thus, published no strategies.
msuozzo added a commit to google/oss-rebuild that referenced this pull request Sep 24, 2026
Starting with npm 11.3.0 (arborist 9.0.2, npm/cli#8185) the install rewrites
lockfile and packument tarball URLs onto the configured registry by copying
only its hostname and port. The timewarp credentials carried in the registry
URL are dropped, every tarball request reaches timewarp unauthenticated, and
the install fails with "unsupported platform". Versions before 11.6.0 fail
earlier with a TLS error because the https scheme is kept as well.

Pass --replace-registry-host=never, the documented approach to falling
through to the hostname string type. npm 12 refuses tarballs from a host
other than the registry unless allow-remote permits it, so pass
--allow-remote=all alongside. Both flags are accepted on npm 11.

Derive the flags from the npm version inside the install tool rather than a
strategy field. This incurs no backwards compatibility concerns as all
applicable builds were failed and, thus, published no strategies.

---------

Co-authored-by: Matthew Suozzo <msuozzo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants