Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support prefix #399

Closed
wants to merge 3 commits into from
Closed

Conversation

RWOverdijk
Copy link
Contributor

https://en.wikipedia.org/wiki/IPv6_address#Transition_from_IPv4

Some issues arose in Waterline, which uses anchor. which uses validator.js. ::ffff:127.0.0.1 is a prefixed IPv4. I found this on StackOverflow.

@addaleax
Copy link
Contributor

I see two small issues with this:

  • There are prefixes which are equivalent to ::ffff, e.g. ::0:ffff:127.0.0.1 is a non-canonical, yet perfectly valid way of writing the same address as ::ffff:127.0.0.1, so simply checking for "::ffff:" + IPv4 is not sufficient.
  • Since you modified ipv4Maybe, the IPv4-mapped IPv6 addresses would be considered valid as IPv4 addresses but not as IPv6 addresses, e.g isIP("::ffff:127.0.0.1", 4) === true. While I can understand this, it probably does not make sense in 99 % of cases: When used programmatically, isIP(…, 4) should return true for input which can be used in places where an IPv4 address is expected, and the same goes for IPv6.

@addaleax
Copy link
Contributor

Regarding my first point: The RFC 3493 says that ::ffff, or rather its capitalisation, is the valid prefix for this notation. On my system (glibc 2.21), the resolver seems to be a lot more liberal in what is accepted, so the last block can always be in IPv4 notation (e.g. even 2620:0:862:ed1a::0.0.0.1 can be used)… I’m not sure whether this library should accept other prefixes that what is RFC-specified, but still, I think that at least capitalisation should not be relevant.

@RWOverdijk
Copy link
Contributor Author

Agreed. So what changes would you like to see? Just case insensitivity for the prefix? Or a split on ::\d+.? Because ::ffff (and any of the examples you gave me) do validate this way. I also get the ignoring bit when it comes to specifically specifying an IP address... But should it? It' a valid IP.

@addaleax
Copy link
Contributor

I’d say case insensitivity + rather testing as IPv6 addresses than as IPv4 addresses. For example, one could insert something like

if (/^::ffff:/i.test(str) && validator.isIP(str.substr(7), 4))
    return true;

after validator.js:274 (i.e. version === '6') and leave the IPv4 code as it is.
But that’s just an idea and my personal opinion – I wrote the IPv6 validation code, but I don’t really know how strict this library is intended to be.
Maybe it would help to see first which other OS (Mac, BSD, Windows) also accept address formats like ::0:ffff:127.0.0.1; If most do, I think that this library also should consider them valid, since that would mean that these addresses can be used whenever an IPv6 address is expected.

@RWOverdijk
Copy link
Contributor Author

Splitting on ::\d+.? seems to also work. Both sides need to match before it returns true. One for ipv6 and one for ipv4...

@addaleax
Copy link
Contributor

Hm, yeah. You can do that, but keep in mind that the left side should have 6 blocks rather than the usual 8…

@RWOverdijk
Copy link
Contributor Author

Small test:

node -e "var v = require('validator'); ipParts = '::ffff:127.0.0.1'.split(':'), ipv4 = ipParts.pop(), ipv6 = ipParts.join(':'); console.log(v.isIP(ipv4) && v.isIP(ipv6));"
// true

More readable

var v       = require('validator'),
    ipParts = '::ffff:127.0.0.1'.split(':'), 
    ipv4    = ipParts.pop(), 
    ipv6    = ipParts.join(':'); 

console.log(v.isIP(ipv4) && v.isIP(ipv6));

// true

this works with the current system, without any of my changes.

@addaleax
Copy link
Contributor

Still, this approach treats 0:0:0:0:0:0:0:ffff:127.0.0.1 (with too many blocks) as valid and, worse 0:0:0:0:0:ffff:127.0.0.1 (with the correct number of blocks) as invalid

@addaleax
Copy link
Contributor

I pushed a suggestion for solving this to addaleax/ipv4-mapped-ipv6-addr, you can take merge it into this PR or ignore it if you’re unhappy with it

@RWOverdijk
Copy link
Contributor Author

Are you sure 0:0:0:0:0:ffff:127.0.0.1 is valid? Because the number of blocks don't match with the transition structure. It would for a full-out ipv6, that's true, but I don't believe it's valid.

@RWOverdijk
Copy link
Contributor Author

> var reg = new RegExp('(([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(\.(25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}))|:))');
> reg.test('0:0:0:0:0:ffff:127.0.0.1');
false
> reg.test('::ffff:127.0.0.1');
true

@addaleax
Copy link
Contributor

Well, given that I can use addresses of the format 0:0:0:0:0:ffff:a.b.c.d and that the number of bits (resp. number of blocks) adds up just fine (6×16 bits for the /96 prefix + 4×8 bits for the 32-bit IPv4 address = 128 bits for an entire IPv6 address), I’d say it’s as valid as it gets in this format. As I mentioned, the RFC talks only about a literal ::ffff:, so one surely can argue.
And I’m not entirely sure where that regexp comes from and I don’t really have the time to take it apart (after all, that’s more than a kilobyte of regexp). I reckon that it accepts the address with one block of zeroes added, but still, just do the math and you’ll see that that’s 16 bits more than an IPv6 address should have.
If you want an usage example,

$ curl http://[0:0:0:0:0:0:ffff:91.198.22.70]/
curl: (6) Could not resolve host: 0:0:0:0:0:0:ffff:91.198.22.70
$ curl http://[0:0:0:0:0:ffff:91.198.22.70]/
<html><head><title>Current IP Check</title></head><body>Current IP Address: […] </body></html>

@RWOverdijk
Copy link
Contributor Author

Valid IPv6 Format. Best representation is ::ffff:127.0.0.1

Nobody is going to write it like that. But you're right, they can. Either way, none of my solutions seem to fit.

@RWOverdijk RWOverdijk closed this Jun 11, 2015
@addaleax
Copy link
Contributor

Don’t get me wrong – I’m not saying that 0:0:0:0:0:ffff:127.0.0.1 should definitely be accepted as valid. It’s really something to argue about, and for example, one could get data from more OS to see how it should be treated.
The problem I saw was that splitting at the last : and checking for the prefix ::ffff: in a case-sensitive manner create false positives/false negatives.
If you don’t mind, I’ll create a new PR for this which includes your commits so far.

@RWOverdijk
Copy link
Contributor Author

I don't mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants