Project

General

Profile

Bug #11531

IPAddr#== implements wrong logic

Added by panasyuk (Aleksander Panasyuk) about 4 years ago. Updated almost 3 years ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:<unknown>]

Description

Description

IPAddr#== should implement the logic of comparison of two IPAddr instances. This generally means that it compares two IP addresses.
Lets look at the code of this method:

https://github.com/ruby/ruby/blob/c8b3f1b470e343e7408ab5883f046b1056d94ccc/lib/ipaddr.rb#L151

return @family == other.family && @addr == other.to_i

It returns the result of comparison of the families and the addresses, but it should also compare the netmask which describes the network where this address is located.
The code below shows the test case for this comparison:

ip1 = IPAddr.new '195.51.100.0/24'
ip2 = IPAddr.new '195.51.100.0/26'
ip1 == ip2 #=> true

This code shows that two identical IP addresses from different networks are equal. But the result should be false because these addresses are not identical.

Possible solution

Depending on Feature #11210 i would propose following implementation of this method:

def ==(other)
other = coerce_other(other)
return @family == other.family && @addr == other.to_i && @mask_addr == other.netmask
end

History

Updated by knu (Akinori MUSHA) almost 3 years ago

I think this is intentional. IPAddr represents an IP address, not an IP network, so it does not consider a difference in netmasks as significant.

Updated by bjmllr (Ben Miller) almost 3 years ago

it does not consider a difference in netmasks as significant

IPAddr.new isn't consistent with this principle:

IPAddr.new("1.2.3.4/24") == IPAddr.new("1.2.3.4/32") # => false

1.2.3.4/24 is valid notation for a host address, yet IPAddr.new will drop the low-order bits to make it a valid network address:

IPAddr.new("1.2.3.4/24") == IPAddr.new("1.2.3.0/24") # => true

I'm not sure why we would want to have a netmask at all if IPAddr is only for host addresses.

Updated by sahglie (Steven Hansen) almost 3 years ago

IPAddr represents an IP address, not an IP network, so it does not consider a difference in netmasks as significant.

I disagree that IPAddr represents an IP address (I think a more accurately represents a CIDR block). To add to Ben's point:

IPAddr.new('128.128.128.128').to_range.to_a # => [#<IPAddr: IPv4:128.128.128.128/255.255.255.255>]
IPAddr.new('128.128.128.128/30').to_range.to_a # => [
  #<IPAddr: IPv4:128.128.128.128/255.255.255.252>, 
  #<IPAddr: IPv4:128.128.128.129/255.255.255.252>, 
  #<IPAddr: IPv4:128.128.128.130/255.255.25│5.252>, 
  #<IPAddr: IPv4:128.128.128.131/255.255.255.252>
]

The fact that IPAddr accepts a CIDR block mean it represents 1 or more IPs. So an IPAddr with 1 IP should not be equal to an IPAddr with 4 IPs

Also available in: Atom PDF