Re: IpAddress comparison operators lie

From: Amos Jeffries <squid3_at_treenet.co.nz>
Date: Sat, 01 May 2010 18:07:18 +1200

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

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.1
Received on Sat May 01 2010 - 06:07:30 MDT

This archive was generated by hypermail 2.2.0 : Sat May 01 2010 - 12:00:19 MDT