Skip to content

Commit

Permalink
fix: do not allow invalid gist urls
Browse files Browse the repository at this point in the history
Gists are either name/hex{32}, or just /hex{32}.

If someone omits the project, and just specifies a name, then that is an
error and should be treated as such.

Also, eliminate some dead code paths and bring the tests up to 100%.
  • Loading branch information
isaacs committed Aug 4, 2019
1 parent e518222 commit d5cf830
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 59 deletions.
2 changes: 1 addition & 1 deletion git-host-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ var gitHosts = module.exports = {
gist: {
'protocols': [ 'git', 'git+ssh', 'git+https', 'ssh', 'https' ],
'domain': 'gist.github.com',
'pathmatch': /^[/](?:([^/]+)[/])?([a-z0-9]+)(?:[.]git)?$/,
'pathmatch': /^[/](?:([^/]+)[/])?([a-z0-9]{32,})(?:[.]git)?$/,
'filetemplate': 'https://gist.githubusercontent.com/{user}/{project}/raw{/committish}/{path}',
'bugstemplate': 'https://{domain}/{project}',
'gittemplate': 'git://{domain}/{project}.git{#committish}',
Expand Down
4 changes: 3 additions & 1 deletion git-host.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'
var gitHosts = require('./git-host-info.js')
/* eslint-disable node/no-deprecated-api */
/* istanbul ignore next */
var extend = Object.assign || require('util')._extend

var GitHost = module.exports = function (type, user, auth, project, committish, defaultRepresentation, opts) {
Expand Down Expand Up @@ -127,5 +128,6 @@ GitHost.prototype.getDefaultRepresentation = function () {
}

GitHost.prototype.toString = function (opts) {
return (this[this.default] || this.sshurl).call(this, opts)
const method = this.default || /* istanbul ignore next */ 'sshurl'
return this[method](opts)
}
22 changes: 12 additions & 10 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ var LRU = require('lru-cache')
var cache = new LRU({max: 1000})

var protocolToRepresentationMap = {
'git+ssh': 'sshurl',
'git+https': 'https',
'ssh': 'sshurl',
'git': 'git'
'git+ssh:': 'sshurl',
'git+https:': 'https',
'ssh:': 'sshurl',
'git:': 'git'
}

function protocolToRepresentation (protocol) {
if (protocol.substr(-1) === ':') protocol = protocol.slice(0, -1)
return protocolToRepresentationMap[protocol] || protocol
return protocolToRepresentationMap[protocol] || protocol.slice(0, -1)
}

var authProtocols = {
Expand Down Expand Up @@ -65,13 +64,17 @@ function fromUrl (giturl, opts) {
var pathmatch = gitHostInfo.pathmatch
var matched = parsed.path.match(pathmatch)
if (!matched) return
if (matched[1] != null) user = decodeURIComponent(matched[1].replace(/^:/, ''))
if (matched[2] != null) project = decodeURIComponent(matched[2])
if (matched[1] !== null && matched[1] !== undefined) {
user = decodeURIComponent(matched[1].replace(/^:/, ''))
}
project = decodeURIComponent(matched[2])
defaultRepresentation = protocolToRepresentation(parsed.protocol)
}
return new GitHost(gitHostName, user, auth, project, committish, defaultRepresentation, opts)
} catch (ex) {
if (!(ex instanceof URIError)) throw ex
/* istanbul ignore else */
if (ex instanceof URIError) {
} else throw ex
}
}).filter(function (gitHostInfo) { return gitHostInfo })
if (matches.length !== 1) return
Expand Down Expand Up @@ -101,7 +104,6 @@ function fixupUnqualifiedGist (giturl) {
}

function parseGitUrl (giturl) {
if (typeof giturl !== 'string') giturl = '' + giturl
var matched = giturl.match(/^([^@]+)@([^:/]+):[/]?((?:[^/]+[/])?[^/]+?)(?:[.]git)?(#.*)?$/)
if (!matched) return url.parse(giturl)
return {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"postrelease": "npm publish && git push --follow-tags",
"pretest": "standard",
"release": "standard-version -s",
"test": "tap -J --nyc-arg=--all --coverage test"
"test": "tap -J --100 test/*.js"
},
"dependencies": {
"lru-cache": "^5.1.1"
Expand Down
47 changes: 24 additions & 23 deletions test/gist.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,42 +3,43 @@ var HostedGit = require('../index')
var test = require('tap').test

test('fromUrl(gist url)', function (t) {
var proj = new Array(33).join('2')
function verify (host, label, branch) {
var hostinfo = HostedGit.fromUrl(host)
var hash = branch ? '#' + branch : ''
t.ok(hostinfo, label)
if (!hostinfo) return
t.is(hostinfo.https(), 'git+https://gist.github.com/222.git' + hash, label + ' -> https')
t.is(hostinfo.git(), 'git://gist.github.com/222.git' + hash, label + ' -> git')
t.is(hostinfo.browse(), 'https://gist.github.com/222' + (branch ? '/' + branch : ''), label + ' -> browse')
t.is(hostinfo.browse('C'), 'https://gist.github.com/222' + (branch ? '/' + branch : '') + '#file-c', label + ' -> browse(path)')
t.is(hostinfo.browse('C', 'A'), 'https://gist.github.com/222' + (branch ? '/' + branch : '') + '#file-c', label + ' -> browse(path, fragment)')
t.is(hostinfo.bugs(), 'https://gist.github.com/222', label + ' -> bugs')
t.is(hostinfo.docs(), 'https://gist.github.com/222' + (branch ? '/' + branch : ''), label + ' -> docs')
t.is(hostinfo.ssh(), 'git@gist.github.com:/222.git' + hash, label + ' -> ssh')
t.is(hostinfo.sshurl(), 'git+ssh://git@gist.github.com/222.git' + hash, label + ' -> sshurl')
t.is(hostinfo.shortcut(), 'gist:222' + hash, label + ' -> shortcut')
t.is(hostinfo.https(), 'git+https://gist.github.com/' + proj + '.git' + hash, label + ' -> https')
t.is(hostinfo.git(), 'git://gist.github.com/' + proj + '.git' + hash, label + ' -> git')
t.is(hostinfo.browse(), 'https://gist.github.com/' + proj + (branch ? '/' + branch : ''), label + ' -> browse')
t.is(hostinfo.browse('C'), 'https://gist.github.com/' + proj + (branch ? '/' + branch : '') + '#file-c', label + ' -> browse(path)')
t.is(hostinfo.browse('C', 'A'), 'https://gist.github.com/' + proj + (branch ? '/' + branch : '') + '#file-c', label + ' -> browse(path, fragment)')
t.is(hostinfo.bugs(), 'https://gist.github.com/' + proj, label + ' -> bugs')
t.is(hostinfo.docs(), 'https://gist.github.com/' + proj + (branch ? '/' + branch : ''), label + ' -> docs')
t.is(hostinfo.ssh(), 'git@gist.github.com:/' + proj + '.git' + hash, label + ' -> ssh')
t.is(hostinfo.sshurl(), 'git+ssh://git@gist.github.com/' + proj + '.git' + hash, label + ' -> sshurl')
t.is(hostinfo.shortcut(), 'gist:' + proj + hash, label + ' -> shortcut')
if (hostinfo.user) {
t.is(hostinfo.file('C'), 'https://gist.githubusercontent.com/111/222/raw/' + (branch ? branch + '/' : '') + 'C', label + ' -> file')
t.is(hostinfo.tarball(), 'https://gist.github.com/111/222/archive/' + (branch || 'master') + '.tar.gz', label + ' -> tarball')
t.is(hostinfo.file('C'), 'https://gist.githubusercontent.com/111/' + proj + '/raw/' + (branch ? branch + '/' : '') + 'C', label + ' -> file')
t.is(hostinfo.tarball(), 'https://gist.github.com/111/' + proj + '/archive/' + (branch || 'master') + '.tar.gz', label + ' -> tarball')
}
}

verify('git@gist.github.com:222.git', 'git@')
var hostinfo = HostedGit.fromUrl('git@gist.github.com:/ef860c7z5e0de3179341.git')
verify('git@gist.github.com:' + proj + '.git', 'git@')
var hostinfo = HostedGit.fromUrl('git@gist.github.com:/c2b12db30a49324325a3781302668408.git')
if (t.ok(hostinfo, 'git@hex')) {
t.is(hostinfo.https(), 'git+https://gist.github.com/ef860c7z5e0de3179341.git', 'git@hex -> https')
t.is(hostinfo.https(), 'git+https://gist.github.com/c2b12db30a49324325a3781302668408.git', 'git@hex -> https')
}
verify('git@gist.github.com:/222.git', 'git@/')
verify('git://gist.github.com/222', 'git')
verify('git://gist.github.com/222.git', 'git.git')
verify('git://gist.github.com/222#branch', 'git#branch', 'branch')
verify('git://gist.github.com/222.git#branch', 'git.git#branch', 'branch')
verify('git@gist.github.com:/' + proj + '.git', 'git@/')
verify('git://gist.github.com/' + proj, 'git')
verify('git://gist.github.com/' + proj + '.git', 'git.git')
verify('git://gist.github.com/' + proj + '#branch', 'git#branch', 'branch')
verify('git://gist.github.com/' + proj + '.git#branch', 'git.git#branch', 'branch')

require('./lib/standard-tests')(verify, 'gist.github.com', 'gist')
require('./lib/standard-tests')(verify, 'gist.github.com', 'gist', proj)

verify(HostedGit.fromUrl('gist:111/222').toString(), 'round-tripped shortcut')
verify('gist:222', 'shortened shortcut')
verify(HostedGit.fromUrl('gist:111/' + proj).toString(), 'round-tripped shortcut')
verify('gist:' + proj, 'shortened shortcut')

t.end()
})
12 changes: 10 additions & 2 deletions test/github.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,23 @@ test('fromUrl(github url)', function (t) {
function verify (host, label, branch) {
var hostinfo = HostedGit.fromUrl(host)
var hash = branch ? '#' + branch : ''
var treebranch = branch ? '/tree/' + branch : ''
t.equal(hostinfo._fill(), undefined)
t.ok(hostinfo, label)
if (!hostinfo) return
t.is(hostinfo.https(), 'git+https://github.com/111/222.git' + hash, label + ' -> https')
t.is(hostinfo.git(), 'git://github.com/111/222.git' + hash, label + ' -> git')
t.is(hostinfo.browse(), 'https://github.com/111/222' + (branch ? '/tree/' + branch : ''), label + ' -> browse')
t.is(hostinfo.browse(), 'https://github.com/111/222' + treebranch, label + ' -> browse')
t.is(hostinfo.browse('C'), 'https://github.com/111/222/tree/' + (branch || 'master') + '/C', label + ' -> browse(path)')
t.is(hostinfo.browse('C', 'A'), 'https://github.com/111/222/tree/' + (branch || 'master') + '/C#a', label + ' -> browse(path, fragment)')
t.is(hostinfo.bugs(), 'https://github.com/111/222/issues', label + ' -> bugs')
t.is(hostinfo.docs(), 'https://github.com/111/222' + (branch ? '/tree/' + branch : '') + '#readme', label + ' -> docs')
t.is(hostinfo.docs(), 'https://github.com/111/222' + treebranch + '#readme', label + ' -> docs')
t.is(hostinfo.ssh(), 'git@github.com:111/222.git' + hash, label + ' -> ssh')
t.is(hostinfo.sshurl(), 'git+ssh://git@github.com/111/222.git' + hash, label + ' -> sshurl')
t.is(hostinfo.sshurl({ noGitPlus: true }), 'ssh://git@github.com/111/222.git' + hash, label + ' -> sshurl (no git plus)')
t.is(hostinfo.path(), '111/222' + hash, ' -> path')
t.is(hostinfo.hash(), hash, ' -> hash')
t.is(hostinfo.path({ noCommittish: true }), '111/222', ' -> path (no committish)')
t.is(hostinfo.shortcut(), 'github:111/222' + hash, label + ' -> shortcut')
t.is(hostinfo.file('C'), 'https://raw.githubusercontent.com/111/222/' + (branch || 'master') + '/C', label + ' -> file')
t.is(hostinfo.tarball(), 'https://codeload.github.com/111/222/tar.gz/' + (branch || 'master'), label + ' -> tarball')
Expand All @@ -41,5 +47,7 @@ test('fromUrl(github url)', function (t) {

require('./lib/standard-tests')(verify, 'www.github.com', 'github')

t.equal(HostedGit.fromUrl('git+ssh://github.com/foo.git'), undefined)

t.end()
})
43 changes: 22 additions & 21 deletions test/lib/standard-tests.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
'use strict'
module.exports = function (verify, domain, shortname) {
verify('https://' + domain + '/111/222', 'https')
verify('https://' + domain + '/111/222.git', 'https.git')
verify('https://' + domain + '/111/222#branch', 'https#branch', 'branch')
verify('https://' + domain + '/111/222.git#branch', 'https.git#branch', 'branch')
module.exports = function (verify, domain, shortname, proj) {
proj = proj || '222'
verify('https://' + domain + '/111/' + proj, 'https')
verify('https://' + domain + '/111/' + proj + '.git', 'https.git')
verify('https://' + domain + '/111/' + proj + '#branch', 'https#branch', 'branch')
verify('https://' + domain + '/111/' + proj + '.git#branch', 'https.git#branch', 'branch')

verify('git+https://' + domain + '/111/222', 'git+https')
verify('git+https://' + domain + '/111/222.git', 'git+https.git')
verify('git+https://' + domain + '/111/222#branch', 'git+https#branch', 'branch')
verify('git+https://' + domain + '/111/222.git#branch', 'git+https.git#branch', 'branch')
verify('git+https://' + domain + '/111/' + proj, 'git+https')
verify('git+https://' + domain + '/111/' + proj + '.git', 'git+https.git')
verify('git+https://' + domain + '/111/' + proj + '#branch', 'git+https#branch', 'branch')
verify('git+https://' + domain + '/111/' + proj + '.git#branch', 'git+https.git#branch', 'branch')

verify('git@' + domain + ':111/222', 'ssh')
verify('git@' + domain + ':111/222.git', 'ssh.git')
verify('git@' + domain + ':111/222#branch', 'ssh', 'branch')
verify('git@' + domain + ':111/222.git#branch', 'ssh.git', 'branch')
verify('git@' + domain + ':111/' + proj, 'ssh')
verify('git@' + domain + ':111/' + proj + '.git', 'ssh.git')
verify('git@' + domain + ':111/' + proj + '#branch', 'ssh', 'branch')
verify('git@' + domain + ':111/' + proj + '.git#branch', 'ssh.git', 'branch')

verify('git+ssh://git@' + domain + '/111/222', 'ssh url')
verify('git+ssh://git@' + domain + '/111/222.git', 'ssh url.git')
verify('git+ssh://git@' + domain + '/111/222#branch', 'ssh url#branch', 'branch')
verify('git+ssh://git@' + domain + '/111/222.git#branch', 'ssh url.git#branch', 'branch')
verify('git+ssh://git@' + domain + '/111/' + proj, 'ssh url')
verify('git+ssh://git@' + domain + '/111/' + proj + '.git', 'ssh url.git')
verify('git+ssh://git@' + domain + '/111/' + proj + '#branch', 'ssh url#branch', 'branch')
verify('git+ssh://git@' + domain + '/111/' + proj + '.git#branch', 'ssh url.git#branch', 'branch')

verify(shortname + ':111/222', 'shortcut')
verify(shortname + ':111/222.git', 'shortcut.git')
verify(shortname + ':111/222#branch', 'shortcut#branch', 'branch')
verify(shortname + ':111/222.git#branch', 'shortcut.git#branch', 'branch')
verify(shortname + ':111/' + proj, 'shortcut')
verify(shortname + ':111/' + proj + '.git', 'shortcut.git')
verify(shortname + ':111/' + proj + '#branch', 'shortcut#branch', 'branch')
verify(shortname + ':111/' + proj + '.git#branch', 'shortcut.git#branch', 'branch')
}

0 comments on commit d5cf830

Please sign in to comment.