search

jsakamoto / ipaddressrange

.NET Class Library for range of IP address, both IPv4 and IPv6.
Mozilla Public License 2.0
368 stars 71 forks source link

Range.Contains regression 4.0.0 to 4.1.0 #63

Closed Ledragon closed 4 years ago

Ledragon commented 4 years ago

Hi, First thanks for the good work. After updating to 4.1.0, we noticed a regression in our application. I managed to narrow it down to behvaior change, either in pasing or contains method. Following test (that can be added in IPAddressRangeTest.cs) works in 4.0.0 but not in 4.1.0:

[TestMethod]
public void Is_Local_In_Range()
{
    var range = IPAddressRange.Parse("::/0");
    range.Contains(IPAddress.Parse("::1")).IsTrue();
}

I did not analyze further than this though. We downgraded to 4.0.0 as of now, so no emergency on that side. Thanks for the help!

jsakamoto commented 4 years ago

@Ledragon Thank you for your report!

However, I think, this behavior is the bug of v.4.0 (!), v.4.1's behavior is correct, doesn't it?

Because the range text "::/0" means IPv6 range that is from "::0" to "::0" (this means the count of IPv6 addresses in this range is zero), so any IPv6 addresses are never contained in this range, I think.

Therefore, IPAddressRange.Parse("::/0").Contains(IPAddress.Parse("::1")) should return false.

How do you think about this my guessing?

Addition: I tested Parse("::/0") method both v.4.0 and v.4.1, both of them behave as I expected, that returned the range object that's BeginAddress was "::" and EndAddress was "::".

jsakamoto commented 4 years ago

Oh, sorry, it's a doubt! 😅

any IPv6 addresses are never contained in this range

It's incorrect.

The range that from "::" to "::" contains just one IPv6 address "::"!

But, anyway, The IPv6 address "::1" is not contained in the range that from "::" to "::".

Ledragon commented 4 years ago

I am honestly not knowledgeable about network ranges and their management to known if the new behaviour is correct or if the old one was, I just noticed the behaviour change :D

jsakamoto commented 4 years ago

No, No...

"::/0" should represent from all 0x00 to 0xff...!

I should cool down my brain 😅

Please give me more time to thinking!

Anyway, thank you for your contribute!

I'll fix this issue 😄

Ledragon commented 4 years ago

Ah ok, I have the following as an argument in favour of the 4.0.0 behaviour: https://www.mediawiki.org/wiki/Help:Range_blocks/IPv6#Range_table

"::/0" represents all addresses for what I see. Again, no rush, we downgraded to 4.0.0 for now

jsakamoto commented 4 years ago

Finally, I fixed it!

I published v.4.1.1 just now. And I deprecated the previous version.

Again, thank you so much for your contribution! 👍 So many people would have been saved by your report!

Ledragon commented 4 years ago

Thanks for acting so quickly, glad I could help!