--------------------- PatchSet 4580 Date: 2007/05/18 04:03:38 Author: amosjeffries Branch: squid3-ipv6 Tag: (none) Log: Fix some URI handling bugs. Changed IPAddress string handling from hostent to addrinfo to resolve some issues where gethostbyname would not properly resolve IPv6. Members: NOTES-IPv6:1.1.2.19->1.1.2.20 include/IPAddress.h:1.1.2.18->1.1.2.19 lib/IPAddress.cc:1.1.2.38->1.1.2.39 lib/tests/testIPAddress.cc:1.1.2.7->1.1.2.8 src/HttpRequest.h:1.10.6.11->1.10.6.12 src/url.cc:1.9.8.15->1.9.8.16 src/tests/testHttpRequest.cc:1.1.14.4->1.1.14.5 Index: squid3/NOTES-IPv6 =================================================================== RCS file: /cvsroot/squid-sf//squid3/Attic/NOTES-IPv6,v retrieving revision 1.1.2.19 retrieving revision 1.1.2.20 diff -u -r1.1.2.19 -r1.1.2.20 --- squid3/NOTES-IPv6 17 May 2007 10:51:01 -0000 1.1.2.19 +++ squid3/NOTES-IPv6 18 May 2007 04:03:38 -0000 1.1.2.20 @@ -1,4 +1,4 @@ -$Id: NOTES-IPv6,v 1.1.2.19 2007/05/17 10:51:01 amosjeffries Exp $ +$Id: NOTES-IPv6,v 1.1.2.20 2007/05/18 04:03:38 amosjeffries Exp $ KNOWN BUGS: @@ -32,12 +32,6 @@ idnsRead: FD 7 recvfrom: (14) Bad address - httpRequestFree: http://2001:200:0:8002:203:47ff:fea5:3085/ - - forward.cc(192) FwdState destructor done -2007/05/17 20:21:54.013| fd_close FD 16 http://2001:200:0:8002:203:47ff:fea5:3085/ -2007/05/17 20:21:54.013| commSetSelect: FD 16 type 1 - commio_call_callback: called for 13 2007/05/17 20:21:54.013| clientWriteComplete: FD 13, sz 2541, err 0, off 2541, len 0xb7b7bb9c 2007/05/17 20:21:54.013| clientReplyStatus: transfer is DONE Index: squid3/include/IPAddress.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/include/Attic/IPAddress.h,v retrieving revision 1.1.2.18 retrieving revision 1.1.2.19 diff -u -r1.1.2.18 -r1.1.2.19 --- squid3/include/IPAddress.h 10 May 2007 12:57:33 -0000 1.1.2.18 +++ squid3/include/IPAddress.h 18 May 2007 04:03:39 -0000 1.1.2.19 @@ -1,6 +1,6 @@ /* - * $Id: IPAddress.h,v 1.1.2.18 2007/05/10 12:57:33 amosjeffries Exp $ + * $Id: IPAddress.h,v 1.1.2.19 2007/05/18 04:03:39 amosjeffries Exp $ * * DEBUG: section 14 IP Storage and Handling * AUTHOR: Amos Jeffries @@ -108,7 +108,8 @@ IPAddress(const struct sockaddr_in6 &); #endif - IPAddress(const hostent *); + IPAddress(const struct hostent *); + IPAddress(const struct addrinfo *); IPAddress(const char*); /// Default destructor. ~IPAddress(); @@ -129,6 +130,7 @@ #endif bool operator =(const struct hostent *s); + bool operator =(const struct addrinfo *s); bool operator =(const char *s); /*@}*/ Index: squid3/lib/IPAddress.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/lib/Attic/IPAddress.cc,v retrieving revision 1.1.2.38 retrieving revision 1.1.2.39 diff -u -r1.1.2.38 -r1.1.2.39 --- squid3/lib/IPAddress.cc 17 May 2007 07:13:42 -0000 1.1.2.38 +++ squid3/lib/IPAddress.cc 18 May 2007 04:03:39 -0000 1.1.2.39 @@ -1,6 +1,6 @@ /* - * $Id: IPAddress.cc,v 1.1.2.38 2007/05/17 07:13:42 amosjeffries Exp $ + * $Id: IPAddress.cc,v 1.1.2.39 2007/05/18 04:03:39 amosjeffries Exp $ * * DEBUG: section 14 IP Storage and Handling * AUTHOR: Amos Jeffries @@ -412,18 +412,36 @@ bool IPAddress::operator =(const char* s) { + int err = 0; + struct addrinfo *res = NULL; + struct addrinfo want; + memset(&want, 0, sizeof(struct addrinfo)); + want.ai_flags = AI_NUMERICHOST; // prevent actual DNS lookups! +#if !USE_IPV6 + want.ai_family = AF_INET; +#endif - struct hostent *hp = NULL; - - if ((hp = gethostbyname(s)) == NULL) { - debugs(14,1, HERE << "Given Bad IP '" << s << "'"); + if ( (err = getaddrinfo(s, NULL, &want, &res)) != 0) { + debugs(14,1, HERE << "Given Bad IP '" << s << "': " << gai_strerror(err) ); return false; } - return operator=(hp); +/* + * NP: =(sockaddr_*) sets SockAddr flag. + * we MUST NOT set that flag from only an IPA assignment + */ + if( IsSockAddr() ) { // does not matter. + operator=(res); + } + else { + // prevent SockAddr being set by the other operator. + operator=(res); + // TODO FIX: really need to get a non-magic way of doing this mask. SockAddr == 0x01 -> 0xe in last bits. + m_Type = (IPAddressType)(m_Type & 0xFFFe ); + } + return true; } - IPAddress::IPAddress(struct sockaddr_in const &s) { memset(this,0,sizeof(IPAddress)); @@ -594,6 +612,11 @@ // char **h_addr_list; /* list of addresses */ //} + if(s == NULL) { + debugs(14,1, HERE << "WARNING: Assignment failed. No hostent details."); + return false; + } + switch(s->h_addrtype) { @@ -622,6 +645,63 @@ return true; } +IPAddress::IPAddress(const struct addrinfo *s) +{ + memset(this,0,sizeof(IPAddress)); + operator=(s); +} + +bool IPAddress::operator =(const struct addrinfo *s) +{ + + struct sockaddr_in* ipv4 = NULL; + + struct sockaddr_in6* ipv6 = NULL; + + //struct addrinfo { + // int ai_flags; /* input flags */ + // int ai_family; /* protocol family for socket */ + // int ai_socktype; /* socket type */ + // int ai_protocol; /* protocol for socket */ + // socklen_t ai_addrlen; /* length of socket-address */ + // struct sockaddr *ai_addr; /* socket-address for socket */ + // char *ai_canonname; /* canonical name for service location */ + // struct addrinfo *ai_next; /* pointer to next in list */ + //} + + if(s == NULL) { + debugs(14,1, HERE << "WARNING: Assignment failed. No AddrInfo details."); + return false; + } + + switch(s->ai_family) + { + + case AF_INET: + ipv4 = (sockaddr_in*)(s->ai_addr); + /* this */ + operator=(*ipv4); + break; + + case AF_INET6: + ipv6 = (sockaddr_in6*)(s->ai_addr); +#if USE_IPV6 + /* this */ + operator=(*ipv6); +#else + + debugs(14,1, HERE << "Discarded IPv6 Address. Protocol disabled."); + + // see if there is another address in the list that might be usable ?? + return operator=(s->ai_next); +#endif + + break; + } + + return true; +} + int IPAddress::matchIPAddr(const IPAddress &rhs) const { unsigned int slen = sizeof(m_SocketAddr.sin6_addr); @@ -758,10 +838,10 @@ } /* 7 being space for [,], and port */ - if( IsIPv4() ) - NtoA(p, blen-7, IPv4); - else + if( IsIPv6() ) NtoA(p, blen-7, IPv6); + else + NtoA(p, blen-7, IPv4); // find the end of the new string while(*p != '\0' && p < buf+blen) Index: squid3/lib/tests/testIPAddress.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/lib/tests/Attic/testIPAddress.cc,v retrieving revision 1.1.2.7 retrieving revision 1.1.2.8 diff -u -r1.1.2.7 -r1.1.2.8 --- squid3/lib/tests/testIPAddress.cc 17 May 2007 07:13:43 -0000 1.1.2.7 +++ squid3/lib/tests/testIPAddress.cc 18 May 2007 04:03:40 -0000 1.1.2.8 @@ -204,26 +204,50 @@ CPPUNIT_ASSERT( !anIPA.IsNoAddr() ); CPPUNIT_ASSERT( anIPA.IsIPv4() ); CPPUNIT_ASSERT( !anIPA.IsIPv6() ); - CPPUNIT_ASSERT( !anIPA.IsSockAddr() ); CPPUNIT_ASSERT_EQUAL( (u_short) 0 , anIPA.GetPort() ); + CPPUNIT_ASSERT( !anIPA.IsSockAddr() ); anIPA.GetInAddr(outval); CPPUNIT_ASSERT( memcmp( &expectval, &outval, sizeof(struct in_addr)) == 0 ); - /* Failing Test: IPv6 CANNOT set 'ffff:*' addresses usually picked by operators as masks. */ - IPAddress bnIPA = "ffff:ffff:fff0::"; - expectval.s_addr = htonl(0x00000000); +#if USE_IPV6 + struct in6_addr expectv6; + struct in6_addr outval6; + + expectv6.s6_addr32[0] = htonl(0x20000800); + expectv6.s6_addr32[1] = htonl(0x00000000); + expectv6.s6_addr32[2] = htonl(0x00000000); + expectv6.s6_addr32[3] = htonl(0x00000045); + + IPAddress bnIPA = "2000:800::45"; /* test stored values */ - CPPUNIT_ASSERT( bnIPA.IsAnyAddr() ); + CPPUNIT_ASSERT( !bnIPA.IsAnyAddr() ); CPPUNIT_ASSERT( !bnIPA.IsNoAddr() ); CPPUNIT_ASSERT( !bnIPA.IsIPv4() ); -#if USE_IPV6 - CPPUNIT_ASSERT( bnIPA.IsIPv6() ); -#else - CPPUNIT_ASSERT( !bnIPA.IsIPv6() ); -#endif + CPPUNIT_ASSERT( bnIPA.IsIPv6() ); CPPUNIT_ASSERT( !bnIPA.IsSockAddr() ); CPPUNIT_ASSERT_EQUAL( (u_short) 0 , bnIPA.GetPort() ); + bnIPA.GetInAddr(outval6); + CPPUNIT_ASSERT( memcmp( &expectv6, &outval6, sizeof(struct in6_addr)) == 0 ); + + /* test IPv6 as an old netmask format. This is invalid but sometimes use. */ + IPAddress cnIPA = "ffff:ffff:fff0::"; + + expectv6.s6_addr32[0] = htonl(0xFFFFFFFF); + expectv6.s6_addr32[1] = htonl(0xFFF00000); + expectv6.s6_addr32[2] = htonl(0x00000000); + expectv6.s6_addr32[3] = htonl(0x00000000); + + /* test stored values */ + CPPUNIT_ASSERT( !cnIPA.IsAnyAddr() ); + CPPUNIT_ASSERT( !cnIPA.IsNoAddr() ); + CPPUNIT_ASSERT( !cnIPA.IsIPv4() ); + CPPUNIT_ASSERT( cnIPA.IsIPv6() ); + CPPUNIT_ASSERT( !cnIPA.IsSockAddr() ); + CPPUNIT_ASSERT_EQUAL( (u_short) 0 , cnIPA.GetPort() ); + cnIPA.GetInAddr(outval6); + CPPUNIT_ASSERT( memcmp( &expectv6, &outval6, sizeof(struct in6_addr)) == 0 ); +#endif } void @@ -399,7 +423,8 @@ anIPA = ip6val; - CPPUNIT_ASSERT( memcmp("[c0a8:640c:ffff:ffff:ffff:ffff:ffff:ffff]:80", anIPA.ToURL(buf,MAX_IPSTRLEN), 46) == 0 ); + anIPA.ToURL(buf,MAX_IPSTRLEN); + CPPUNIT_ASSERT( memcmp("[c0a8:640c:ffff:ffff:ffff:ffff:ffff:ffff]:80", buf, 45) == 0 ); #endif @@ -502,23 +527,29 @@ maskIPA.SetNoAddr(); /* IPv6 masks MUST be CIDR representations. */ + /* however as with IPv4 they can technically be represented as a bitmask */ maskIPA = "ffff:ffff:fff0::"; CPPUNIT_ASSERT( !maskIPA.IsAnyAddr() ); - CPPUNIT_ASSERT( maskIPA.IsNoAddr() ); + CPPUNIT_ASSERT( !maskIPA.IsNoAddr() ); anIPA.ApplyMask(maskIPA); - CPPUNIT_ASSERT( anIPA.IsNoAddr() ); - CPPUNIT_ASSERT_EQUAL( 128 , anIPA.GetCIDR() ); + CPPUNIT_ASSERT( !anIPA.IsNoAddr() ); + CPPUNIT_ASSERT_EQUAL( 44 , anIPA.GetCIDR() ); anIPA.SetNoAddr(); maskIPA.SetNoAddr(); - /* IPv4 masks MUST not be represented in IPv6 as IPA format. */ + /* IPv4 masks represented in IPv6 as IPv4 bitmasks. */ maskIPA = "::ffff:ffff:f000"; CPPUNIT_ASSERT( !maskIPA.IsAnyAddr() ); - CPPUNIT_ASSERT( maskIPA.IsNoAddr() ); + CPPUNIT_ASSERT( !maskIPA.IsNoAddr() ); + CPPUNIT_ASSERT( maskIPA.IsIPv4() ); + CPPUNIT_ASSERT( !maskIPA.IsIPv6() ); anIPA.ApplyMask(maskIPA); - CPPUNIT_ASSERT( anIPA.IsNoAddr() ); - CPPUNIT_ASSERT_EQUAL( 128 , anIPA.GetCIDR() ); + CPPUNIT_ASSERT( !maskIPA.IsAnyAddr() ); + CPPUNIT_ASSERT( !maskIPA.IsNoAddr() ); + CPPUNIT_ASSERT( maskIPA.IsIPv4() ); + CPPUNIT_ASSERT( !maskIPA.IsIPv6() ); + CPPUNIT_ASSERT_EQUAL( 20 , anIPA.GetCIDR() ); #endif } Index: squid3/src/HttpRequest.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/HttpRequest.h,v retrieving revision 1.10.6.11 retrieving revision 1.10.6.12 diff -u -r1.10.6.11 -r1.10.6.12 --- squid3/src/HttpRequest.h 17 May 2007 10:51:05 -0000 1.10.6.11 +++ squid3/src/HttpRequest.h 18 May 2007 04:03:40 -0000 1.10.6.12 @@ -1,6 +1,6 @@ /* - * $Id: HttpRequest.h,v 1.10.6.11 2007/05/17 10:51:05 amosjeffries Exp $ + * $Id: HttpRequest.h,v 1.10.6.12 2007/05/18 04:03:40 amosjeffries Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -71,13 +71,16 @@ /* Now that we care what host contains it is better off being protected. */ /* HACK: These two methods are only inline to get around Makefile dependancies */ /* caused by HttpRequest being used in places it really shouldn't. */ + /* ideally they would be methods of URL instead. */ inline void SetHost(const char *src) { IPAddress test = src; - if(test.IsAnyAddr()) + if( test.IsAnyAddr() ) { xstrncpy(host, src, SQUIDHOSTNAMELEN); + } else { -// AYJ: DEBUG see what happends without this. test.ToHostname(host, SQUIDHOSTNAMELEN); + test.ToHostname(host, SQUIDHOSTNAMELEN); + debugs(14,1, "HttpRequest::SetHost() given IP: " << test); } }; inline const char* GetHost(void) const { return host; }; @@ -93,7 +96,7 @@ char login[MAX_LOGIN_SZ]; private: - char host[SQUIDHOSTNAMELEN + 1]; + char host[SQUIDHOSTNAMELEN]; public: AuthUserRequest *auth_user_request; Index: squid3/src/url.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/url.cc,v retrieving revision 1.9.8.15 retrieving revision 1.9.8.16 diff -u -r1.9.8.15 -r1.9.8.16 --- squid3/src/url.cc 10 May 2007 12:57:47 -0000 1.9.8.15 +++ squid3/src/url.cc 18 May 2007 04:03:40 -0000 1.9.8.16 @@ -1,6 +1,6 @@ /* - * $Id: url.cc,v 1.9.8.15 2007/05/10 12:57:47 amosjeffries Exp $ + * $Id: url.cc,v 1.9.8.16 2007/05/18 04:03:40 amosjeffries Exp $ * * DEBUG: section 23 URL Parsing * AUTHOR: Duane Wessels @@ -436,7 +436,7 @@ request->initHTTP(method, protocol, urlpath); } - request->SetHost(host); //xstrncpy(request->host(), host, SQUIDHOSTNAMELEN); + request->SetHost(host); xstrncpy(request->login, login, MAX_LOGIN_SZ); request->port = (u_short) port; return request; Index: squid3/src/tests/testHttpRequest.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/tests/testHttpRequest.cc,v retrieving revision 1.1.14.4 retrieving revision 1.1.14.5 diff -u -r1.1.14.4 -r1.1.14.5 --- squid3/src/tests/testHttpRequest.cc 10 May 2007 12:57:47 -0000 1.1.14.4 +++ squid3/src/tests/testHttpRequest.cc 18 May 2007 04:03:40 -0000 1.1.14.5 @@ -105,15 +105,15 @@ HttpRequest *aRequest = NULL; /* valid IPv6 address without port */ - url = xstrdup("http://[200:800::45]/foo"); + url = xstrdup("http://[2000:800::45]/foo"); aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT_EQUAL(METHOD_GET, aRequest->method); - CPPUNIT_ASSERT_EQUAL(String("200:800::45"), String(aRequest->GetHost())); + CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(PROTO_HTTP, aRequest->protocol); - CPPUNIT_ASSERT_EQUAL(String("http://[200:800::45]/foo"), String(url)); + CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]/foo"), String(url)); xfree(url); /* valid IPv6 address with port */ @@ -122,21 +122,21 @@ expected_port = 90; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT_EQUAL(METHOD_GET, aRequest->method); - CPPUNIT_ASSERT_EQUAL(String("2000:800::45"), String(aRequest->GetHost())); + CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(PROTO_HTTP, aRequest->protocol); CPPUNIT_ASSERT_EQUAL(String("http://[2000:800::45]:90/foo"), String(url)); xfree(url); /* IPv6 address as invalid (bug trigger) */ - url = xstrdup("http://2000:800:45/foo"); + url = xstrdup("http://2000:800::45/foo"); aRequest = HttpRequest::CreateFromUrlAndMethod(url, METHOD_GET); expected_port = 80; CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->port); CPPUNIT_ASSERT_EQUAL(METHOD_GET, aRequest->method); - CPPUNIT_ASSERT_EQUAL(String("2000:800:45"), String(aRequest->GetHost())); + CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->GetHost())); CPPUNIT_ASSERT_EQUAL(String("/foo"), aRequest->urlpath); CPPUNIT_ASSERT_EQUAL(PROTO_HTTP, aRequest->protocol); - CPPUNIT_ASSERT_EQUAL(String("http://2000:800:45/foo"), String(url)); + CPPUNIT_ASSERT_EQUAL(String("http://2000:800::45/foo"), String(url)); xfree(url); }