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
=== modified file 'src/ip/Address.cc'
--- src/ip/Address.cc 2011-07-04 04:28:59 +0000
+++ src/ip/Address.cc 2011-07-22 11:12:06 +0000
@@ -164,44 +164,6 @@
return true;
}
-bool
-Ip::Address::IsSockAddr() const
-{
- return (m_SocketAddr.sin6_port != 0);
-}
-
-bool
-Ip::Address::IsIPv4() const
-{
- return IsAnyAddr() || IsNoAddr() || IN6_IS_ADDR_V4MAPPED( &m_SocketAddr.sin6_addr );
-}
-
-bool
-Ip::Address::IsIPv6() const
-{
- return IsAnyAddr() || IsNoAddr() || !IN6_IS_ADDR_V4MAPPED( &m_SocketAddr.sin6_addr );
-}
-
-bool
-Ip::Address::IsAnyAddr() const
-{
- return IN6_IS_ADDR_UNSPECIFIED( &m_SocketAddr.sin6_addr );
-}
-
-/// NOTE: Does NOT clear the Port stored. Ony the Address and Type.
-void
-Ip::Address::SetAnyAddr()
-{
- memset(&m_SocketAddr.sin6_addr, 0, sizeof(struct in6_addr) );
-}
-
-/// NOTE: completely empties the Ip::Address structure. Address, Port, Type, everything.
-void
-Ip::Address::SetEmpty()
-{
- memset(&m_SocketAddr, 0, sizeof(m_SocketAddr) );
-}
-
#if _SQUID_AIX_
// Bug 2885 comment 78 explains.
// In short AIX has a different netinet/in.h union definition
@@ -225,6 +187,45 @@
#endif
bool
+Ip::Address::IsSockAddr() const
+{
+ return (m_SocketAddr.sin6_port != 0);
+}
+
+bool
+Ip::Address::IsIPv4() const
+{
+ return IN6_IS_ADDR_V4MAPPED( &m_SocketAddr.sin6_addr );
+}
+
+bool
+Ip::Address::IsIPv6() const
+{
+ return !IN6_IS_ADDR_V4MAPPED( &m_SocketAddr.sin6_addr );
+}
+
+bool
+Ip::Address::IsAnyAddr() const
+{
+ return IN6_IS_ADDR_UNSPECIFIED( &m_SocketAddr.sin6_addr )
+ || IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v4_anyaddr );
+}
+
+/// NOTE: Does NOT clear the Port stored. Ony the Address and Type.
+void
+Ip::Address::SetAnyAddr()
+{
+ memset(&m_SocketAddr.sin6_addr, 0, sizeof(struct in6_addr) );
+}
+
+/// NOTE: completely empties the Ip::Address structure. Address, Port, Type, everything.
+void
+Ip::Address::SetEmpty()
+{
+ memset(&m_SocketAddr, 0, sizeof(m_SocketAddr) );
+}
+
+bool
Ip::Address::SetIPv4()
{
if ( IsLocalhost() ) {
@@ -237,6 +238,11 @@
return true;
}
+ if ( IsNoAddr() ) {
+ m_SocketAddr.sin6_addr = v4_noaddr;
+ return true;
+ }
+
if ( IsIPv4())
return true;
@@ -247,7 +253,8 @@
bool
Ip::Address::IsLocalhost() const
{
- return IN6_IS_ADDR_LOOPBACK( &m_SocketAddr.sin6_addr ) || IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v4_localhost );
+ return IN6_IS_ADDR_LOOPBACK( &m_SocketAddr.sin6_addr )
+ || IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v4_localhost );
}
void
@@ -279,7 +286,8 @@
Ip::Address::IsNoAddr() const
{
// IFF the address == 0xff..ff (all ones)
- return IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v6_noaddr );
+ return IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v6_noaddr )
+ || IN6_ARE_ADDR_EQUAL( &m_SocketAddr.sin6_addr, &v4_noaddr );
}
void
