Skip to content

Commit

Permalink
Fix offline mirror file collision with internal registries and scoped…
Browse files Browse the repository at this point in the history
… packages

When using artifactory as an internal npm registry in conjunction with
scoped packages, yarn install was failing (about 50% of the time) with
messages of the form

  Hashes don't match when extracting file
  "https://artifactory.internal.site:443/artifactory/api/npm/external-mirror/@types/react/-/react-15.6.4.tgz".
  Expected "3bb57bd43183a05919ceb025a264287348f47e9d" but got
  "da39a3ee5e6b4b0d3255bfef95601890afd80709"

The problem was that yarn was writing both @types/react-15.6.4.tgz
and react-15.6.4.tgz to the same location within the offline mirror due
to making an assumption about no additional leading paths before the
scope name in the URL -- an assumption which was true for
registry.npmjs.org, but not for artifactory npm repositories.
  • Loading branch information
newren committed Nov 2, 2017
1 parent 1c845bd commit b85ab09
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
22 changes: 22 additions & 0 deletions __tests__/fetchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,3 +233,25 @@ test('TarballFetcher.fetch properly stores tarball of scoped package in offline
const exists = await fs.exists(path.join(offlineMirrorDir, '@exponent-configurator-1.0.2.tgz'));
expect(exists).toBe(true);
});

test('TarballFetcher.tarball mirror path correctly chosen for scoped package resolved from internal artifactory registry', async () => {
const dir = await mkdir('tarball-fetcher');
const offlineMirrorDir = await mkdir('offline-mirror');

const config = await Config.create();
config.registries.npm.config['yarn-offline-mirror'] = offlineMirrorDir;

const fetcher = new TarballFetcher(
dir,
{
type: 'tarball',
hash: '6f0ab73cdd7b82d8e81e80838b49e9e4c7fbcc44',
reference:
'https://artifactory.internal.site:443/artifactory/api/npm/external-mirror/@exponent/configurator/-/configurator-1.0.2.tgz',
registry: 'npm',
},
config,
);

expect(fetcher.getTarballMirrorPath()).toBe(path.join(offlineMirrorDir, '@exponent-configurator-1.0.2.tgz'));
});
15 changes: 10 additions & 5 deletions src/fetchers/tarball-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,18 @@ export default class TarballFetcher extends BaseFetcher {
return null;
}

// handle scoped packages
const pathParts = pathname.replace(/^\//, '').split(/\//g);
const tarballBasename = pathParts[pathParts.length - 1];

const packageFilename =
pathParts.length >= 2 && pathParts[0][0] === '@'
? `${pathParts[0]}-${pathParts[pathParts.length - 1]}` // scopped
: `${pathParts[pathParts.length - 1]}`;
// handle scoped packages; the scope is the 4th-to-last path, as
// seen in the following examples:
// 'https://registry.npmjs.org/@exponent/configurator/-/configurator-1.0.2.tgz'
// 'https://artifactory.internal.site:443/' +
// 'artifactory/api/npm/release-repo/@exponent/configurator/-/configurator-1.0.2.tgz'

const scopeName = pathParts.length >= 4 ? pathParts[pathParts.length - 4] : '';
const hasScopeName = scopeName !== '' && scopeName[0] == '@';
const packageFilename = hasScopeName ? `${scopeName}-${tarballBasename}` : tarballBasename;

return this.config.getOfflineMirrorPath(packageFilename);
}
Expand Down

0 comments on commit b85ab09

Please sign in to comment.