Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh]

On Mon, Aug 25, 2003 at 06:32:08PM +0100, Joe Orton wrote:
> I committed a patch (different to the one I posted) which passes the
> right thing to the IN6_blah macro, so it should compile still on Darwin.
> Ta!

To further illustrate how silly it was of me to write a patch at
4am on Sunday morning it actually needs one more change.

Setting lr to NULL causes a seg-fault if the port was already 
in use, and didnt do what it was intended to anyway since the
for loop terminates on lr == NULL.


Index: server/listen.c
===
RCS file: /home/cvspublic/httpd-2.0/server/listen.c,v
retrieving revision 1.91
diff -u -r1.91 listen.c
--- server/listen.c 25 Aug 2003 16:00:49 -  1.91
+++ server/listen.c 25 Aug 2003 23:18:54 -
@@ -395,8 +395,13 @@
 ap_listeners = lr->next;
 }
 
-/* So that previous becomes NULL in the next iteration */
-lr = NULL;
+   /* Allthough we've removed ourselves from the list, 
+ * we need to make sure that the next iteration won't
+ * consider "previous" a working IPv6 '::' socket. 
+ * Changing the family is enough to make sure the
+ * conditions before make_sock() fail. 
+ */
+lr->bind_addr->family = AF_INET;
 
 continue;
 }

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: cvs commit: httpd-2.0/server listen.c

[Joe Orton]

On Mon, Aug 25, 2003 at 08:28:10AM -0700, Justin Erenkrantz wrote:
> --On Monday, August 25, 2003 9:27 AM +0100 Joe Orton <[EMAIL PROTECTED]> 
> wrote:
> 
> >This doesn't compile on some older Unixes since in_addr_t is a recent
> >invention; the casts aren't really necessary anyway.
> 
> They are required on Darwin since IN6_IS_ADDR_UNSPECIFIED is a macro that 
> requires the types to be correct.  The macros appear identically in 
> FreeBSD's netinet6/in6.h, so I think this is a KAME/BSD thing.

I committed a patch (different to the one I posted) which passes the
right thing to the IN6_blah macro, so it should compile still on Darwin.
Ta!

joe

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Monday, August 25, 2003 9:27 AM +0100 Joe Orton <[EMAIL PROTECTED]> wrote:

This doesn't compile on some older Unixes since in_addr_t is a recent
invention; the casts aren't really necessary anyway.
They are required on Darwin since IN6_IS_ADDR_UNSPECIFIED is a macro that 
requires the types to be correct.  The macros appear identically in FreeBSD's 
netinet6/in6.h, so I think this is a KAME/BSD thing.

#define IN6_IS_ADDR_UNSPECIFIED(a)  \
   ((*(const u_int32_t *)(const void *)(&(a)->s6_addr[0]) == 0) && \
(*(const u_int32_t *)(const void *)(&(a)->s6_addr[4]) == 0) && \
(*(const u_int32_t *)(const void *)(&(a)->s6_addr[8]) == 0) && \
(*(const u_int32_t *)(const void *)(&(a)->s6_addr[12]) == 0))
In order for the compiler to do anything, a has to be of a type that has 
s6_addr.  So, I'm not sure what we can do.

Which OSes exactly are broken?

Perhaps we need to add some wrapper code in APR for this, but the casts are 
essential on *BSD.

Also the below conditional looks dubious - previous->bind_addr should be
lr->bind_addr, and the lr->next family is never checked?  Attached a
patch which looks logically right and fixes the compile error, not
tested though - can you test it?
I'll take a look at this.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh]

On Mon, Aug 25, 2003 at 09:27:03AM +0100, Joe Orton wrote:
> Also the below conditional looks dubious - previous->bind_addr should be
> lr->bind_addr, and the lr->next family is never checked? 

*cough*, yes

> -*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
> +lr->bind_addr->sa.sin.sin_addr.s_addr == INADDR_ANY &&
>  lr->bind_addr->port == previous->bind_addr->port &&
>  previous->bind_addr->family == APR_INET6 &&
>  IN6_IS_ADDR_UNSPECIFIED(
> -(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&
> +previous->bind_addr->sa.sin6.sin6_addr.s6_addr) &&


That's what I had first, Justin added this to get it compiling on
Darwin, so unless I'm picking things up wrong I'd suggest leaving
it.

The rest looks better alright, attachted patch works and running for me.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
--- /home/colmmacc/httpd-2.0/server/listen.cMon Aug 25 12:14:12 2003
+++ httpd-2.0/server/listen.c   Mon Aug 25 12:22:09 2003
@@ -355,7 +355,7 @@
  */
 if (previous != NULL &&
 lr->bind_addr->family == APR_INET &&
-*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
+lr->bind_addr->sa.sin.sin_addr.s_addr == INADDR_ANY &&
 lr->bind_addr->port == previous->bind_addr->port &&
 previous->bind_addr->family == APR_INET6 &&
 IN6_IS_ADDR_UNSPECIFIED(
@@ -382,10 +382,10 @@
  */
 if (lr->next != NULL && lr->bind_addr->family == APR_INET6 &&
 IN6_IS_ADDR_UNSPECIFIED(
-(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&
+(struct in6_addr*)(lr->bind_addr->ipaddr_ptr)) &&
 lr->bind_addr->port == lr->next->bind_addr->port &&
-*((in_addr_t *)lr->next->bind_addr->ipaddr_ptr)
-== INADDR_ANY) {
+lr->next->bind_addr->family == APR_INET &&
+lr->next->bind_addr->sa.sin.sin_addr.s_addr == INADDR_ANY) {
 
 /* Remove the current listener from the list */
 if (previous) {

Re: cvs commit: httpd-2.0/server listen.c

[Joe Orton]

On Sun, Aug 24, 2003 at 10:43:36PM -, [EMAIL PROTECTED] wrote:
...
>   +#if APR_HAVE_IPV6
>   +int v6only_setting;
>   +/* If we are trying to bind to 0.0.0.0 and the previous listener
>   + * was :: on the same port and in turn that socket does not have
>   + * the IPV6_V6ONLY flag set; we must skip the current attempt to
>   + * listen (which would generate an error). IPv4 will be handled
>   + * on the established IPv6 socket.
>   + */
>   +if (previous != NULL &&
>   +lr->bind_addr->family == APR_INET &&
>   +*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
>   +lr->bind_addr->port == previous->bind_addr->port &&
>   +previous->bind_addr->family == APR_INET6 &&
>   +IN6_IS_ADDR_UNSPECIFIED(
>   +(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&

This doesn't compile on some older Unixes since in_addr_t is a recent
invention; the casts aren't really necessary anyway.

Also the below conditional looks dubious - previous->bind_addr should be
lr->bind_addr, and the lr->next family is never checked?  Attached a
patch which looks logically right and fixes the compile error, not
tested though - can you test it?

>else {
>   +#if APR_HAVE_IPV6
>   +/* If we tried to bind to ::, and the next listener is
>   + * on 0.0.0.0 with the same port, don't give a fatal
>   + * error. The user will still get a warning from make_sock
>   + * though.
>   + */
>   +if (lr->next != NULL && lr->bind_addr->family == APR_INET6 &&
>   +IN6_IS_ADDR_UNSPECIFIED(
>   +(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&
>   +lr->bind_addr->port == lr->next->bind_addr->port &&
>   +*((in_addr_t *)lr->next->bind_addr->ipaddr_ptr)
>   +== INADDR_ANY) {

Index: listen.c
===
RCS file: /home/cvs/httpd-2.0/server/listen.c,v
retrieving revision 1.90
diff -u -r1.90 listen.c
--- listen.c24 Aug 2003 22:43:36 -  1.90
+++ listen.c25 Aug 2003 08:14:39 -
@@ -355,11 +355,11 @@
  */
 if (previous != NULL &&
 lr->bind_addr->family == APR_INET &&
-*((in_addr_t *)lr->bind_addr->ipaddr_ptr) == INADDR_ANY &&
+lr->bind_addr->sa.sin.sin_addr.s_addr == INADDR_ANY &&
 lr->bind_addr->port == previous->bind_addr->port &&
 previous->bind_addr->family == APR_INET6 &&
 IN6_IS_ADDR_UNSPECIFIED(
-(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&
+previous->bind_addr->sa.sin6.sin6_addr.s6_addr) &&
 apr_socket_opt_get(previous->sd, APR_IPV6_V6ONLY,
&v6only_setting) == APR_SUCCESS &&
 v6only_setting == 0) {
@@ -382,10 +382,10 @@
  */
 if (lr->next != NULL && lr->bind_addr->family == APR_INET6 &&
 IN6_IS_ADDR_UNSPECIFIED(
-(struct in6_addr*)(previous->bind_addr->ipaddr_ptr)) &&
+lr->bind_addr->sa.sin6.sin6_addr.s6_addr) &&
 lr->bind_addr->port == lr->next->bind_addr->port &&
-*((in_addr_t *)lr->next->bind_addr->ipaddr_ptr)
-== INADDR_ANY) {
+lr->next->bind_addr->family == APR_INET && 
+lr->next->bind_addr->sa.sin.sin_addr.s_addr == INADDR_ANY) {
 
 /* Remove the current listener from the list */
 if (previous) {

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh]

On Thu, Aug 14, 2003 at 10:45:20PM -0700, Justin Erenkrantz wrote:
> --On Thursday, August 14, 2003 23:43:23 +0100 "Colm MacCarthaigh,,," 
> <[EMAIL PROTECTED]> wrote:
> 
> >good good, patch works so :) In which I'll now cc [EMAIL PROTECTED] and include
> >the neccessary autoconf voodoo to fix OSX/Darwin.
> 
> I'm not entirely sure the getnameinfo() patch is needed on current releases 
> of Darwin.  Host-based authorization *seems* to work without it.

According to my darwin sources, it definitely fixes things for them,
but is fixed with Jaguar I believe.

> Can someone please verify this to make sure I'm not doing something wrong 
> or misunderstanding what the original problem was 10 months ago or whatnot?
> 
> It's very possible this was fixed in a point release since last October. 
> If so, we can allow IPv6 on 10.2.6 and perhaps earlier versions if we can 
> get confirmation on those versions.  And, we *could* also enable it on 
> known-getnameinfo()-broken releases with this patch in place.  But, I'm not 
> really tempted to go that far.  -- justin

Well if it ever does worm it's way in, for the sake of ultra-paranioa
it really needs a ntohl() in the RHS of the assignment. IPv6 stores
everything in network-byte-order, IPv4 structures are stored locally
as host. And you never know someone might just roll out Darwin on Sparc ;p~

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Thursday, August 14, 2003 23:43:23 +0100 "Colm MacCarthaigh,,," 
<[EMAIL PROTECTED]> wrote:

good good, patch works so :) In which I'll now cc [EMAIL PROTECTED] and include
the neccessary autoconf voodoo to fix OSX/Darwin.
I'm not entirely sure the getnameinfo() patch is needed on current releases 
of Darwin.  Host-based authorization *seems* to work without it.

Can someone please verify this to make sure I'm not doing something wrong 
or misunderstanding what the original problem was 10 months ago or whatnot?

It's very possible this was fixed in a point release since last October. 
If so, we can allow IPv6 on 10.2.6 and perhaps earlier versions if we can 
get confirmation on those versions.  And, we *could* also enable it on 
known-getnameinfo()-broken releases with this patch in place.  But, I'm not 
really tempted to go that far.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Wednesday, August 13, 2003 16:57:34 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

in the non-getaddrinfo() implementation).  It seems that both
implementations should be sync'ed up to produce the same results.  It
appears that defaulting to "0.0.0.0" rather than NULL would be the safer
thing to do.
But, that's an IPv4 address, but we support IPv6.

Apparently, this series of commits broke Linux.  I bet this is where Joe's 
ADDRCONFIG patch comes into play.  Looking into it now.  -- justin

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Thursday, August 14, 2003 10:20 PM +0100 "Colm MacCarthaigh,,," 
<[EMAIL PROTECTED]> wrote:

On Thu, Aug 14, 2003 at 04:56:48PM -0400, John K. Sterling wrote:
Hi Colm -

I'm not sure what to be looking for, but i applied your patch, rebuilt
(with the broken_ipv6 set to 0, of course), turned on HostnameLookups,
and my access logs have remote hostname properly resolved.
That should mean it's working fine :)
I've got the patch to do multiple listeners off one Listen directive in my 
tree and tested, but I'm currently swamped with other stuff.

I'll also take a look at this patch for getnameinfo() when I commit the other 
portions.  Thanks for the feedback!  -- justin

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Thu, Aug 14, 2003 at 05:48:38PM -0400, John K. Sterling wrote:
> On Thursday, August 14, 2003, at 05:20 PM, Colm MacCarthaigh,,, wrote:
> 
> >Can you just confirm it's listening in v6 only ? the output of
> >"netstat -an | grep LISTEN" (Darwin has netstat and grep, right?)
> >should be enough.
> 
> heh.  very funny:
> 
> % netstat -an | grep LISTEN
> tcp46  0  0  *.80   *.*
> LISTEN

good good, patch works so :) In which I'll now cc [EMAIL PROTECTED] and include 
the neccessary autoconf voodoo to fix OSX/Darwin.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
Index: configure.in
===
RCS file: /home/cvspublic/apr/configure.in,v
retrieving revision 1.531
diff -u -r1.531 configure.in
--- configure.in11 Aug 2003 17:40:38 -  1.531
+++ configure.in14 Aug 2003 22:40:37 -
@@ -1778,17 +1778,6 @@
 fi ],
   [ user_disabled_ipv6=0 ] )
 
-case $host in
-  *apple-darwin*)
-# It appears that Jaguar has all the right features, but
-# getnameinfo() fails to find the hostname for a mapped
-# address.
-broken_ipv6=1
-;;
-  *)
-broken_ipv6=0
-esac
-
 AC_SEARCH_LIBS(getaddrinfo, socket inet6)
 AC_SEARCH_LIBS(gai_strerror, socket inet6)
 AC_SEARCH_LIBS(getnameinfo, socket inet6)
@@ -1803,23 +1792,19 @@
 if test "$user_disabled_ipv6" = 1; then
 AC_MSG_RESULT([no -- disabled by user])
 else
-if test "x$broken_ipv6" = "x0"; then
-if test "x$have_sockaddr_in6" = "x1"; then
-if test "x$ac_cv_working_getaddrinfo" = "xyes"; then
-if test "x$ac_cv_working_getnameinfo" = "xyes"; then
-have_ipv6="1"
-AC_MSG_RESULT([yes])
-else
-AC_MSG_RESULT([no -- no getnameinfo])
-fi
+if test "x$have_sockaddr_in6" = "x1"; then
+if test "x$ac_cv_working_getaddrinfo" = "xyes"; then
+if test "x$ac_cv_working_getnameinfo" = "xyes"; then
+have_ipv6="1"
+AC_MSG_RESULT([yes])
 else
-AC_MSG_RESULT([no -- no working getaddrinfo])
+AC_MSG_RESULT([no -- no getnameinfo])
 fi
 else
-AC_MSG_RESULT([no -- no sockaddr_in6])
+AC_MSG_RESULT([no -- no working getaddrinfo])
 fi
 else
-AC_MSG_RESULT([no -- the platform has problems supporting IPv6])
+AC_MSG_RESULT([no -- no sockaddr_in6])
 fi
 fi
 
Index: network_io/unix/sockaddr.c
===
RCS file: /home/cvspublic/apr/network_io/unix/sockaddr.c,v
retrieving revision 1.41
diff -u -r1.41 sockaddr.c
--- network_io/unix/sockaddr.c  14 Aug 2003 17:36:17 -  1.41
+++ network_io/unix/sockaddr.c  14 Aug 2003 22:40:38 -
@@ -634,9 +634,28 @@
  * a numeric address string if it fails to resolve the host name;
  * that is *not* what we want here
  */
+#ifdef DARWIN && APR_HAVE_IPV6
+/* Darwin's getnameinfo does not work for IPv4-mapped-IPv6 addresses,
+ * the workaround is to convert the last 32 bits of the IPv6 address 
+ * to a valid sockaddr_in structure. This is extremely hacky, avoid
+ * re-use. */
+if (sockaddr->family == AF_INET6 && 
IN6_IS_ADDR_V4MAPPED(&sockaddr->sa.sin6.sin6_addr))
+{
+struct apr_sockaddr_t tmpsa;
+
+tmpsa.sa.sin.sin_family = AF_INET;
+tmpsa.sa.sin.sin_addr.s_addr = ((uint32_t *)sockaddr->ipaddr_ptr)[3]; 
+
+rc = getnameinfo((const struct sockaddr *)&tmpsa.sa, sizeof(struct 
sockaddr_in),
+ tmphostname, sizeof(tmphostname), NULL, 0,
+ flags != 0 ? flags : NI_NAMEREQD);
+}
+else
+#endif
 rc = getnameinfo((const struct sockaddr *)&sockaddr->sa, sockaddr->salen,
  tmphostname, sizeof(tmphostname), NULL, 0,
  flags != 0 ? flags : NI_NAMEREQD);
+
 if (rc != 0) {
 *hostname = NULL;
 

Re: cvs commit: httpd-2.0/server listen.c

[Brad Nicholes]

   I couldn't answer that question for sure.  Taking a quick look
through the Apache code and modules, I don't see anything.  But that
doesn't mean that some third party module doesn't depend on it.  Do the
recent changes, include allowing sa->hostname to be NULL where it didn't
before?

  From what I see in CVS now, there is only one implementation of
apr_sockaddr_info_get() which is in unix/sockaddr.c.  This calls one of
two implementation of find_addresses() (whether you have getaddrinfo()
or not).  This is where sa->hostname is being defaulted to "0.0.0.0" (
in the non-getaddrinfo() implementation).  It seems that both
implementations should be sync'ed up to produce the same results.  It
appears that defaulting to "0.0.0.0" rather than NULL would be the safer
thing to do.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, August 13, 2003 4:18:19 PM >>>
--On Wednesday, August 13, 2003 16:07:12 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

>I guess I don't follow here.  The problem I have is that
> alloc_listener() allows "addr" to be NULL.  Because of the lame

Okay, let me try to restate the problem:

- addr *may* be NULL when alloc_listener is invoked.  Adding a NULL
check 
before doing the strcmp would indeed fix this problem, but prevents 
listener reuse.

- We *have* to pass NULL to apr_sockaddr_info_get so that getaddrname()
can 
do the IPv6/IPv4 fail-over on its own (as it is supposed to do - httpd

can't do the guessing correctly).

- However, on *some* implementations (based on the code in CVS right
now), 
the sa->hostname returned by apr_sockaddr_info_get may be NULL or
"0.0.0.0" 
even if we passed NULL into apr_sockaddr_info_get.

What we're trying to accomplish in this code fragment is to reuse old 
walkers that are already listening on the same port.  So, this 
inconsistency means that an addr with a value of NULL *could* be
equivalent 
to NULL or "0.0.0.0" (if the OS doesn't have IPv6).  So, one of two
things 
should happen:

- We should allow NULLs in sa->hostname.  We fix up the IPv4
implementation 
to not store "0.0.0.0" in the sa->hostname field, but NULL.

- We don't allow NULLs in sa->hostname.  Then, we need to take the
'name' 
returned by getaddrinfo() and store it.  However, we would have to do
this 
resolution before we can do the walking as NULL may resolve to (say): 
'0.0.0.0' or '::'.  Only after calling apr_sockaddr_info_get would we
know 
which it resolves to.

My question was whether anything relies upon sa->hostname being a valid

value?  I don't think they should, but the sockaddr_t is part of the
public 
conn_rec.  -- justin

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[John K . Sterling]

On Thursday, August 14, 2003, at 05:20 PM, Colm MacCarthaigh,,, wrote:

Can you just confirm it's listening in v6 only ? the output of
"netstat -an | grep LISTEN" (Darwin has netstat and grep, right?)
should be enough.
heh.  very funny:

% netstat -an | grep LISTEN
tcp46  0  0  *.80   *.*
LISTEN

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Thu, Aug 14, 2003 at 04:56:48PM -0400, John K. Sterling wrote:
> Hi Colm -
> 
> I'm not sure what to be looking for, but i applied your patch, rebuilt 
> (with the broken_ipv6 set to 0, of course), turned on HostnameLookups, 
> and my access logs have remote hostname properly resolved.  

That should mean it's working fine :)

> I'm running 
> Darwin Kernel Version 6.6.  Let me know if you want me to try out 
> anything else.

Can you just confirm it's listening in v6 only ? the output of
"netstat -an | grep LISTEN" (Darwin has netstat and grep, right?)
should be enough.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[John K . Sterling]

On Thursday, August 14, 2003, at 03:55 PM, Colm MacCarthaigh,,, wrote:

Patch attached. Since I havnt got a DARWIN machine to test on I
can't confirm it works, but it should. It works on Linux anyway
(not that it needs to).
Darwin might need some slightly different sa.[members] set to make
getnameinfo work but it shouldnt be anything major. If someone
with a darwin box could try it out I'd be much obliged. Turn
HostnameLookups On to get an idea as to whether it's working or
not.
Hi Colm -

I'm not sure what to be looking for, but i applied your patch, rebuilt 
(with the broken_ipv6 set to 0, of course), turned on HostnameLookups, 
and my access logs have remote hostname properly resolved.  I'm running 
Darwin Kernel Version 6.6.  Let me know if you want me to try out 
anything else.

sterling

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Thu, Aug 14, 2003 at 09:14:35AM -0700, Justin Erenkrantz wrote:
> - Trim IPv6 addresses from getaddrinfo()'s return values if APR_HAVE_IPV6 
> is 0.
>
>  (Perhaps only allow PF_INET sockets values to go in there.)
> 
> - Re-enable IPv6 on Darwin, and try to come up with a better solution to 
> the problem proposed last October.  (I proposed this before, but no one was 
> interested.)

Both these things really need to happen. The first is relatively
trivial:

Index: sockaddr.c
===
RCS file: /home/cvspublic/apr/network_io/unix/sockaddr.c,v
retrieving revision 1.40
diff -u -r1.40 sockaddr.c
--- sockaddr.c  14 Aug 2003 00:09:28 -  1.40
+++ sockaddr.c  14 Aug 2003 16:39:47 -
@@ -599,6 +599,9 @@
 if (flags & APR_IPV6_ADDR_OK) {
 return APR_ENOTIMPL;
 }
+if (family == AF_UNSPEC) {
+family = AF_INET:
+}
 #endif
 }
 

.. or similar. The darwin one is complicated, but I know of at least
3 people running Apache on Darwin for months, all they've been doing
is changing v6_broken to 0 in the apr configure script ;)

Until a truly AF agnostic ACL implementation can be used (see 
http://www.stdlib.net/~colmmacc/nsd/subnet.h and the accomponying
subnet.c for a rough idea) the Darwin problem should be solveable
by checking the IN6_IS_ADDR_V4MAPPED macro, and if so ignoring
the first 96 bytes and passing the last 32 on.

> And, I'm not sure if this is expected or not, but binding to the IPv6 
> address on my Darwin box does *not* bind to the IPv4 interface.  In order 
> to do that, we'd have to bind to each address returned from getaddrinfo().  

This is the bug I was trying to get accross (badly) the day before
yesterday. The fact that getaddrinfo returns a linked list is pretty
much ignored by alloc_listener :(

> Try out:
> 
> http://www.apache.org/~jerenkrantz/getaddr.c

That has a a bug of it's own :)

--- getaddr.c   Thu Aug 14 17:12:10 2003
+++ getaddr-new.c   Thu Aug 14 17:58:22 2003
@@ -2,6 +2,7 @@
 #include 
 #include 
 #include 
+#include 
 
 int main() {
 struct addrinfo hints, *res, *res0;
@@ -42,7 +43,7 @@
 s = -1;
 continue;
 }
-if (accept(s, &client, &addrlen) < 0) {
+if (accept(s, res->ai_addr, &res->ai_addrlen) < 0) {
 cause = "accept";
 printf("%s %d\n", cause, errno);
 close(s);

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Wednesday, August 13, 2003 16:07:12 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

   I guess I don't follow here.  The problem I have is that
alloc_listener() allows "addr" to be NULL.  Because of the lame
Okay, let me try to restate the problem:

- addr *may* be NULL when alloc_listener is invoked.  Adding a NULL check 
before doing the strcmp would indeed fix this problem, but prevents 
listener reuse.

- We *have* to pass NULL to apr_sockaddr_info_get so that getaddrname() can 
do the IPv6/IPv4 fail-over on its own (as it is supposed to do - httpd 
can't do the guessing correctly).

- However, on *some* implementations (based on the code in CVS right now), 
the sa->hostname returned by apr_sockaddr_info_get may be NULL or "0.0.0.0" 
even if we passed NULL into apr_sockaddr_info_get.

What we're trying to accomplish in this code fragment is to reuse old 
walkers that are already listening on the same port.  So, this 
inconsistency means that an addr with a value of NULL *could* be equivalent 
to NULL or "0.0.0.0" (if the OS doesn't have IPv6).  So, one of two things 
should happen:

- We should allow NULLs in sa->hostname.  We fix up the IPv4 implementation 
to not store "0.0.0.0" in the sa->hostname field, but NULL.

- We don't allow NULLs in sa->hostname.  Then, we need to take the 'name' 
returned by getaddrinfo() and store it.  However, we would have to do this 
resolution before we can do the walking as NULL may resolve to (say): 
'0.0.0.0' or '::'.  Only after calling apr_sockaddr_info_get would we know 
which it resolves to.

My question was whether anything relies upon sa->hostname being a valid 
value?  I don't think they should, but the sockaddr_t is part of the public 
conn_rec.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Wednesday, August 13, 2003 17:13:33 -0700 Justin Erenkrantz 
<[EMAIL PROTECTED]> wrote:

I just committed fixes to sync up the other way (NULL returns) and added
back the 'retry' logic if we have IPv6-enabled, but not configured (but
in a different way than before).
Looking at the original find_default_family code, it also tried to do a 
bind to see if IPv6 works just for 'certain levels of OpenUNIX.'  So, the 
socket() call would work on those OSes, but not later on when we try to 
bind to the socket.  (So it seems to me based on that comment.)

I guess what would make sense to me to do is to try the apr_bind() in that 
code segment where we create the socket on those releases of OpenUNIX with 
a platform-specific define.  If it works, close the socket, then recreate 
it.  (Ugh, but we're stretching for ideas here and doing the bind() for 
everyone seems, well, silly.)  But, I have no idea what 'OpenUNIX' is, nor 
how to test it.

Anyone with any thoughts?  (Solaris still works, FWIW.)  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Wed, Aug 13, 2003 at 03:18:19PM -0700, Justin Erenkrantz wrote:
> - addr *may* be NULL when alloc_listener is invoked.  Adding a NULL check 
> before doing the strcmp would indeed fix this problem, but prevents 
> listener reuse.

Since NULL only defines unqualified addresses, ie: "Listen 80" , any reuse 
on the same port will always fail. Would it be enough to simply do:

 if (sa) {
 apr_sockaddr_port_get(&oldport, sa);
- if (!strcmp(sa->hostname, addr) && port == oldport) {
+ if (( !addr || strcmp(sa->hostname, add)) && port == oldport) {
  /* re-use existing record */

After all, if someone did:

Listen 80

and then:

Listen 127.0.0.1 80

Shouldnt the original socket get re-used ? Or alternatively an error 
delivered.

> - We don't allow NULLs in sa->hostname.  Then, we need to take the 'name' 
> returned by getaddrinfo() and store it.  However, we would have to do this 
> resolution before we can do the walking as NULL may resolve to (say): 
> '0.0.0.0' or '::'.  Only after calling apr_sockaddr_info_get would we know 
> which it resolves to.

getaddrinfo won't return a 'name' in this situation though, even with
AI_CANONAME set for ai_flags. the v6 call_resolver would have to detect the
passive bind and hardcode '::' and '0.0.0.0' all over again :(

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Thu, Aug 14, 2003 at 10:41:44AM -0700, Justin Erenkrantz wrote:
> --On Thursday, August 14, 2003 6:06 PM +0100 "Colm MacCarthaigh,,," 
> <[EMAIL PROTECTED]> wrote:
> 
> >Both these things really need to happen. The first is relatively
> >trivial:
> 
> Yup, I agree.  I committed a variant of this to the tree.  (The patch you 
> submitted would only execute if the APR_IPV4_ADDR_ONLY flag or whatever 
> would be passed in.)

Good good, silly me :)

> >Until a truly AF agnostic ACL implementation can be used (see
> >http://www.stdlib.net/~colmmacc/nsd/subnet.h and the accomponying
> >subnet.c for a rough idea) the Darwin problem should be solveable
> >by checking the IN6_IS_ADDR_V4MAPPED macro, and if so ignoring
> >the first 96 bytes and passing the last 32 on.
> 
> Hmm.  Would it be possible to submit a patch for this?  I'm not entirely 
> clear where this would go.

Patch attached. Since I havnt got a DARWIN machine to test on I
can't confirm it works, but it should. It works on Linux anyway
(not that it needs to).

Darwin might need some slightly different sa.[members] set to make
getnameinfo work but it shouldnt be anything major. If someone
with a darwin box could try it out I'd be much obliged. Turn
HostnameLookups On to get an idea as to whether it's working or
not.

Basically the patch spoofs enough of a valid _in v4 type structure
to make getnameinfo work, and grabs the address itself from the
appropriate place in the mapped one.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/
Index: sockaddr.c
===
RCS file: /home/cvspublic/apr/network_io/unix/sockaddr.c,v
retrieving revision 1.41
diff -u -r1.41 sockaddr.c
--- sockaddr.c  14 Aug 2003 17:36:17 -  1.41
+++ sockaddr.c  14 Aug 2003 19:48:38 -
@@ -634,9 +634,28 @@
  * a numeric address string if it fails to resolve the host name;
  * that is *not* what we want here
  */
+#ifdef DARWIN && APR_HAVE_IPV6
+/* Darwin's getnameinfo does not work for IPv4-mapped-IPv6 addresses,
+ * the workaround is to convert the last 32 bits of the IPv6 address 
+ * to a valid sockaddr_in structure. This is extremely hacky, avoid
+ * re-use. */
+if (sockaddr->family == AF_INET6 && 
IN6_IS_ADDR_V4MAPPED(&sockaddr->sa.sin6.sin6_addr))
+{
+struct apr_sockaddr_t tmpsa;
+
+tmpsa.sa.sin.sin_family = AF_INET;
+tmpsa.sa.sin.sin_addr.s_addr = ((uint32_t *)sockaddr->ipaddr_ptr)[3]; 
+
+rc = getnameinfo((const struct sockaddr *)&tmpsa.sa, sizeof(struct 
sockaddr_in),
+ tmphostname, sizeof(tmphostname), NULL, 0,
+ flags != 0 ? flags : NI_NAMEREQD);
+}
+else
+#endif
 rc = getnameinfo((const struct sockaddr *)&sockaddr->sa, sockaddr->salen,
  tmphostname, sizeof(tmphostname), NULL, 0,
  flags != 0 ? flags : NI_NAMEREQD);
+
 if (rc != 0) {
 *hostname = NULL;
 

Re: cvs commit: httpd-2.0/server listen.c

[John K . Sterling]

--On Wednesday, August 13, 2003 17:13:33 -0700 Justin Erenkrantz 
<[EMAIL PROTECTED]> wrote:

I just committed fixes to sync up the other way (NULL returns) and 
added
back the 'retry' logic if we have IPv6-enabled, but not configured 
(but
in a different way than before).
Hey Justin -

I'm still having problems with HEAD this morning on os X (Darwin Kernel 
Version 6.6) - is that expected?  (i'm not sure what the status is, but 
the above email implies a fix was applied).

(22)Invalid argument: make_sock: could not bind to address :80
no listening sockets available, shutting down
sterling

Re: cvs commit: httpd-2.0/server listen.c

[Brad Nicholes]

  This is looking much better.  At least we are not faulting anymore.  I
haven't been able to watch the code to make sure it is doing everything
else as expected, but it looks good so far.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, August 13, 2003 6:13:33 PM >>>
--On Wednesday, August 13, 2003 17:08:40 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

>   Right, so default to "::" for IPV6.  Either way, these two versions
of
> find_addresses() should be sync'ed up.

I just committed fixes to sync up the other way (NULL returns) and
added 
back the 'retry' logic if we have IPv6-enabled, but not configured (but
in 
a different way than before).

Please let me know how that works.  It works on Linux, and I'll test 
Solaris again.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Brad Nicholes]

This change causes a problem on NetWare.  In the function
alloc_listener() before the change,  the code did not allow the variable
addr to be NULL.  In the changed code addr is allowed to be NULL which
means that the call to strcmp() after the call to
apr_sockaddr_port_get() causes NetWare to fault.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, August 13, 2003 1:17:45 PM >>>
jerenkrantz2003/08/13 12:17:45

  Modified:.CHANGES
   server   listen.c
  Log:
  Correct failure with Listen directives on machines with IPv6 enabled
by
  removing find_default_family() and letting APR determine what should
be done
  without a hostname.
  
  This patch requires the corollary APR patch to properly call
getaddrinfo().
  
  (Justin modified Colm's patch to always walk the old listeners even
when
  we have an address.  That part of the patch wasn't really relevant.)
  
  Submitted by: Colm MacCárthaigh <[EMAIL PROTECTED]>
  Reviewed by:  Justin Erenkrantz
  
  Revision  ChangesPath
  1.1251+3 -0  httpd-2.0/CHANGES
  
  Index: CHANGES
  ===
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.1250
  retrieving revision 1.1251
  diff -u -u -r1.1250 -r1.1251
  --- CHANGES   12 Aug 2003 21:02:00 -  1.1250
  +++ CHANGES   13 Aug 2003 19:17:45 -  1.1251
  @@ -2,6 +2,9 @@
   
 [Remove entries to the current 2.0 section below, when
backported]
   
  +  *) Correct failure with Listen directives on machines with IPv6
enabled.
  + [Colm MacCárthaigh <[EMAIL PROTECTED]>, Justin Erenkrantz]
  +  
 *) Fix a link failure in mod_ssl when the OpenSSL libraries
contain
the ENGINE functions but the engine header files are missing.
[Cliff Woolley]
  
  
  
  1.87  +0 -50 httpd-2.0/server/listen.c
  
  Index: listen.c
  ===
  RCS file: /home/cvs/httpd-2.0/server/listen.c,v
  retrieving revision 1.86
  retrieving revision 1.87
  diff -u -u -r1.86 -r1.87
  --- listen.c  31 Mar 2003 04:30:38 -  1.86
  +++ listen.c  13 Aug 2003 19:17:45 -  1.87
  @@ -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;
  @@ -274,24 +242,6 @@
   apr_status_t status;
   apr_port_t oldport;
   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) {
  
  
  

Re: cvs commit: httpd-2.0/server listen.c

[Brad Nicholes]

   I guess I don't follow here.  The problem I have is that
alloc_listener() allows "addr" to be NULL.  Because of the lame
implementation of strcmp() on NetWare, this causes a fault  (The same
thing would happen if sa->hostname where NULL also in this function). 
Allowing sa->hostname to be NULL by definition is fine.  I just need to
make sure that functions like strcmp() don't get called with a NULL
value.  Therefore, in this particular case, either "addr" needs to be
initialized as it was before, or a check needs to be made before calling
strcmp().  I just don't know which is the correct behavior given the
refactoring of the code.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, August 13, 2003 3:44:52 PM >>>
--On Wednesday, August 13, 2003 15:14:40 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

> This change causes a problem on NetWare.  In the function
> alloc_listener() before the change,  the code did not allow the
variable
> addr to be NULL.  In the changed code addr is allowed to be NULL
which
> means that the call to strcmp() after the call to
> apr_sockaddr_port_get() causes NetWare to fault.

Hmm.  Now, I wonder how reliant we are upon the sockaddr_t having a 
non-NULL hostname field.  Are we going to be allowed to leave it NULL?

apr_sockaddr_info_get's documentation explicitly says it allows NULL 
inputs, but it doesn't say what the 'hostname' field will turn into.

The IPv4 apr_sockaddr_info_get impl. just hardcodes sa->hostname to be

"0.0.0.0", while the IPv6 branch (getaddrinfo impl) will leave it NULL.

So, we may not reuse listeners on IPv4 right now (NULL gets translated
into 
"0.0.0.0" and stored as that string in the sockaddr_t).  I guess I'll 
prefer to have the hostname be what we passed into
apr_sockaddr_info_get - 
that is the sa->hostname should be NULL even with IPv4
implementations.

Then, we'll have to fix up the references to sa->hostname to allow for
NULL 
values.

Thoughts?  Make sense?  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Colm MacCarthaigh,,,]

On Wed, Aug 13, 2003 at 11:30:04PM +0100, Colm MacCarthaigh,,, wrote:
> 
> After all, if someone did:
> 
> Listen 80
> 
> and then:
> 
> Listen 127.0.0.1 80
> 
> Shouldnt the original socket get re-used ? Or alternatively an error 
> delivered.

*actually thinks about it*

obviously then "Listen 127.0.0.1 80" followed by "Listen 80" wont work
in an acceptable manner. It really needs to bomb out if anything tries
to listen on the same port as a passive address.

-- 
Colm MacCárthaighPublic Key: [EMAIL PROTECTED]
[EMAIL PROTECTED] http://www.stdlib.net/

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[John K . Sterling]

On Thursday, August 14, 2003, at 01:06 PM, Colm MacCarthaigh,,, wrote:

.. or similar. The darwin one is complicated, but I know of at least
3 people running Apache on Darwin for months, all they've been doing
is changing v6_broken to 0 in the apr configure script ;)
Yup.. that worked.  make that 4 people :)

sterling

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Wednesday, August 13, 2003 15:14:40 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

This change causes a problem on NetWare.  In the function
alloc_listener() before the change,  the code did not allow the variable
addr to be NULL.  In the changed code addr is allowed to be NULL which
means that the call to strcmp() after the call to
apr_sockaddr_port_get() causes NetWare to fault.
Hmm.  Now, I wonder how reliant we are upon the sockaddr_t having a 
non-NULL hostname field.  Are we going to be allowed to leave it NULL? 
apr_sockaddr_info_get's documentation explicitly says it allows NULL 
inputs, but it doesn't say what the 'hostname' field will turn into.

The IPv4 apr_sockaddr_info_get impl. just hardcodes sa->hostname to be 
"0.0.0.0", while the IPv6 branch (getaddrinfo impl) will leave it NULL. 
So, we may not reuse listeners on IPv4 right now (NULL gets translated into 
"0.0.0.0" and stored as that string in the sockaddr_t).  I guess I'll 
prefer to have the hostname be what we passed into apr_sockaddr_info_get - 
that is the sa->hostname should be NULL even with IPv4 implementations.

Then, we'll have to fix up the references to sa->hostname to allow for NULL 
values.

Thoughts?  Make sense?  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Brad Nicholes]

  Right, so default to "::" for IPV6.  Either way, these two versions of
find_addresses() should be sync'ed up.

Brad

Brad Nicholes
Senior Software Engineer
Novell, Inc., the leading provider of Net business solutions
http://www.novell.com 

>>> [EMAIL PROTECTED] Wednesday, August 13, 2003 5:00:21 PM >>>
--On Wednesday, August 13, 2003 16:57:34 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

> in the non-getaddrinfo() implementation).  It seems that both
> implementations should be sync'ed up to produce the same results. 
It
> appears that defaulting to "0.0.0.0" rather than NULL would be the
safer
> thing to do.

But, that's an IPv4 address, but we support IPv6.

Apparently, this series of commits broke Linux.  I bet this is where
Joe's 
ADDRCONFIG patch comes into play.  Looking into it now.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Wednesday, August 13, 2003 17:08:40 -0600 Brad Nicholes 
<[EMAIL PROTECTED]> wrote:

  Right, so default to "::" for IPV6.  Either way, these two versions of
find_addresses() should be sync'ed up.
I just committed fixes to sync up the other way (NULL returns) and added 
back the 'retry' logic if we have IPv6-enabled, but not configured (but in 
a different way than before).

Please let me know how that works.  It works on Linux, and I'll test 
Solaris again.  -- justin

Re: Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Thursday, August 14, 2003 6:06 PM +0100 "Colm MacCarthaigh,,," 
<[EMAIL PROTECTED]> wrote:

Both these things really need to happen. The first is relatively
trivial:
Yup, I agree.  I committed a variant of this to the tree.  (The patch you 
submitted would only execute if the APR_IPV4_ADDR_ONLY flag or whatever would 
be passed in.)

Until a truly AF agnostic ACL implementation can be used (see
http://www.stdlib.net/~colmmacc/nsd/subnet.h and the accomponying
subnet.c for a rough idea) the Darwin problem should be solveable
by checking the IN6_IS_ADDR_V4MAPPED macro, and if so ignoring
the first 96 bytes and passing the last 32 on.
Hmm.  Would it be possible to submit a patch for this?  I'm not entirely clear 
where this would go.

This is the bug I was trying to get across (badly) the day before
yesterday. The fact that getaddrinfo returns a linked list is pretty
much ignored by alloc_listener :(
Okay, let me take a pass at trying to fix this.  I think this should be 
solvable pretty easily.

http://www.apache.org/~jerenkrantz/getaddr.c
That has a a bug of it's own :)
Thanks.  Fixed.  ;-)  -- justin

Darwin and IPv6 was Re: cvs commit: httpd-2.0/server listen.c

[Justin Erenkrantz]

--On Thursday, August 14, 2003 7:56 AM -0400 "John K. Sterling" 
<[EMAIL PROTECTED]> wrote:

I'm still having problems with HEAD this morning on os X (Darwin Kernel
Version 6.6) - is that expected?  (i'm not sure what the status is, but the
above email implies a fix was applied).
(22)Invalid argument: make_sock: could not bind to address :80
no listening sockets available, shutting down
No, it's not expected.  ;-)

But, here's what the problem is:

- We've disabled IPv6 across the board on Darwin because getnameinfo() doesn't 
do IPv6->IPv4 mapping correctly (my rough probably incorrect remembrance of 
the thread).  The thread in question was from last October:



- We're getting IPv6 addresses back from getaddrinfo() because, well, that's 
what getaddrinfo is supposed to return.  The IPv6 addresses are returned back 
to alloc_listener.

- APR_HAVE_IPV6 is 0, so the apr_sockaddr_t doesn't contain the sin6 field in 
the union.

- You'll notice that the 22 is EINVAL (*not* EHOSTUNREACH).  That's because 
the sockaddr_in6 that we copied from getaddrinfo's return list is corrupted.

So, a few suggestions:

- Trim IPv6 addresses from getaddrinfo()'s return values if APR_HAVE_IPV6 is 0.
 (Perhaps only allow PF_INET sockets values to go in there.)
- Re-enable IPv6 on Darwin, and try to come up with a better solution to the 
problem proposed last October.  (I proposed this before, but no one was 
interested.)

And, I'm not sure if this is expected or not, but binding to the IPv6 address 
on my Darwin box does *not* bind to the IPv4 interface.  In order to do that, 
we'd have to bind to each address returned from getaddrinfo().  But, we'll 
only bind to the first one.  I think that's wrong.  Or, perhaps binding to the 
IPv6 interface is supposed to bind to the IPv4 interface, but my tests on 
Darwin don't show that to hold.  Try out:

http://www.apache.org/~jerenkrantz/getaddr.c

This IPv6 code is just a complete mess.  -- justin

Re: cvs commit: httpd-2.0/server listen.c

[Jeff Trawick]

hiroyuki hanai <[EMAIL PROTECTED]> writes:

> On 14 Nov 2002 14:17:11 -, [EMAIL PROTECTED] wrote:
> 
> > trawick 2002/11/14 06:17:11
> > 
> >   Modified:.CHANGES acinclude.m4 configure.in
> >docs/conf httpd-std.conf.in ssl-std.conf
> >server   listen.c
> >   Log:
> >   Add --[enable|disable]-v4-mapped configure option to control
> >   whether or not Apache expects to handle IPv4 connections
> >   on IPv6 listening sockets.  Either setting will work on
> >   systems with the IPV6_V6ONLY socket option.  --enable-v4-mapped
> >   must be used on systems that always allow IPv4 connections on
> >   IPv6 listening sockets.
> 
> after this commit, httpd cannot run with following error;
> 
>   [Fri Nov 22 20:01:43 2002] [crit] (22)Invalid argument: make_sock: for address 
>127.0.0.1:80, apr_socket_opt_set: (IPV6_V6ONLY)
>   no listening sockets available, shutting down
>   Unable to open logs
> 
> on my FreeBSD boxes, both -current and -stable with ipv4-mapping
> disabled and enabled respectively.
> 
> i think this error occurs because IPV6_V6ONLY option are being set
> on all sockets; even if on the IPv4 sockets.
> 
> following is a patch to set IPV6_V6ONLY option on only IPv6 sockets.
> i don't know if this is a correct answer but it solves at least my problem...

Yes, this is so obviously a necessary change (blush).

I had so much trouble with the config-time changes to generate the
right httpd.conf that I guess my brain was fried when coming up with
test variations for the actual code :(

> Regards,
> 
> hiro hanai

Thanks so much!

I expect to commit shortly, after which the beatings will resume.

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/server listen.c

[hiroyuki hanai]

On 14 Nov 2002 14:17:11 -, [EMAIL PROTECTED] wrote:

> trawick 2002/11/14 06:17:11
> 
>   Modified:.CHANGES acinclude.m4 configure.in
>docs/conf httpd-std.conf.in ssl-std.conf
>server   listen.c
>   Log:
>   Add --[enable|disable]-v4-mapped configure option to control
>   whether or not Apache expects to handle IPv4 connections
>   on IPv6 listening sockets.  Either setting will work on
>   systems with the IPV6_V6ONLY socket option.  --enable-v4-mapped
>   must be used on systems that always allow IPv4 connections on
>   IPv6 listening sockets.

after this commit, httpd cannot run with following error;

  [Fri Nov 22 20:01:43 2002] [crit] (22)Invalid argument: make_sock: for address 
127.0.0.1:80, apr_socket_opt_set: (IPV6_V6ONLY)
  no listening sockets available, shutting down
  Unable to open logs

on my FreeBSD boxes, both -current and -stable with ipv4-mapping
disabled and enabled respectively.

i think this error occurs because IPV6_V6ONLY option are being set
on all sockets; even if on the IPv4 sockets.

following is a patch to set IPV6_V6ONLY option on only IPv6 sockets.
i don't know if this is a correct answer but it solves at least my problem...

Regards,

hiro hanai

-
Index: listen.c
===
RCS file: /fs/pub/cvs/Apache/httpd-2.0/server/listen.c,v
retrieving revision 1.83
diff -u -r1.83 listen.c
--- listen.c14 Nov 2002 14:17:11 -  1.83
+++ listen.c22 Nov 2002 09:52:45 -
@@ -118,14 +118,16 @@
 }
 
 #if APR_HAVE_IPV6
-stat = apr_socket_opt_set(s, APR_IPV6_V6ONLY, v6only_setting);
-if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
+if (server->bind_addr->family == AF_INET6) {
+  stat = apr_socket_opt_set(s, APR_IPV6_V6ONLY, v6only_setting);
+  if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
 ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p,
   "make_sock: for address %pI, apr_socket_opt_set: "
   "(IPV6_V6ONLY)",
   server->bind_addr);
 apr_socket_close(s);
 return stat;
+  }
 }
 #endif
-

RE: cvs commit: httpd-2.0/server listen.c

[Sander Striker]

> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick
> Sent: 20 March 2002 11:48

> [EMAIL PROTECTED] writes:
> 
> > wrowe   02/03/19 23:03:07
> > 
> >   Modified:server   listen.c
> >   Log:
> > Here's the patch that really sucks.  old_listeners points to an array
> > of apr_socket objects already destroyed by their cleanups, and in any
> > case they now live in invalid memory.  Extend their lifetimes.
> >   
> > This implies that the process pool grows on every restart for no good
> > reason.  One possible solution is to let the old pconf survive until
> > the new pconf is alive.  Another is to create the listeners in a subpool
> > of process->pool, destroyed after the old_listeners are closed.
> >   
> > Either which way, a better solution exists, but this closes the immediate
> > bug.  [How haven't we been segfaulting in unix on restarts before this
> > patch, gurus?]
> 
> good question...  even more interesting (to me) is the question "how
> haven't we been segfaulting in unix on restarts even when we build with
> electric fence and pool debug, time after time after time" :)
> 
> maybe somebody can make us both happy with the answer

Maybe the memory wasn't being accessed on unix??

Sander 'with a puzzled look on his face'

Re: cvs commit: httpd-2.0/server listen.c

[Jeff Trawick]

[EMAIL PROTECTED] writes:

> wrowe   02/03/19 23:03:07
> 
>   Modified:server   listen.c
>   Log:
> Here's the patch that really sucks.  old_listeners points to an array
> of apr_socket objects already destroyed by their cleanups, and in any
> case they now live in invalid memory.  Extend their lifetimes.
>   
> This implies that the process pool grows on every restart for no good
> reason.  One possible solution is to let the old pconf survive until
> the new pconf is alive.  Another is to create the listeners in a subpool
> of process->pool, destroyed after the old_listeners are closed.
>   
> Either which way, a better solution exists, but this closes the immediate
> bug.  [How haven't we been segfaulting in unix on restarts before this
> patch, gurus?]

good question...  even more interesting (to me) is the question "how
haven't we been segfaulting in unix on restarts even when we build with
electric fence and pool debug, time after time after time" :)

maybe somebody can make us both happy with the answer

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: Windows is able to listen on the same port with multiple servers?! WAS: RE: cvs commit: httpd-2.0/server listen.c

[rbb]

On Tue, 5 Feb 2002, Sander Striker wrote:

> Hi,
> 
> Stupid question time: how does windows hand of something to the right server?
> I thought that the port/ip combination was supposed to be unique.

It doesn't.  Basically, the second server can bind to the port without an
error, but it won't get any normal connections.  In order for the second
server to get a connection, it must be a broadcast connection.  At least,
that is how I am reading the docs.

Ryan

> 
> > [EMAIL PROTECTED] writes:
> > 
> > > rbb 02/02/04 22:16:04
> > > 
> > >   Modified:.STATUS
> > >server   listen.c
> > >   Log:
> > >   This change keeps the server from allowing multiple instances to bind to
> > >   the same port.  Previously, this was necessary, because the Windows MPM
> > >   was binding to the socket in both the parent and child.  Today's code
> > >   passes the attached socket to the child from the parent, so we don't need
> > >   to re-attach in the child.
> > 
> > what the *&%#...  you broke everybody else (and perhaps Windows in
> > some scenarios) 
> > 
> > -1
> > 
> > This change also prevents a single instance (i.e., normal scenario) to
> > bind to the desired port when an old connection is in TIME_WAIT or some
> > other TCP state.
> > 
> > I can't see any SO_REUSEADDR being done on Unix now and meanwhile I
> > have a continual stream of regression test failures saying:
> > 
> > (98)Address already in use: make_sock: could not bind to address
> > 0.0.0.0:8099
> > no listening sockets available, shutting down
> 
> Very nice.  Great for restarts... Not.
> 
> 
> Sander
> 

-- 

___
Ryan Bloom  [EMAIL PROTECTED]
550 Jean St
Oakland CA 94610
---

Windows is able to listen on the same port with multiple servers ?! WAS: RE: cvs commit: httpd-2.0/server listen.c

[Sander Striker]

Hi,

Stupid question time: how does windows hand of something to the right server?
I thought that the port/ip combination was supposed to be unique.

> [EMAIL PROTECTED] writes:
> 
> > rbb 02/02/04 22:16:04
> > 
> >   Modified:.STATUS
> >server   listen.c
> >   Log:
> >   This change keeps the server from allowing multiple instances to bind to
> >   the same port.  Previously, this was necessary, because the Windows MPM
> >   was binding to the socket in both the parent and child.  Today's code
> >   passes the attached socket to the child from the parent, so we don't need
> >   to re-attach in the child.
> 
> what the *&%#...  you broke everybody else (and perhaps Windows in
> some scenarios) 
> 
> -1
> 
> This change also prevents a single instance (i.e., normal scenario) to
> bind to the desired port when an old connection is in TIME_WAIT or some
> other TCP state.
> 
> I can't see any SO_REUSEADDR being done on Unix now and meanwhile I
> have a continual stream of regression test failures saying:
> 
> (98)Address already in use: make_sock: could not bind to address
> 0.0.0.0:8099
> no listening sockets available, shutting down

Very nice.  Great for restarts... Not.


Sander

Re: cvs commit: httpd-2.0/server listen.c

[Jeff Trawick]

[EMAIL PROTECTED] writes:

> rbb 02/02/04 22:16:04
> 
>   Modified:.STATUS
>server   listen.c
>   Log:
>   This change keeps the server from allowing multiple instances to bind to
>   the same port.  Previously, this was necessary, because the Windows MPM
>   was binding to the socket in both the parent and child.  Today's code
>   passes the attached socket to the child from the parent, so we don't need
>   to re-attach in the child.

what the *&%#...  you broke everybody else (and perhaps Windows in
some scenarios) 

-1

This change also prevents a single instance (i.e., normal scenario) to
bind to the desired port when an old connection is in TIME_WAIT or some
other TCP state.

I can't see any SO_REUSEADDR being done on Unix now and meanwhile I
have a continual stream of regression test failures saying:

(98)Address already in use: make_sock: could not bind to address
0.0.0.0:8099
no listening sockets available, shutting down

>   1.70  +0 -9  httpd-2.0/server/listen.c
>   
>   Index: listen.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/server/listen.c,v
>   retrieving revision 1.69
>   retrieving revision 1.70
>   diff -u -r1.69 -r1.70
>   --- listen.c4 Feb 2002 16:58:54 -   1.69
>   +++ listen.c5 Feb 2002 06:16:04 -   1.70
>   @@ -89,15 +89,6 @@
>int one = 1;
>apr_status_t stat;
>
>   -stat = apr_setsocketopt(s, APR_SO_REUSEADDR, one);
>   -if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
>   -ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p,
>   -"make_sock: for address %pI, setsockopt: (SO_REUSEADDR)", 
>   - server->bind_addr);
>   -apr_socket_close(s);
>   -return stat;
>   -}
>   -
>stat = apr_setsocketopt(s, APR_SO_KEEPALIVE, one);
>if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
>ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p,

-- 
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
   http://www.geocities.com/SiliconValley/Park/9289/
 Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/server listen.c

[Cliff Woolley]

On 5 Feb 2002 [EMAIL PROTECTED] wrote:

>   Index: listen.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/server/listen.c,v
>   retrieving revision 1.69
>   retrieving revision 1.70
>   diff -u -r1.69 -r1.70
>   --- listen.c4 Feb 2002 16:58:54 -   1.69
>   +++ listen.c5 Feb 2002 06:16:04 -   1.70
>   @@ -89,15 +89,6 @@
>int one = 1;
>apr_status_t stat;
>
>   -stat = apr_setsocketopt(s, APR_SO_REUSEADDR, one);
>   -if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
>   -ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p,
>   -"make_sock: for address %pI, setsockopt: (SO_REUSEADDR)",
>   - server->bind_addr);
>   -apr_socket_close(s);
>   -return stat;
>   -}
>   -
>stat = apr_setsocketopt(s, APR_SO_KEEPALIVE, one);
>if (stat != APR_SUCCESS && stat != APR_ENOTIMPL) {
>ap_log_perror(APLOG_MARK, APLOG_CRIT, stat, p,
>

This appears to have broken shutdowns on Unix.


Prior to this patch, httpd-test will do this:
root@deepthought:/root/apache/httpd-test/perl-framework# t/TEST
setting ulimit to allow core files
ulimit -c unlimited; t/TEST
/root/apache/test/bin/httpd  -d /root/apache/httpd-test-6/perl-framework/t
-f /root/apache/httpd-test-6/perl-framework/t/conf/httpd.conf -DAPACHE2
using Apache/2.0.32-dev (worker MPM)
waiting for server to start: ok (waited 0 secs)
server localhost:8529 started
server localhost:8530 listening (mod_ssl)
server localhost:8531 listening (mod_headers)
server localhost:8532 listening (mod_echo)
server localhost:8533 listening (mod_echo_ssl)
server localhost:8534 listening (mod_proxy)
server localhost:8535 listening (mod_vhost_alias)
server localhost:8536 listening (mod_nntp_like)
server localhost:8537 listening (mod_nntp_like_ssl)
apache/404..ok
apache/byterangeok 33/92^C
halting tests
server localhost:8529 shutdown
root@deepthought:/root/apache/httpd-test/perl-framework#



No problem.  But after this patch, this happens:

root@deepthought:/root/apache/httpd-test/perl-framework# t/TEST
setting ulimit to allow core files
ulimit -c unlimited; t/TEST
/root/apache/test/bin/httpd  -d /root/apache/httpd-test-6/perl-framework/t
-f /root/apache/httpd-test-6/perl-framework/t/conf/httpd.conf -DAPACHE2
using Apache/2.0.32-dev (worker MPM)
waiting for server to start: ok (waited 1 secs)
server localhost:8529 started
server localhost:8530 listening (mod_ssl)
server localhost:8531 listening (mod_headers)
server localhost:8532 listening (mod_echo)
server localhost:8533 listening (mod_echo_ssl)
server localhost:8534 listening (mod_proxy)
server localhost:8535 listening (mod_vhost_alias)
server localhost:8536 listening (mod_nntp_like)
server localhost:8537 listening (mod_nntp_like_ssl)
apache/404..ok
apache/byterangeok
apache/etagsok
apache/getfile..^C
halting tests
server localhost:8529 shutdown
port 8529 still in use...
.
server was shutdown but port 8529 is still in use, please shutdown the
service using this port or select another port for the tests
port 8529 is in use, cannot determine server pid to shutdown


[so we check the ps listing:]

root@deepthought:/root/apache/httpd-test/perl-framework# ps -afx |grep httpd
11453 pts/2S  0:00  |   \_ grep httpd

[notice no instances of httpd still running... so we check netstat]


root@deepthought:/root/apache/httpd-test/perl-framework# netstat -an
Active Internet connections (servers and established)
Proto Recv-Q Send-Q Local Address   Foreign Address  State
tcp0  0 0.0.0.0:111 0.0.0.0:*LISTEN
tcp0  0 0.0.0.0:60000.0.0.0:*LISTEN
tcp0  0 0.0.0.0:22  0.0.0.0:*LISTEN
tcp0  0 0.0.0.0:631 0.0.0.0:*LISTEN
tcp0  0 127.0.0.1:8529  127.0.0.1:59297  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59553  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59361  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59617  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59681  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59425  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59233  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59489  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59296  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59552  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59360  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59616  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59424  TIME_WAIT
tcp0  0 127.0.0.1:8529  127.0.0.1:59680  TIME_WAIT
tcp