Doist/ffs

Add support for IPv6 in ip-related rules

kemasdimas opened this issue · 5 comments

Regarding #62, currently, it only supports IPv4 and I'm proposing an enhancement to support IPv6

I'm thinking of using the same functions ip and cidr that also accept IPv6 format

Continuing discussion in #62 (comment)

Something to note about eval is that it's the last operation of all. There are intermediate representations of data (string, boolean, number, array, etc.) and that's OK. In fact, many functions expect to work with certain data structures that are not floats.

It's also worth noting that the existing functions -- eq, lt (+ all others), contains -- don't make many assumptions. As long as something understands equality and implements Comparable, it should work. For example, if ipv6 returned an internal class backed by 2 longs that implements Comparable and equals/hashCode, and cidrv6 returned a range between 2 of those instances, the rest should mostly work out of the box.

We could also look into supporting both formats under ip and cidr. I don't think it would require many changes, but to be fair, I haven't prototyped it.

ip implementation will return a pair of Long, which will extend Comparable

It could make sense to have a supporting class. 👍

Implementing both IPv4 and IPv6 can have this edge case of mismatched IP type

contains(ip("192.168.1.2"), cidr("::ff:22:33/122")) <- IPv6 rule, but user device has IPv4

or vice versa

contains(ip("::00ff:0021:FFFF"), cidr("192.168.1.1/16")) <- IPv4 rule, but user has IPv6

I suggest we default this condition to always return false, so only the same IP type within the chosen range can access the feature

Please let me know if you have other thoughts / considerations @goncalossilva

Sounds good, @kemasdimas, if it doesn't complicate the flow. I think we might have a few other edge cases like this, although the effort to prevent them is probably not worth it.

Added in #69. 🎉