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
