Re: Ip::Address::IsAnyAddr

On 22/07/11 22:53, Tsantilas Christos wrote:
> I did a small research and I am finding many problems with the current
> implementation and use of the Ip::Address::IsAnyAddr() method.
>
> To summarize the problem:
> - Currently the ip::Address::IsAnyAddr() return true only for ipv6
> anyaddr (0000:0000:0000:0000:0000:0000:0000:0000). It return false when
> we have an ipv4 anyaddr (0000:0000:0000:0000:0000:FFFF:0000:0000)
> This is not normal.
>
> - The ip::Address::IsIPv6 and ip::Address::IsIPv4 methods return true in
> the case the IsAnyAddr return true, this is mean that the
> ip::Address::IsIPv4 return false in the case of ipv4 anyaddr.
> This is a bug.
>
>
> Searching in the code I am finding other bugs too. For example:
> - inside Ip::Address::SetIPv4(). If we have an ip address which is
> (ipv6) anyaddr and we call SetIPv4 for this ip address to convert it to
> ipv4 ip address, the conversion will done, but the ip address will not
> considred as anyaddr any more, because the IsAnyAddr will return false,
> for ipv4 anyaddr.
>
> - inside cache_cf.cc file inside dump_generic_http_port function there
> is the code:
> if (s->s.IsAnyAddr() && !s->s.IsIPv6())
> storeAppendPrintf(e, " ipv4");
> The if condition in the above statement can never be true. But the s->s
> can be an ipv4 anyaddr.
>
> Also in the code I am finding other places where my sense is that the
> code will not work as expected in the case we are listening to an ipv4
> anyaddr ip address.
>
> I am attaching a patch here which:
> - Ip::Address::IsAnyAddr() return true for both ipv6 or ipv4 anyaddr.
> - IsIPv6 return true only in the case of ipv6 anyaddr. If the ip address
> is an ipv4 anyaddr return false.
> - IsIPv4 return true only in the case of ipv4 anyaddr. If the ip address
> is an ipv6 anyaddr return false.
>
> From what I can see looking the code this is what expected by these
> methods.
> I did not do extended tests, but looks that this is the best solution...
>

Hmm. Okay, but not quite far enough. As Alex pointed out IsNoAddr() also
need to be kept in sync. Also IsLocalhost().

So, attached is a slight update:
  * moving the IsIPv4/6 to base purely on the v4-mapped or not.
  * making both protocols ANYADDR match the same test
  * making both protocols NOADDR match the same test
  * polishing all Is*() tests to be independent of each others results

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.14
   Beta testers wanted for 3.2.0.9