diff --git a/__tests__/resolvers/exotics/bitbucket-resolver.js b/__tests__/resolvers/exotics/bitbucket-resolver.js index 168be27b39..de7f6c18ae 100644 --- a/__tests__/resolvers/exotics/bitbucket-resolver.js +++ b/__tests__/resolvers/exotics/bitbucket-resolver.js @@ -120,5 +120,5 @@ test('getGitSSHUrl should return URL containing protocol', () => { }); expect(url.parse(gitSSHUrl).protocol).toEqual('git+ssh:'); - expect(url.parse(Git.cleanUrl(gitSSHUrl)).protocol).toEqual('ssh:'); + expect(Git.npmUrlToGitUrl(gitSSHUrl).protocol).toEqual('ssh:'); }); diff --git a/__tests__/resolvers/exotics/git-resolver.js b/__tests__/resolvers/exotics/git-resolver.js deleted file mode 100644 index e0ff5c1b8a..0000000000 --- a/__tests__/resolvers/exotics/git-resolver.js +++ /dev/null @@ -1,45 +0,0 @@ -/* @flow */ - -import {parse} from 'url'; - -import GitResolver from '../../../src/resolvers/exotics/git-resolver.js'; - -test('GitResolver transformUrl method is defined', () => { - expect(GitResolver.transformUrl).toBeDefined(); -}); - -test('GitResolver transformUrl does not affect normal urls', () => { - - const urls = [ - 'git+https://github.com/npm-ml/ocaml.git#npm-4.02.3', - 'git+ssh://git@gitlab.com/user/repo.git', - 'git+ssh://git@github.com/user/repo.git', - 'git+ssh://username@private.url/sub/right-pad', - 'https://github.com/npm-ml/re', - 'https://github.com/npm-ml/ocaml.git#npm-4.02.3', - 'https://git@github.com/stevemao/left-pad.git', - 'https://bitbucket.org/hgarcia/node-bitbucket-api.git', - 'https://github.com/yarnpkg/yarn/releases/download/v0.18.1/yarn-v0.18.1.tar.gz', - 'https://github.com/babel/babel-loader.git#greenkeeper/cross-env-3.1.4', - 'package@git@bitbucket.org:team/repo.git', - ]; - - urls.forEach((url) => { - expect(GitResolver.transformUrl(parse(url))).toBe(url); - }); - -}); - -test('GitResolver transformUrl affect host colon separated urls', () => { - - const urls = [ - 'git+ssh://username@private.url:sub/right-pad', - 'git+ssh://private.url:sub/right-pad', - 'https://private.url:sub/right-pad', - ]; - - urls.forEach((url) => { - expect(GitResolver.transformUrl(parse(url))).toBe(url.replace(':sub', '/sub')); - }); - -}); diff --git a/__tests__/resolvers/exotics/github-resolver.js b/__tests__/resolvers/exotics/github-resolver.js index 368a373433..ad360fab28 100644 --- a/__tests__/resolvers/exotics/github-resolver.js +++ b/__tests__/resolvers/exotics/github-resolver.js @@ -46,5 +46,5 @@ test('getGitSSHUrl should return URL containing protocol', () => { }); expect(url.parse(gitSSHUrl).protocol).toEqual('git+ssh:'); - expect(url.parse(Git.cleanUrl(gitSSHUrl)).protocol).toEqual('ssh:'); + expect(Git.npmUrlToGitUrl(gitSSHUrl).protocol).toEqual('ssh:'); }); diff --git a/__tests__/resolvers/exotics/gitlab-resolver.js b/__tests__/resolvers/exotics/gitlab-resolver.js index 0cc468b126..3c7d1816f4 100644 --- a/__tests__/resolvers/exotics/gitlab-resolver.js +++ b/__tests__/resolvers/exotics/gitlab-resolver.js @@ -46,5 +46,5 @@ test('getGitSSHUrl should return URL containing protocol', () => { }); expect(url.parse(gitSSHUrl).protocol).toEqual('git+ssh:'); - expect(url.parse(Git.cleanUrl(gitSSHUrl)).protocol).toEqual('ssh:'); + expect(Git.npmUrlToGitUrl(gitSSHUrl).protocol).toEqual('ssh:'); }); diff --git a/__tests__/util/git.js b/__tests__/util/git.js index 1e3790864e..20bcb556fd 100644 --- a/__tests__/util/git.js +++ b/__tests__/util/git.js @@ -5,13 +5,43 @@ import {NoopReporter} from '../../src/reporters/index.js'; jasmine.DEFAULT_TIMEOUT_INTERVAL = 90000; -test('cleanUrl', () => { - expect(Git.cleanUrl('git+https://github.com/npm-opam/ocamlfind.git')) - .toEqual('https://github.com/npm-opam/ocamlfind.git'); - expect(Git.cleanUrl('https://github.com/npm-opam/ocamlfind.git')) - .toEqual('https://github.com/npm-opam/ocamlfind.git'); - expect(Git.cleanUrl('git://github.com/npm-opam/ocamlfind.git')) - .toEqual('git://github.com/npm-opam/ocamlfind.git'); +test('npmUrlToGitUrl', () => { + expect(Git.npmUrlToGitUrl('git+https://github.com/npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'https:', + hostname: 'github.com', + repository: 'https://github.com/npm-opam/ocamlfind.git', + }); + expect(Git.npmUrlToGitUrl('https://github.com/npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'https:', + hostname: 'github.com', + repository: 'https://github.com/npm-opam/ocamlfind.git', + }); + expect(Git.npmUrlToGitUrl('git://github.com/npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'git:', + hostname: 'github.com', + repository: 'git://github.com/npm-opam/ocamlfind.git', + }); + expect(Git.npmUrlToGitUrl('git+ssh://git@github.com/npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'ssh:', + hostname: 'github.com', + repository: 'ssh://git@github.com/npm-opam/ocamlfind.git', + }); + expect(Git.npmUrlToGitUrl('git+ssh://scp-host-nickname:npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'ssh:', + hostname: 'scp-host-nickname', + repository: 'scp-host-nickname:npm-opam/ocamlfind.git', + }); + expect(Git.npmUrlToGitUrl('git+ssh://user@scp-host-nickname:npm-opam/ocamlfind.git')) + .toEqual({ + protocol: 'ssh:', + hostname: 'scp-host-nickname', + repository: 'user@scp-host-nickname:npm-opam/ocamlfind.git', + }); }); test('isCommitHash', () => { @@ -30,25 +60,25 @@ test('isCommitHash', () => { }); -test('secureUrl', async function (): Promise { +test('secureGitUrl', async function (): Promise { const reporter = new NoopReporter(); let hasException = false; try { - await Git.secureUrl('http://fake-fake-fake-fake.com/123.git', '', reporter); + await Git.secureGitUrl(Git.npmUrlToGitUrl('http://fake-fake-fake-fake.com/123.git'), '', reporter); } catch (e) { hasException = true; } expect(hasException).toEqual(true); - let url = await Git.secureUrl('http://github.com/yarnpkg/yarn.git', '', reporter); - expect(url).toEqual('https://github.com/yarnpkg/yarn.git'); + let gitURL = await Git.secureGitUrl(Git.npmUrlToGitUrl('http://github.com/yarnpkg/yarn.git'), '', reporter); + expect(gitURL.repository).toEqual('https://github.com/yarnpkg/yarn.git'); - url = await Git.secureUrl('https://github.com/yarnpkg/yarn.git', '', reporter); - expect(url).toEqual('https://github.com/yarnpkg/yarn.git'); + gitURL = await Git.secureGitUrl(Git.npmUrlToGitUrl('https://github.com/yarnpkg/yarn.git'), '', reporter); + expect(gitURL.repository).toEqual('https://github.com/yarnpkg/yarn.git'); - url = await Git.secureUrl('git://github.com/yarnpkg/yarn.git', '', reporter); - expect(url).toEqual('https://github.com/yarnpkg/yarn.git'); + gitURL = await Git.secureGitUrl(Git.npmUrlToGitUrl('git://github.com/yarnpkg/yarn.git'), '', reporter); + expect(gitURL.repository).toEqual('https://github.com/yarnpkg/yarn.git'); }, ); diff --git a/src/fetchers/git-fetcher.js b/src/fetchers/git-fetcher.js index 773fdb86bf..7e3f60d224 100644 --- a/src/fetchers/git-fetcher.js +++ b/src/fetchers/git-fetcher.js @@ -110,7 +110,8 @@ export default class GitFetcher extends BaseFetcher { const hash = this.hash; invariant(hash, 'Commit hash required'); - const git = new Git(this.config, this.reference, hash); + const gitUrl = Git.npmUrlToGitUrl(this.reference); + const git = new Git(this.config, gitUrl, hash); await git.init(); await git.clone(this.dest); diff --git a/src/resolvers/exotics/git-resolver.js b/src/resolvers/exotics/git-resolver.js index ba91a4b8ff..0791ab12f9 100644 --- a/src/resolvers/exotics/git-resolver.js +++ b/src/resolvers/exotics/git-resolver.js @@ -11,7 +11,6 @@ import ExoticResolver from './exotic-resolver.js'; import Git from '../../util/git.js'; const urlParse = require('url').parse; -const urlFormat = require('url').format; // we purposefully omit https and http as those are only valid if they end in the .git extension const GIT_PROTOCOLS = ['git:', 'git+ssh:', 'git+https:', 'ssh:']; @@ -61,14 +60,6 @@ export default class GitResolver extends ExoticResolver { return false; } - // This transformUrl util is here to replace colon separators in the pathname - // from private urls. It takes the url parts retrieved using urlFormat and - // returns the associated url. Related to #573, introduced in #2519. - static transformUrl(parts) : string { - const pathname = parts.pathname ? parts.pathname.replace(/^\/:/, '/') : ''; - return urlFormat({...parts, pathname}); - } - async resolve(forked?: true): Promise { const {url} = this; @@ -102,9 +93,8 @@ export default class GitResolver extends ExoticResolver { const {config} = this; - const transformedUrl = GitResolver.transformUrl(parts); - - const client = new Git(config, transformedUrl, this.hash); + const gitUrl = Git.npmUrlToGitUrl(url); + const client = new Git(config, gitUrl, this.hash); const commit = await client.init(); async function tryRegistry(registry): Promise { @@ -115,12 +105,12 @@ export default class GitResolver extends ExoticResolver { return null; } - const json = await config.readJson(`${transformedUrl}/${filename}`, () => JSON.parse(file)); + const json = await config.readJson(`${url}/${filename}`, () => JSON.parse(file)); json._uid = commit; json._remote = { - resolved: `${transformedUrl}#${commit}`, + resolved: `${url}#${commit}`, type: 'git', - reference: transformedUrl, + reference: url, hash: commit, registry, }; @@ -143,6 +133,6 @@ export default class GitResolver extends ExoticResolver { } } - throw new MessageError(this.reporter.lang('couldntFindManifestIn', transformedUrl)); + throw new MessageError(this.reporter.lang('couldntFindManifestIn', url)); } } diff --git a/src/resolvers/exotics/hosted-git-resolver.js b/src/resolvers/exotics/hosted-git-resolver.js index 0be7d9c49e..f8797ea361 100644 --- a/src/resolvers/exotics/hosted-git-resolver.js +++ b/src/resolvers/exotics/hosted-git-resolver.js @@ -104,7 +104,8 @@ export default class HostedGitResolver extends ExoticResolver { } async getRefOverHTTP(url: string): Promise { - const client = new Git(this.config, url, this.hash); + const gitUrl = Git.npmUrlToGitUrl(url); + const client = new Git(this.config, gitUrl, this.hash); let out = await this.config.requestManager.request({ url: `${url}/info/refs?service=git-upload-pack`, @@ -211,8 +212,9 @@ export default class HostedGitResolver extends ExoticResolver { // NOTE: Here we use a different url than when we delegate to the git resolver later on. // This is because `git archive` requires access over ssh and github only allows that // if you have write permissions - if (await Git.hasArchiveCapability(sshUrl)) { - const archiveClient = new Git(this.config, sshUrl, this.hash); + const sshGitUrl = Git.npmUrlToGitUrl(sshUrl); + if (await Git.hasArchiveCapability(sshGitUrl)) { + const archiveClient = new Git(this.config, sshGitUrl, this.hash); const commit = await archiveClient.init(); return await this.fork(GitResolver, true, `${sshUrl}#${commit}`); } diff --git a/src/util/git.js b/src/util/git.js index 3d3f4ce542..d388972a59 100644 --- a/src/util/git.js +++ b/src/util/git.js @@ -21,20 +21,26 @@ type GitRefs = { [name: string]: string }; -const supportsArchiveCache: { [key: string]: ?boolean } = map({ +type GitUrl = { + protocol: string, // parsed from URL + hostname: ?string, + repository: string, // git-specific "URL" +}; + +const supportsArchiveCache: { [key: string]: boolean } = map({ 'github.com': false, // not support, doubt they will ever support it }); export default class Git { - constructor(config: Config, url: string, hash: string) { + constructor(config: Config, gitUrl: GitUrl, hash: string) { this.supportsArchive = false; this.fetched = false; this.config = config; this.reporter = config.reporter; this.hash = hash; this.ref = hash; - this.url = Git.cleanUrl(url); - this.cwd = this.config.getTemp(crypto.hash(this.url)); + this.gitUrl = gitUrl; + this.cwd = this.config.getTemp(crypto.hash(this.gitUrl.repository)); } supportsArchive: boolean; @@ -44,32 +50,49 @@ export default class Git { hash: string; ref: string; cwd: string; - url: string; + gitUrl: GitUrl; + /** + * npm URLs contain a 'git+' scheme prefix, which is not understood by git. + * git "URLs" also allow an alternative scp-like syntax, so they're not standard URLs. + */ + static npmUrlToGitUrl(npmUrl: string): GitUrl { + // Special case in npm, where ssh:// prefix is stripped to pass scp-like syntax + // which works as remote path only if there are no slashes before ':' + const match = npmUrl.match(/^git\+ssh:\/\/((?:[^@:\/]+@)?([^@:\/]+):.*)/); + if (match) { + return { + protocol: 'ssh:', + hostname: match[2], + repository: match[1], + }; + } - static cleanUrl(url): string { - return url.replace(/^git\+/, ''); + const repository = npmUrl.replace(/^git\+/, ''); + const parsed = url.parse(repository); + return { + protocol: parsed.protocol || 'file:', + hostname: parsed.hostname || null, + repository, + }; } /** * Check if the host specified in the input `gitUrl` has archive capability. */ - static async hasArchiveCapability(gitUrl: string): Promise { - // USER@HOSTNAME:PATHNAME - const match = gitUrl.match(/^(.*?)@(.*?):(.*?)$/); - if (!match) { + static async hasArchiveCapability(ref: GitUrl): Promise { + const hostname = ref.hostname; + if (ref.protocol !== 'ssh:' || hostname == null) { return false; } - const [,, hostname] = match; - const cached = supportsArchiveCache[hostname]; - if (cached != null) { - return cached; + if (hostname in supportsArchiveCache) { + return supportsArchiveCache[hostname]; } try { - await child.spawn('git', ['archive', `--remote=${gitUrl}`, 'HEAD', Date.now() + '']); + await child.spawn('git', ['archive', `--remote=${ref.repository}`, 'HEAD', Date.now() + '']); throw new Error(); } catch (err) { const supports = err.message.indexOf('did not match any files') >= 0; @@ -85,29 +108,34 @@ export default class Git { return !!target && /^[a-f0-9]{5,40}$/.test(target); } - static async repoExists(gitUrl: string): Promise { + static async repoExists(ref: GitUrl): Promise { try { - await child.spawn('git', ['ls-remote', '-t', gitUrl]); + await child.spawn('git', ['ls-remote', '-t', ref.repository]); return true; } catch (err) { return false; } } + static replaceProtocol(ref: GitUrl, protocol: string): GitUrl { + return { + protocol, + hostname: ref.hostname, + repository: ref.repository.replace(/^(?:git|http):/, protocol), + }; + } + /** * Attempt to upgrade insecure protocols to secure protocol */ - - static async secureUrl(ref: string, hash: string, reporter: Reporter): Promise { + static async secureGitUrl(ref: GitUrl, hash: string, reporter: Reporter): Promise { if (Git.isCommitHash(hash)) { // this is cryptographically secure return ref; } - const parts = url.parse(ref); - - if (parts.protocol === 'git:') { - const secureUrl = ref.replace(/^git:/, 'https:'); + if (ref.protocol === 'git:') { + const secureUrl = Git.replaceProtocol(ref, 'https:'); if (await Git.repoExists(secureUrl)) { return secureUrl; } else { @@ -117,10 +145,10 @@ export default class Git { } } - if (parts.protocol === 'http:') { - const secureUrl = ref.replace(/^http:/, 'https:'); - if (await Git.repoExists(secureUrl)) { - return secureUrl; + if (ref.protocol === 'http:') { + const secureRef = Git.replaceProtocol(ref, 'https:'); + if (await Git.repoExists(secureRef)) { + return secureRef; } else { if (await Git.repoExists(ref)) { return ref; @@ -132,7 +160,7 @@ export default class Git { } } - if (parts.protocol === 'https:') { + if (ref.protocol === 'https:') { if (await Git.repoExists(ref)) { return ref; } else { @@ -159,7 +187,7 @@ export default class Git { async _archiveViaRemoteArchive(dest: string): Promise { const hashStream = new crypto.HashStream(); - await child.spawn('git', ['archive', `--remote=${this.url}`, this.ref], { + await child.spawn('git', ['archive', `--remote=${this.gitUrl.repository}`, this.ref], { process(proc, resolve, reject, done) { const writeStream = createWriteStream(dest); proc.on('error', reject); @@ -205,7 +233,7 @@ export default class Git { } async _cloneViaRemoteArchive(dest: string): Promise { - await child.spawn('git', ['archive', `--remote=${this.url}`, this.ref], { + await child.spawn('git', ['archive', `--remote=${this.gitUrl.repository}`, this.ref], { process(proc, update, reject, done) { const extractor = tarFs.extract(dest, { dmode: 0o555, // all dirs should be readable @@ -242,13 +270,13 @@ export default class Git { */ fetch(): Promise { - const {url, cwd} = this; + const {gitUrl, cwd} = this; - return fs.lockQueue.push(url, async () => { + return fs.lockQueue.push(gitUrl.repository, async () => { if (await fs.exists(cwd)) { await child.spawn('git', ['pull'], {cwd}); } else { - await child.spawn('git', ['clone', url, cwd]); + await child.spawn('git', ['clone', gitUrl.repository, cwd]); } this.fetched = true; @@ -286,7 +314,7 @@ export default class Git { async _getFileFromArchive(filename: string): Promise { try { - return await child.spawn('git', ['archive', `--remote=${this.url}`, this.ref, filename], { + return await child.spawn('git', ['archive', `--remote=${this.gitUrl.repository}`, this.ref, filename], { process(proc, update, reject, done) { const parser = tarStream.extract(); @@ -337,9 +365,9 @@ export default class Git { * set the ref to match an input `target`. */ async init(): Promise { - this.url = await Git.secureUrl(this.url, this.hash, this.reporter); + this.gitUrl = await Git.secureGitUrl(this.gitUrl, this.hash, this.reporter); // check capabilities - if (await Git.hasArchiveCapability(this.url)) { + if (await Git.hasArchiveCapability(this.gitUrl)) { this.supportsArchive = true; } else { await this.fetch(); @@ -349,7 +377,7 @@ export default class Git { } async setRefRemote(): Promise { - const stdout = await child.spawn('git', ['ls-remote', '--tags', '--heads', this.url]); + const stdout = await child.spawn('git', ['ls-remote', '--tags', '--heads', this.gitUrl.repository]); const refs = Git.parseRefs(stdout); return await this.setRef(refs); } @@ -388,7 +416,7 @@ export default class Git { this.ref = ref; return this.hash = commit; } else { - throw new MessageError(this.reporter.lang('couldntFindMatch', ref, names.join(','), this.url)); + throw new MessageError(this.reporter.lang('couldntFindMatch', ref, names.join(','), this.gitUrl.repository)); } }