Skip to content

Commit

Permalink
fix(parse): Crash on strings that parse to having no host
Browse files Browse the repository at this point in the history
Fixes: #35
  • Loading branch information
Erveon authored and iarna committed Jul 7, 2018
1 parent b7b18a9 commit c931482
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function fromUrl (giturl, opts) {
project = decodeURIComponent(shortcutMatch[3])
defaultRepresentation = 'shortcut'
} else {
if (parsed.host !== gitHostInfo.domain && parsed.host.replace(/^www[.]/, '') !== gitHostInfo.domain) return
if (parsed.host && parsed.host !== gitHostInfo.domain && parsed.host.replace(/^www[.]/, '') !== gitHostInfo.domain) return
if (!gitHostInfo.protocols_re.test(parsed.protocol)) return
if (!parsed.path) return
var pathmatch = gitHostInfo.pathmatch
Expand Down
7 changes: 7 additions & 0 deletions test/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,12 @@ test('basic', function (t) {
t.is(HostedGit.fromUrl('gist:123').https(), 'git+https://gist.github.com/123.git', 'non-user shortcut')

t.is(HostedGit.fromUrl('git+https://github.com:foo/repo.git#master').https(), 'git+https://github.com/foo/repo.git#master', 'scp style urls are upgraded')

t.is(HostedGit.fromUrl(''), undefined, 'empty strings are not hosted')
t.is(HostedGit.fromUrl(null), undefined, 'null is not hosted')
t.is(HostedGit.fromUrl(), undefined, 'no value is not hosted')
t.is(HostedGit.fromUrl('git+file:///foo/bar'), undefined, 'url that has no host')
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.end()
})

2 comments on commit c931482

@jaydenseric
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iarna I don't think this should have been published as a semver patch, it appears to be a breaking change! It seems to have broken my prepublish scripts:

screen shot 2018-07-07 at 11 31 04 am

@jaydenseric
Copy link

@jaydenseric jaydenseric commented on c931482 Jul 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reinstall just now fixed it. I think I was being affected by #35 and mistook this fix with the cause.

Please sign in to comment.