Re: IpAddress comparison operators lie

Amos Jeffries wrote:
> Alex Rousskov wrote:
>> Hello,
>>
>> Current IpAddress comparison operators are broken from logic point
>> of view. For example, if a and b are IpAddress objects, then
>>
>> * both (a < b) and (a > b) are true if exactly one address is NoAddr;
>>
>
> Sorry.
>
> (a > b) looks correct.
>
> (a < b) should return false on that test instead of true. Or using the
> AnyAddr test like <= operator does.
>
>
>> * (ip1 == ip2) may be true even if the addresses have different ports
>>
>
>
> We have to ignore ports in the boolean operators due to all the code
> which does ACL matching on IPs. The squid.conf provided address has a
> zero port.
> If we include port in the comparison we will have to resort to
> duplicating every address being ACL tested and set its port to zero
> before doing each the test.
> It's better to check the addr, then check the addr.GetPort() as needed
> for the minority of things which check the port.
>
>> There may be more inconsistencies; I have not checked all possible
>> combinations.
>>
>> These bugs make it impossible to reliably sort or compare addresses
>> using C++ operators. However, I do not know whether some code already
>> relies on this broken behavior. Does it? In other words, should we just
>> fix the operators or is there more to it?
>
> None that I know of. +1 to fix IpAddress.
>
> Looks like a few more sets of cases to testBooleans() to catch the
> ANYADDR / NOADDR cases as well.
>
> Amos

PS. this looks serious enough to delay 3.1.2. So I'm adding the
unit-tests immediately for all boolean cases against ANYADR/NOADDR and
we'll see what pops up for the other operators.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.1