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

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: 

- Inputs: 
- `registry=https://internal.mycompany.com/artifactory/api/npm/npm-all`
   - `replace-registry-host=always`
- resolved: "https://external.mycompany.com/artifactory/api/npm/npm-all"
(package-lock.json)

- Output:
"https://internal.mycompany.com/artifactory/api/npm/npm-all/api/npm/npm-all"

Expected Behavior:
- Inputs: 
- `registry=https://internal.mycompany.com/artifactory/api/npm/npm-all`
   - `replace-registry-host=always`
- resolved: "https://external.mycompany.com/artifactory/api/npm/npm-all"
(package-lock.json)

- Output: "https://internal.mycompany.com/artifactory/api/npm/npm-all"

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!

Author: billy-briggs-dev

workspaces/arborist/lib/arborist/reify.js

@@ -8,7 +8,6 @@ const semver = require('semver')
 const debug = require('../debug.js')
 const { walkUp } = require('walk-up-path')
 const { log, time } = require('proc-log')
-const hgi = require('hosted-git-info')
 const rpj = require('read-package-json-fast')
 
 const { dirname, resolve, relative, join } = require('node:path')
@@ -833,21 +832,23 @@ module.exports = cls => class Reifier extends cls {
     // ${REGISTRY} or something.  This has to be threaded through the
     // Shrinkwrap and Node classes carefully, so for now, just treat
     // the default reg as the magical animal that it has been.
-    const resolvedURL = hgi.parseUrl(resolved)
-
-    if (!resolvedURL) {
+    try {
+      const resolvedURL = new URL(resolved)
+
+      if ((this.options.replaceRegistryHost === resolvedURL.hostname) ||
+           this.options.replaceRegistryHost === 'always') {
+        const registryURL = new URL(this.registry)
+        // Replace the host with the registry host while keeping the path intact
+        resolvedURL.hostname = registryURL.hostname
+        resolvedURL.port = registryURL.port
+        return resolvedURL.toString()
+      }
+      return resolved
+    } catch (e) {
       // if we could not parse the url at all then returning nothing
       // here means it will get removed from the tree in the next step
-      return
+      return undefined
     }
-
-    if ((this.options.replaceRegistryHost === resolvedURL.hostname)
-      || this.options.replaceRegistryHost === 'always') {
-      // this.registry always has a trailing slash
-      return `${this.registry.slice(0, -1)}${resolvedURL.pathname}${resolvedURL.searchParams}`
-    }
-
-    return resolved
   }
 
   // bundles are *sort of* like shrinkwraps, in that the branch is defined

workspaces/arborist/test/arborist/reify.js

@@ -3171,7 +3171,7 @@ t.test('installLinks', (t) => {
   t.end()
 })
 
-t.only('should preserve exact ranges, missing actual tree', async (t) => {
+t.test('should preserve exact ranges, missing actual tree', async (t) => {
   const Pacote = require('pacote')
   const Arborist = t.mock('../../lib/arborist', {
     pacote: {
@@ -3256,7 +3256,7 @@ t.only('should preserve exact ranges, missing actual tree', async (t) => {
     },
   })
 
-  t.only('host should not be replaced replaceRegistryHost=never', async (t) => {
+  t.test('host should not be replaced replaceRegistryHost=never', async (t) => {
     const testdir = t.testdir({
       project: {
         'package.json': JSON.stringify({
@@ -3296,7 +3296,7 @@ t.only('should preserve exact ranges, missing actual tree', async (t) => {
     await arb.reify()
   })
 
-  t.only('host should be replaced replaceRegistryHost=npmjs', async (t) => {
+  t.test('host should be replaced replaceRegistryHost=npmjs', async (t) => {
     const testdir = t.testdir({
       project: {
         'package.json': JSON.stringify({
@@ -3336,7 +3336,7 @@ t.only('should preserve exact ranges, missing actual tree', async (t) => {
     await arb.reify()
   })
 
-  t.only('host should be always replaceRegistryHost=always', async (t) => {
+  t.test('host should be always replaceRegistryHost=always', async (t) => {
     const testdir = t.testdir({
       project: {
         'package.json': JSON.stringify({
@@ -3375,6 +3375,52 @@ t.only('should preserve exact ranges, missing actual tree', async (t) => {
     })
     await arb.reify()
   })
+
+  t.test('registry with path should only swap hostname', async (t) => {
+    const abbrevPackument3 = JSON.stringify({
+      _id: 'abbrev',
+      _rev: 'lkjadflkjasdf',
+      name: 'abbrev',
+      'dist-tags': { latest: '1.1.1' },
+      versions: {
+        '1.1.1': {
+          name: 'abbrev',
+          version: '1.1.1',
+          dist: {
+            tarball: 'https://artifactory.example.com/api/npm/npm-all/abbrev/-/abbrev-1.1.1.tgz',
+          },
+        },
+      },
+    })
+
+    const testdir = t.testdir({
+      project: {
+        'package.json': JSON.stringify({
+          name: 'myproject',
+          version: '1.0.0',
+          dependencies: {
+            abbrev: '1.1.1',
+          },
+        }),
+      },
+    })
+
+    tnock(t, 'https://new-host.artifactory.example.com')
+      .get('/api/npm/npm-all/abbrev')
+      .reply(200, abbrevPackument3)
+
+    tnock(t, 'https://new-host.artifactory.example.com')
+      .get('/api/npm/npm-all/abbrev/-/abbrev-1.1.1.tgz')
+      .reply(200, abbrevTGZ)
+
+    const arb = new Arborist({
+      path: resolve(testdir, 'project'),
+      registry: 'https://new-host.artifactory.example.com/api/npm/npm-all',
+      cache: resolve(testdir, 'cache'),
+      replaceRegistryHost: 'always',
+    })
+    await arb.reify()
+  })
 })
 
 t.test('install stategy linked', async (t) => {