fix: calculating tarball location when registry has no trailing slash (…

…#4053) close #4052
pnpm · Dec 1, 2021 · 3cf543f · 3cf543f
1 parent 8a99a01
commit 3cf543f
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 6 deletions.
diff --git a/.changeset/quiet-bears-hang.md b/.changeset/quiet-bears-hang.md
@@ -0,0 +1,7 @@
+---
+"@pnpm/lockfile-utils": patch
+"@pnpm/resolve-dependencies": patch
+"pnpm": patch
+---
+
+Non-standard tarball URL should be correctly calculated when the registry has no traling slash in the configuration file [#4052](https://github.com/pnpm/pnpm/issues/4052). This is a regression caused introduced in v6.23.2 caused by [#4032](https://github.com/pnpm/pnpm/pull/4032).
diff --git a/packages/lockfile-utils/src/pkgSnapshotToResolution.ts b/packages/lockfile-utils/src/pkgSnapshotToResolution.ts
@@ -16,14 +16,16 @@ export default (
     return pkgSnapshot.resolution as Resolution
   }
   const { name } = nameVerFromPkgSnapshot(depPath, pkgSnapshot)
-  const registry = pkgSnapshot.resolution['registry'] ||
+  const registry: string = pkgSnapshot.resolution['registry'] ||
     (name[0] === '@' && registries[name.split('/')[0]]) ||
     registries.default
   let tarball!: string
   if (!pkgSnapshot.resolution['tarball']) {
     tarball = getTarball(registry)
   } else {
-    tarball = new url.URL(pkgSnapshot.resolution['tarball'], registry).toString()
+    tarball = new url.URL(pkgSnapshot.resolution['tarball'],
+      registry.endsWith('/') ? registry : `${registry}/`
+    ).toString()
   }
   return {
     ...pkgSnapshot.resolution,

diff --git a/packages/lockfile-utils/test/pkgSnapshotToResolution.ts b/packages/lockfile-utils/test/pkgSnapshotToResolution.ts
@@ -22,6 +22,17 @@ test('pkgSnapshotToResolution()', () => {
     tarball: 'https://mycompany.jfrog.io/mycompany/api/npm/npm-local/@mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz',
   })
 
+  expect(pkgSnapshotToResolution('/@mycompany/mypackage/2.0.0', {
+    resolution: {
+      integrity: 'AAAA',
+      tarball: '@mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz',
+    },
+  }, { default: 'https://registry.npmjs.org/', '@mycompany': 'https://mycompany.jfrog.io/mycompany/api/npm/npm-local' })).toEqual({
+    integrity: 'AAAA',
+    registry: 'https://mycompany.jfrog.io/mycompany/api/npm/npm-local',
+    tarball: 'https://mycompany.jfrog.io/mycompany/api/npm/npm-local/@mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz',
+  })
+
   expect(pkgSnapshotToResolution('/foo/1.0.0', {
     resolution: {
       integrity: 'AAAA',

diff --git a/packages/resolve-dependencies/src/updateLockfile.ts b/packages/resolve-dependencies/src/updateLockfile.ts
@@ -246,7 +246,7 @@ function removeProtocol (url: string) {
   return url.split('://')[1]
 }
 
-function relativeTarball (tarball: string, registry: string) {
+export function relativeTarball (tarball: string, registry: string) {
   // It is important to save the tarball URL as "relative-path" (without the leading '/').
   // Sometimes registries are located in a subdirectory of a website.
   // For instance, https://mycompany.jfrog.io/mycompany/api/npm/npm-local/
@@ -255,8 +255,10 @@ function relativeTarball (tarball: string, registry: string) {
   // So we add @mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz
   // not /@mycompany/mypackage/-/@mycompany/mypackage-2.0.0.tgz
   // Related issue: https://github.com/pnpm/pnpm/issues/1827
-  if (tarball.substr(0, registry.length) === registry) {
-    return tarball.substr(registry.length)
+  if (tarball.substr(0, registry.length) !== registry) {
+    return tarball
   }
-  return tarball
+  const relative = tarball.substr(registry.length)
+  if (relative[0] === '/') return relative.substring(1)
+  return relative
 }
diff --git a/packages/resolve-dependencies/test/relativeTarball.test.ts b/packages/resolve-dependencies/test/relativeTarball.test.ts
@@ -0,0 +1,7 @@
+/// <reference path="../../../typings/index.d.ts" />
+import { relativeTarball } from '@pnpm/resolve-dependencies/lib/updateLockfile'
+
+test('relativeTarball()', () => {
+  expect(relativeTarball('https://registry.com/foo/bar.tgz', 'https://registry.com/foo')).toBe('bar.tgz')
+  expect(relativeTarball('https://registry.com/foo/bar.tgz', 'https://registry.com/foo/')).toBe('bar.tgz')
+})