feat: regex for ipv6

[vasco-santos]

This PR adds regex for ipv6 addresses per #3 . I needed to do a special validation for addresses started by 0 as they were not being considered ipv4. Open to improve this solution

This was based on https://en.m.wikipedia.org/wiki/Reserved_IP_addresses#IPv6

cc @sickcodes @frenchbread

[sickcodes]

I'll give it a test soon! Looks good.

[sickcodes]

What was wrong with netmask?

[vasco-santos]

netmask does not support ipv6 addresses

EDIT: rs/node-netmask#8 I would like support on that, but I am not familiar with coffeescript at all

[sickcodes]

Looks good to me

[sickcodes]

npm run test
168 passed

Also tested the inverse of the test and 168 failed which is good.

Here's upstream of is-ip anyway: https://github.com/sindresorhus/ip-regex/blob/master/index.js

@frenchbread

[frenchbread]

Thanks for the PR! Good work. Works fine for me as well.

Could you please update the package version also, @vasco-santos & I'll merge it.

[vasco-santos]

Sure, added a new commit for the release.

[frenchbread]

@vasco-santos Cool, thanks!

[frenchbread]

Btw, @vasco-santos, why did you remove this return statement? Now when I test it by passing public IP it returns undefiled rather than true.

[vasco-santos]

Oh this is a bug! I was initially doing everything in the same function, as ipv6 being use d as a fallback for the ipv4 validation. But I moved later. I am going to PR this

[frenchbread]

Also, pardon me If I'm misunderstanding, but why add ip.startsWith('0') if netmaskCheck() already handles it? IMHO we could just have:

export default ip => netmaskCheck(ip) || ipv6Check(ip)

or

export default ip => {
  if (isIp.v4(ip)) return netmaskCheck(ip)
  else if (isIp.v6(ip)) return ipv6Check(ip)
  else return false
}

Update: isIp.v6(ip) check does not pass when running tests.

[vasco-santos]

Yeah, the first was my initial approach. But as I mentioned in the PR description the ipv4 with 0.0.0.0.0 is not a valid ipv4 per the isIp.v4(), which is correct... So this should not really be handled in ipv4.

The second approach suffer from the same issue I believe. 0.0.0.0.0 will not be nor ipv4, nor ipv6 and it will return false, when it should be true. We can figure out a better solution for such addresses

[frenchbread]

I've modified second code snippet:

export default ip => {
  if (isIp.v4(ip) || ip.startsWith('0')) return netmaskCheck(ip)
  else if (isIp.v6(ip)) return ipv6Check(ip)
  else return false
}

and getting this:

t.truthy(isPrivate(ip), true)
           |         |         
           false     "::ffff:0.0.255.255.255"
-----
t.truthy(isPrivate(ip), true)
           |         |         
           false     "::ffff:0.255.255.255.255"
-----
t.truthy(isPrivate(ip), true)
           |         |         
           false     "100::ffff::"

seems like isIp is not picking up on those...

[vasco-santos]

Yeah, perhaps creating an issue to rethink this logic is a better way

[frenchbread]

Sorry, @vasco-santos, for rising these points after code has been merged. I am just wondering for future improvements.