Feature #1905: How about IPAddr#<=> to take care of mask_addr?

• Status: Rejected
• Assignee: knu (Akinori MUSHA)
• Target version: 2.0.0

[no6v (Nobuhiro IMAI)]

=begin
いまいです。

#1877 はありがとうございました。

IPAddr のインスタンスがハッシュのキーに出来るようになったので、簡単な
Access Control List を書いてみたのですが、

RUBY_DESCRIPTION
=> "ruby 1.9.2dev (2009-08-07 trunk 24440) [i686-linux]"
acl = {
?> IPAddr.new("192.168.2.0/29") => :admin,
?> IPAddr.new("192.168.2.0/24") => :user,
?> }
=> {#<IPAddr: IPv4:192.168.2.0/255.255.255.248>=>:admin, #<IPAddr: IPv4:192.168.2.0/255.255.255.0>=>:user}
acl.select{|r, | r === "192.168.2.56"}.max.last
=> :user
acl.select{|r, | r === "192.168.2.1"}.max.last
=> :user

と、ちょっと残念な感じです。<=> で、@addr が同じ場合は @mask_addr の大
きい方が IPAddr として大きいとすると、ネットマスクの長い方が採用される
ようなイメージでうれしい気がするのですが、どうでしょうか？

また、これで(というか元から？) IPAddr#== は不要な気がするのですが、何
か見落としてるでしょうか？

IPAddr.new("192.168.2.0/24") == IPAddr.new("192.168.2.0/32")

が true から false に変わるという仕様変更なのですが、どうでしょうか？

Nobuhiro IMAI [e-mail]
Key fingerprint = F39E D552 545D 7C64 D690 F644 5A15 746C BD8E 7106

Index: lib/ipaddr.rb

--- lib/ipaddr.rb (revision 24440)
+++ lib/ipaddr.rb (working copy)
@@ -133,16 +133,10 @@ class IPAddr
# Returns a new ipaddr built by bitwise negation.
def ~
return self.clone.set(addr_mask(~@addr))
end

• Returns true if two ipaddrs are equal.

• def ==(other)

• other = coerce_other(other)

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

• end

• Returns a new ipaddr built by masking IP address with the given

  prefixlen/netmask. (e.g. 8, 64, "255.255.255.0", etc.)

  def mask(prefixlen)
  return self.clone.mask!(prefixlen)
  end
  @@ -325,11 +319,11 @@ class IPAddr
  def <=>(other)
  other = coerce_other(other)

  return nil if other.family != @family

• return @addr <=> other.to_i

• return [@addr, @mask_addr] <=> [other.to_i, other.mask_addr]
  end
  include Comparable

# Checks equality used by Hash.
def eql?(other)

@@ -372,10 +366,12 @@ class IPAddr
af, _to_string(@addr), _to_string(@mask_addr))
end

protected

• attr_reader :mask_addr
• def set(addr, *family)
  case family[0] ? family[0] : @family
  when Socket::AF_INET
  if addr < 0 || addr > IN4MASK
  raise ArgumentError, "invalid address"
  @@ -789,15 +785,24 @@ class TC_Operator < Test::Unit::TestCase
  a = ~@in6_addr_any
  assert_equal(IN6MASK128, a.to_s)
  assert_equal("::", @in6_addr_any.to_s)
  end

• def test_equal
• assert_equal(true, @A (A A) == IPAddr.new("3ffe:505:2::"))

• def test_compare
• assert_equal(true, @A (A A) == IPAddr.new("3ffe:505:2::/48"))
• assert_equal(false, @A (A A) == IPAddr.new("3ffe:505:2::"))
  assert_equal(false, @A (A A) == IPAddr.new("3ffe:505:3::"))
  assert_equal(true, @A (A A) != IPAddr.new("3ffe:505:3::"))

• assert_equal(false, @A (A A) != IPAddr.new("3ffe:505:2::"))

• assert_equal(false, @A (A A) != IPAddr.new("3ffe:505:2::/48"))
• x = IPAddr.new("3ffe:505:3::/48")
• y = IPAddr.new("3ffe:505:2::/64")
• assert_equal(true, @A (A A) < x)
• assert_equal(true, @A (A A) < y)
• assert_equal(false, x < y)
• min, max = [y, x, @A (A A)].minmax
• assert_equal(@A (A A), min)
• assert_equal(x, max)
  end

def test_mask
  a = @a.mask(32)
  assert_equal("3ffe:505::", a.to_s)

=end

[no6v (Nobuhiro IMAI)]

=begin
いまいです。

From: Nobuhiro IMAI <nov_at_yo.rim.or.jp>
Date: Fri, 7 Aug 2009 18:31:36 +0900

と、ちょっと残念な感じです。<=> で、@addr が同じ場合は @mask_addr の大
きい方が IPAddr として大きいとすると、ネットマスクの長い方が採用される
ようなイメージでうれしい気がするのですが、どうでしょうか？

また、これで(というか元から？) IPAddr#== は不要な気がするのですが、何
か見落としてるでしょうか？

IPAddr.new("192.168.2.0/24") == IPAddr.new("192.168.2.0/32")

が true から false に変わるという仕様変更なのですが、どうでしょうか？

少し時間が開いてしまいましたが、

• アドレスが同じ場合、ネットマスク長が長い方が IPAddr として大きいとみ
  なす(IPAddr#<=> で @mask_addr を考慮する)
• <=> と Comparable により == は定義されるので、IPAddr#== は廃止する

という [ruby-dev:39038] のパッチはどうでしょうか？

Nobuhiro IMAI [e-mail]
Key fingerprint = F39E D552 545D 7C64 D690 F644 5A15 746C BD8E 7106

=end

[no6v (Nobuhiro IMAI)]

=begin
いまいです。

From: Nobuhiro IMAI <nov_at_yo.rim.or.jp>
Date: Fri, 14 Aug 2009 21:19:42 +0900

少し時間が開いてしまいましたが、

• アドレスが同じ場合、ネットマスク長が長い方が IPAddr として大きいとみ
  なす(IPAddr#<=> で @mask_addr を考慮する)
• <=> と Comparable により == は定義されるので、IPAddr#== は廃止する

という [ruby-dev:39038] のパッチはどうでしょうか？

さらに時間が開いてしまいましたが、どちらも受け入れられないでしょうか？

IPAddr.new("192.168.2.0/24") == IPAddr.new("192.168.2.0/32")

が true から false に変わるという仕様変更なのですが、どうでしょうか？

というのがまずいでしょうか。

Nobuhiro IMAI [e-mail]
Key fingerprint = F39E D552 545D 7C64 D690 F644 5A15 746C BD8E 7106

=end

[znz (Kazuhiro NISHIYAMA)]

=begin
At Sun, 27 Sep 2009 12:00:48 +0900,
Akinori MUSHA wrote:

　これは受け入れられません。IPAddrはネットマスクも保持するため
ネットワークも表現できますが、第一義はIPアドレスなので、ネット
マスクの違いで等しくなくなるのはまずいです。

ネットマスクで思い出したのですが、

IPAddr.new("192.168.0.1/24") #=> #<IPAddr: IPv4:192.168.0.0/255.255.255.0>
IPAddr.new("192.168.0.2/24") #=> #<IPAddr: IPv4:192.168.0.0/255.255.255.0>

のようにネットマスクがあるとnewの引数の文字列よりも情報が減ってしまって
別途元のIPアドレスを持っておかないといけないのが不便です。

=end

[no6v (Nobuhiro IMAI)]

=begin
いまいです。

From: "Akinori MUSHA" <knu_at_iDaemons.org>
Date: Sun, 27 Sep 2009 12:00:48 +0900

• アドレスが同じ場合、ネットマスク長が長い方が IPAddr として大きいとみ
  なす(IPAddr#<=> で @mask_addr を考慮する)
• <=> と Comparable により == は定義されるので、IPAddr#== は廃止する

という [ruby-dev:39038] のパッチはどうでしょうか？

IPAddr.new("192.168.2.0/24") == IPAddr.new("192.168.2.0/32")

が true から false に変わるという仕様変更なのですが、どうでしょうか？

というのがまずいでしょうか。

　これは受け入れられません。IPAddrはネットマスクも保持するため
ネットワークも表現できますが、第一義はIPアドレスなので、ネット
マスクの違いで等しくなくなるのはまずいです。

そうですか、分かりました。<=> の変更は取り下げます。

それはそれとして、<=> と include Comparable があれば == の定義は不要に
思えるのですが、== を明示的に定義する理由があれば教えてください。現状
では == の定義を全部消しても、動作は同じだと思います。

　その上で、ソートの便宜を考えて <=> についてはネットマスクを
見て a1 == a2 && (a1 <=> a2) != 0 というケースを許すというのも
なくはないと思いますが、ユースケースに見られるニーズを満たす
よりよい方法はほかにあると思います。

　まず、改めてAPIを見るとネットマスクを取る手段がないので、
#mask_addr や #prefixlen のようなメソッドを用意して
sort_by/max_by {|i| [i, i.mask_addr] } できるようにするのが
ひとつ。(これは本件に関係なく)

　もう一つは class NetAddr < IPAddr のようなサブクラスを作り、
そちらで == や <=> をオーバーライドする方法。

　両方やってもいいかもしれませんね。どうでしょうか。

こちらは時間を見つけてパッチを書ければいいなと思います。

Nobuhiro IMAI [e-mail]
Key fingerprint = F39E D552 545D 7C64 D690 F644 5A15 746C BD8E 7106

=end

[znz (Kazuhiro NISHIYAMA)]

• Category set to lib
• Status changed from Open to Assigned
• Assignee set to knu (Akinori MUSHA)
• Target version set to 2.0.0
• Start date set to 08/07/2009

=begin

=end

[mame (Yusuke Endoh)]

• Status changed from Assigned to Rejected

長期間進展がみられないので閉じます。[ruby-core:42391] を参照。

武者さんのメールが抜け落ちていてわかりにくいですね。
[ruby-dev:39381] [ruby-dev:39391]

武者さんの提案する API ([ruby-dev:39391]) のパッチを作ってきたら
取りこんでやらんこともない、という待ち状態と思われます。
しかし長期間パッチが来ていないので、一旦とじさせてもらいます。
パッチができたら reopen なり新チケット作成なりしてください。

FYI: znz さんと同じ問題意識 (ネットマスクで IP が消されると面倒)
は、 https://bugs.ruby-lang.org/issues/1275 でも挙がってました。
もう閉じてしまってますが、やる気がある人は reopen してください。

--
Yusuke Endoh [e-mail]