From f2cdfcf33ad2bd3bd1acdba0326281089f53c5b1 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 26 Feb 2020 14:20:02 -0800 Subject: [PATCH 1/2] fix: Do not pass scp-style URLs to the WhatWG url.URL Fix #60 (for the legacy branch) --- index.js | 19 ++++++++++++++++--- package.json | 2 +- test/basic.js | 2 ++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index 301f5d4..b57880c 100644 --- a/index.js +++ b/index.js @@ -109,9 +109,22 @@ function parseGitUrl (giturl) { if (!matched) { var legacy = url.parse(giturl) if (legacy.auth) { - var whatwg = new url.URL(giturl) - legacy.auth = whatwg.username || '' - if (whatwg.password) legacy.auth += ':' + whatwg.password + // git urls can be in the form of scp-style/ssh-connect strings, like + // git+ssh://user@host.com:some/path, which the legacy url parser + // supports, but WhatWG url.URL class does not. However, the legacy + // parser de-urlencodes the username and password, so something like + // https://user%3An%40me:p%40ss%3Aword@x.com/ becomes + // https://user:n@me:p@ss:word@x.com/ which is all kinds of wrong. + // Pull off just the auth and host, so we dont' get the confusing + // scp-style URL, then pass that to the WhatWG parser to get the + // auth properly escaped. + const authmatch = giturl.match(/[^@]+@[^:/]+/) + /* istanbul ignore else - this should be impossible */ + if (authmatch) { + var whatwg = new url.URL(authmatch[0]) + legacy.auth = whatwg.username || '' + if (whatwg.password) legacy.auth += ':' + whatwg.password + } } return legacy } diff --git a/package.json b/package.json index 5afd1a5..2f70777 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "scripts": { "prerelease": "npm t", "postrelease": "npm publish --tag=ancient-legacy-fixes && git push --follow-tags", - "pretest": "standard", + "posttest": "standard", "release": "standard-version -s", "test:coverage": "tap --coverage-report=html -J --100 --no-esm test/*.js", "test": "tap -J --100 --no-esm test/*.js" diff --git a/test/basic.js b/test/basic.js index 76a8d6f..e41b637 100644 --- a/test/basic.js +++ b/test/basic.js @@ -37,6 +37,8 @@ test('basic', function (t) { t.is(HostedGit.fromUrl('github.com/abc/def/'), undefined, 'forgot the protocol') t.is(HostedGit.fromUrl('completely-invalid'), undefined, 'not a url is not hosted') + t.is(HostedGit.fromUrl('git+ssh://git@git.unlucky.com:RND/electron-tools/some-tool#2.0.1'), undefined, 'properly ignores non-hosted scp style urls') + t.is(HostedGit.fromUrl('http://github.com/foo/bar').toString(), 'git+ssh://git@github.com/foo/bar.git', 'github http protocol use git+ssh urls') t.end() }) From bb123d285cac851110fc3b7a0055646f5341f656 Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 26 Feb 2020 14:30:20 -0800 Subject: [PATCH 2/2] fix: Do not attempt to use url.URL when unavailable Fix #61 This should not be ported to the latest branch, as Node.js v6 support was dropped there anyway. --- index.js | 4 +++- test/auth.js | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/index.js b/index.js index b57880c..08fa329 100644 --- a/index.js +++ b/index.js @@ -108,7 +108,9 @@ function parseGitUrl (giturl) { var matched = giturl.match(/^([^@]+)@([^:/]+):[/]?((?:[^/]+[/])?[^/]+?)(?:[.]git)?(#.*)?$/) if (!matched) { var legacy = url.parse(giturl) - if (legacy.auth) { + // If we don't have url.URL, then sorry, this is just not fixable. + // This affects Node <= 6.12. + if (legacy.auth && typeof url.URL === 'function') { // git urls can be in the form of scp-style/ssh-connect strings, like // git+ssh://user@host.com:some/path, which the legacy url parser // supports, but WhatWG url.URL class does not. However, the legacy diff --git a/test/auth.js b/test/auth.js index 0e5c752..1fda1dd 100644 --- a/test/auth.js +++ b/test/auth.js @@ -16,3 +16,9 @@ tap.equal(parsedUrl.hostname, 'github.com') // For full backwards-compatibility; support auth where only username or only password is provided tap.equal(HostedGitInfo.fromUrl('https://user%3An%40me@github.com/npm/hosted-git-info.git').auth, 'user%3An%40me') tap.equal(HostedGitInfo.fromUrl('https://:p%40ss%3Aword@github.com/npm/hosted-git-info.git').auth, ':p%40ss%3Aword') + +// don't try to url.URL parse it if url.URL is not available +// ie, node <6.13. This is broken, but at least it doesn't throw. +url.URL = null +var parsedInfo = HostedGitInfo.fromUrl('https://user%3An%40me:p%40ss%3Aword@github.com/npm/xyz.git') +tap.equal(parsedInfo.auth, 'user:n@me:p@ss:word')