(Cc:ing [EMAIL PROTECTED] now, because of APR patch)
On Tue, Aug 12, 2003 at 10:12:45AM -0700, Justin Erenkrantz wrote:
> --On Tuesday, August 12, 2003 5:58 PM +0100 "Colm MacCarthaigh,,,"
> <[EMAIL PROTECTED]> wrote:
>
> >Even lo0 ? That's hard ;)
>
> % ifconfig -a6
> %
As an asside I maintain that this is a broken system configuration, it's
just as if you had IPv4 built in, but chose not to configure any IPv4
addresses. What would have Apache do in such a situation ? Surely exiting
is appropriate ...
But since you're less wrong than I am on this one, using AI_ADDRCONFIG
is best. This will weed out the problem.
But just to make clear I think "Listen 80" *really* needs to mean
"Listen on port 80", not "Listen on port 80 in IPv4 only" :)
> >if getaddrinfo for PF_UNSPEC returns '::' in this situation
> >then it's really a lib(c|socket|nsl) bug, it does this on
> >Linux aswell, but the KAME implementation (and I believe windows)
> >have it fixed.
>
> No, the OS is not returning '::'. We're not using PF_UNSPEC, but
> hard-coding PF_INET6. (AF_INET6, AP_INET6, whatever the #define is.)
This is the real bug, Apache shouldnt be second-guessing getaddrinfo,
APR's call_resolver is an excellent example of how to do it properly,
AI_ADDRCONFIG is the key. Trouble is apr_sockaddr_info_get isnt
calling it for us. This is an APR bug, see the attached patch.
> In alloc_listener, we're hardcoding '::' and PF_INET6 unconditionally if
> the OS supports IPv6 (as determined by find_default_family). (See the
> !addr branch.)
It's not as broken as it seems, it does:
254 apr_sockaddr_info_get(&sa, NULL, APR_INET6, 0, 0, p) == APR_SUCCESS
So it *should* be valid. Problem is it *always* returns APR_SUCCESS
for a NULL hostname :( Line 583 of apr/network_io/unix/sockaddr.c
is the real problem.
> I have a hunch that we may be able to create ephemeral ports (NULL address
> passed in) without a configured IPv6 interface on Solaris. Which strikes
> me as not being totally wrong on Solaris's part.
>
> I still maintain that my patch should be applied. Let PF_UNSPEC figure it
> out.
But your patch didnt do this, it just blindly returned 0.0.0.0. This
would break literally hundreds of peoples configs!
APR patch attached, it should fix your problem. I don't
have any systems I can configure easily into the state you
describe, so I can't test it properly just yet.
What should happen:
apr_sockaddr_info will call find_addresses, which will call
call_resolver which will have AI_ADDRCONFIG dutifuly set. So
if apply the patch , the problem should go away.
This all leaves listen.c bigger than it needs to be. So there's
changes in the patch for that too. The original find_default_family
didnt even compile for me anyway ;) The changes to listen.c create
one very slight logic problem in that:
"Listen 80
Listen 0.0.0.0 80"
will now result in a failure of apache to start because binding
to the socket twice will fail, wheras previously the dupe would
have been cought. I don't think this is a major problem.
Now, the extra cookie that we all deserve. This all shows up
a massive problem with apr_socket_create, it only creates
one socket. But apr_sockaddr_info_get returns a linked
list to many. It's allowable for AI_PASSIVE to return both
:: and 0.0.0.0, for example, or for round-robin DNS records
to return multiple addresses and this isnt handled properly.
My head hurts already.
There really needs to be an APR call that means: "walk this
linked list and creete/bind sockets for each node."
--
Colm MacC�rthaigh Public Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
Index: server/listen.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/listen.c,v
retrieving revision 1.86
diff -u -r1.86 listen.c
--- server/listen.c 31 Mar 2003 04:30:38 -0000 1.86
+++ server/listen.c 13 Aug 2003 00:45:02 -0000
@@ -235,38 +235,6 @@
}
-static void find_default_family(apr_pool_t *p)
-{
-#if APR_HAVE_IPV6
- /* We know the platform supports IPv6, but this particular
- * system may not have IPv6 enabled. See if we can get an
- * AF_INET6 socket and bind to an ephemeral port. (On most
- * systems, getting an AF_INET6 socket is a sufficient test.
- * On certain levels of OpenUNIX, getting the socket is
- * successful but bind always returns ENETUNREACH.)
- */
- if (default_family == APR_UNSPEC) {
- apr_status_t sock_rv;
- apr_socket_t *tmp_sock;
- apr_sockaddr_t *sa;
-
- if ((sock_rv = apr_socket_create(&tmp_sock, APR_INET6, SOCK_STREAM, p))
- == APR_SUCCESS &&
- apr_sockaddr_info_get(&sa, NULL, APR_INET6, 0, 0, p) == APR_SUCCESS &&
- apr_bind(tmp_sock, sa) == APR_SUCCESS) {
- default_family = APR_INET6;
- }
- else {
- default_family = APR_INET;
- }
- if (sock_rv == APR_SUCCESS) {
- apr_socket_close(tmp_sock);
- }
- }
-#endif
-}
-
-
static const char *alloc_listener(process_rec *process, char *addr, apr_port_t port)
{
ap_listen_rec **walk;
@@ -276,36 +244,20 @@
apr_sockaddr_t *sa;
if (!addr) { /* don't bind to specific interface */
- find_default_family(process->pool);
- switch(default_family) {
- case APR_INET:
- addr = "0.0.0.0";
- break;
-
-#if APR_HAVE_IPV6
- case APR_INET6:
- addr = "::";
- break;
-#endif
-
- default:
- ap_assert(1 != 1); /* should not occur */
- }
- }
-
- /* see if we've got an old listener for this address:port */
- for (walk = &old_listeners; *walk; walk = &(*walk)->next) {
- sa = (*walk)->bind_addr;
- /* Some listeners are not real so they will not have a bind_addr. */
- if (sa) {
- apr_sockaddr_port_get(&oldport, sa);
- if (!strcmp(sa->hostname, addr) && port == oldport) {
- /* re-use existing record */
- new = *walk;
- *walk = new->next;
- new->next = ap_listeners;
- ap_listeners = new;
- return NULL;
+ /* see if we've got an old listener for this address:port */
+ for (walk = &old_listeners; *walk; walk = &(*walk)->next) {
+ sa = (*walk)->bind_addr;
+ /* Some listeners are not real so they will not have a bind_addr. */
+ if (sa) {
+ apr_sockaddr_port_get(&oldport, sa);
+ if (!strcmp(sa->hostname, addr) && port == oldport) {
+ /* re-use existing record */
+ new = *walk;
+ *walk = new->next;
+ new->next = ap_listeners;
+ ap_listeners = new;
+ return NULL;
+ }
}
}
}
Index: srclib/apr/network_io/unix/sockaddr.c
===================================================================
RCS file: /home/cvspublic/apr/network_io/unix/sockaddr.c,v
retrieving revision 1.38
diff -u -r1.38 sockaddr.c
--- srclib/apr/network_io/unix/sockaddr.c 17 Jul 2003 14:27:39 -0000 1.38
+++ srclib/apr/network_io/unix/sockaddr.c 13 Aug 2003 00:45:03 -0000
@@ -381,6 +381,13 @@
hints.ai_flags = AI_ADDRCONFIG;
}
#endif
+#ifdef AI_PASSIVE
+ if(hostname == NULL) {
+ /* If hostname is NULL, assume we are trying to bind to all
+ * interfaces. */
+ hints.ai_flags |= AI_PASSIVE;
+ }
+#endif
error = getaddrinfo(hostname, NULL, &hints, &ai_list);
#ifdef AI_ADDRCONFIG
if (error == EAI_BADFLAGS && family == AF_UNSPEC) {
@@ -490,6 +497,11 @@
struct in_addr ipaddr;
char *addr_list[2];
+ if (hostname == NULL) {
+ /* if we are given a NULL hostname, assume '0.0.0.0' */
+ hostname = "0.0.0.0";
+ }
+
if (*hostname >= '0' && *hostname <= '9' &&
strspn(hostname, "0123456789.") == strlen(hostname)) {
@@ -580,21 +592,13 @@
#endif
}
- if (hostname) {
#if !APR_HAVE_IPV6
- if (family == APR_UNSPEC) {
- family = APR_INET;
- }
-#endif
- return find_addresses(sa, hostname, family, port, flags, p);
+ if (family == APR_UNSPEC) {
+ family = APR_INET;
}
+#endif
- *sa = apr_pcalloc(p, sizeof(apr_sockaddr_t));
- (*sa)->pool = p;
- apr_sockaddr_vars_set(*sa,
- family == APR_UNSPEC ? APR_INET : family,
- port);
- return APR_SUCCESS;
+ return find_addresses(sa, hostname, family, port, flags, p);
}
APR_DECLARE(apr_status_t) apr_getnameinfo(char **hostname,
